sTg
sTg

Reputation: 4424

Sorting date values in java

I have a List<object> where I have a property manufacturedDate where I have to do sorting. In the List<Object> there are some values as null for manufacturedDate. The sorting is not happening properly. I am not sure if it is because of the null values. The list does not returns values on asc or desc order of manufacturedDate.

Below is my code:

    if(sortType.equalsIgnoreCase("manufacturedDate"))
    {
        DateFormat sdf1 = new SimpleDateFormat("dd-MMM-yyyy", Locale.ENGLISH);
        if(manufacturedDateSortAsc.equalsIgnoreCase("Desc"))
        {
            manufacturedDateSortAsc = "Asc";
            Collections.sort(carList, new Comparator<CarDataTransferObject>() 
            {
                public int compare(CarDataTransferObject object1, CarDataTransferObject object2) 
                {
                    int returnVal = 0;
                    try 
                    {
                        returnVal = sdf1.parse(object1.getManufacturedDate()).compareTo(sdf1.parse(object2.getManufacturedDate()));
                    } 
                    catch (Exception e) 
                    {
                        log.error("Error inside sortList method"+e);
                    }
                    return returnVal;                       
                }
            });
        }
        else
        {
            usReleaseDateSortAsc = "Desc";
            Collections.sort(carList, new Comparator<CarDataTransferObject>() 
            {
                public int compare(CarDataTransferObject object1, CarDataTransferObject object2) 
                {
                    int returnVal = 0;
                    try 
                    {
                        returnVal = sdf1.parse(object2.getManufacturedDate()).compareTo(sdf1.parse(object1.getManufacturedDate()));                             
                    } 
                    catch (Exception e) 
                    {
                        log.error("Error inside sortList method"+e);
                    }
                    return returnVal;                       
                }
            });
        }
    }

Upvotes: 1

Views: 734

Answers (4)

Anonymous
Anonymous

Reputation: 86130

I am using this class for my demonstration:

public class CarDataTransferObject {

    private static final DateTimeFormatter DATE_FORMATTER
            = DateTimeFormatter.ofPattern("dd-MMM-yyyy", Locale.ENGLISH);

    LocalDate manufacturedDate;

    public CarDataTransferObject(String manufacturedDateString) {
        if (manufacturedDateString == null) {
            manufacturedDate = null;
        } else {
            manufacturedDate = LocalDate.parse(manufacturedDateString, DATE_FORMATTER);
        }
    }

    // getter, toString, …

}

To sort ascending:

    List<CarDataTransferObject> carList = Arrays.asList(
            new CarDataTransferObject("23-Sep-2018"),
            new CarDataTransferObject(null),
            new CarDataTransferObject("04-Jul-2018"),
            new CarDataTransferObject("11-Aug-2018"),
            new CarDataTransferObject(null),
            new CarDataTransferObject("30-May-2018"));
    carList.sort(Comparator.comparing(
            CarDataTransferObject::getManufacturedDate, 
            Comparator.nullsLast(Comparator.naturalOrder())));
    carList.forEach(System.out::println);

Output:

Car manufactured 30-May-2018
Car manufactured 04-Jul-2018
Car manufactured 11-Aug-2018
Car manufactured 23-Sep-2018
Car (no manufactured date)
Car (no manufactured date)

To sort descending:

    carList.sort(Comparator.comparing(
            CarDataTransferObject::getManufacturedDate, 
            Comparator.nullsFirst(Comparator.reverseOrder())));

Output:

Car (no manufactured date)
Car (no manufactured date)
Car manufactured 23-Sep-2018
Car manufactured 11-Aug-2018
Car manufactured 04-Jul-2018
Car manufactured 30-May-2018

java.time and other tips

Use LocalDate from java.time, the modern Java date and time API, for you manufactured date. Don’t use strings. LocalDate is a date without time of day, so what you need here. Only format the date into a string for presentation to the user or for serialization. java.time is built into Java from Java 8 (and has also been backported to Java 6 and 7).

The static methods in the Comparator interface also added in Java 8 — comparing, nullsFirst and many more — make building of fairly complex comparators not only more straightforward and terser, but also less error-prone.

What went wrong in your code?

You need to handle nulls explicitly. The sort method may choose any comparisons it likes (in practice it is deterministic which it uses, but don’t count on being able to look through it). So if your list contains an object with a null date, it may compare any two (or more) other objects to the one with the null. Each comparison yields 0, so the sort now “knows” (incorrectly) that all of those objects belong in the same place in the sorting. The sorting is stable, meaning that all of those objects come out in the same order they had in the list before sorting.

Instead tell the sort method whether you want the nulls first or last, and you will get what you ask for.

Link

Oracle tutorial: Date Time explaining how to use java.time.

Upvotes: 1

Leviand
Leviand

Reputation: 2805

Since you have tagget java 8, you can sort it by using stream:

System.out.println(list.stream()
            .sorted((obj1,obj2) -> {
                if(obj1.getManufacturedDate() != null && obj2 != null){
                    DateFormat sdf1 = new SimpleDateFormat("dd-MMM-yyyy", Locale.ENGLISH);
                    try {
                        Date date1 = sdf1.parse(obj1.getManufacturedDate());
                        Date date2 = sdf1.parse(obj2.getManufacturedDate());

                        return date1.compareTo(date2);
                    } catch (ParseException e) {
                        e.printStackTrace();
                    }
                }
                return 0;
            })
            .collect(Collectors.toList())
    );

Since you have a Date that is a String, we need to do that formatting.

I've used a structure like this class for testing:

private class MyObject {
    String manufacturedDate;

    public MyObject(String date){
        this.manufacturedDate = date;
    }
    public String getManufacturedDate(){
        return manufacturedDate;
    }

    @Override
    public String toString() {
        if(manufacturedDate != null) {
            return this.manufacturedDate;
        }else {
            return "empty";
        }
    }
}

So with sample data like this:

    List<MyObject> list = new ArrayList<>();
    list.add(new MyObject("01-JAN-2018"));
    list.add(new MyObject("01-FEB-2015"));
    list.add(new MyObject("01-MAR-2012"));
    list.add(new MyObject("01-JAN-2019"));
    list.add(new MyObject(null));

You will get as result:

[01-MAR-2012, 01-FEB-2015, 01-JAN-2018, 01-JAN-2019, empty]

Upvotes: 1

Kishita Variya
Kishita Variya

Reputation: 810

Try this:

 returnVal = sdf1.parse(object1.getManufacturedDate()).getTime() - sdf1.parse(object2.getManufacturedDate()).getTime();

Upvotes: 0

Thomas
Thomas

Reputation: 88707

I'm pretty sure this is a problem because of the null values.

Have a look at your compare() method:

int returnVal = 0;
try {
 returnVal = sdf1.parse(object2.getManufacturedDate()).compareTo(sdf1.parse(object1.getManufacturedDate()));                             
} catch (Exception e) {
  log.error("Error inside sortList method"+e);
}
return returnVal;      

What will happen if one of the two is null? You'll get an exception and return 0.

However, null should always be considered either greater or smaller than any non-null value. The requirement for Comparator is that the results must be transitive, i.e. if a < b and b < c then a < c. Now if one of those was null (let's say c) then your code would violate that, i.e. it could result in a < b, b = c and a = c which clearly violates the contract.

Thus you need to check for the nulls separately:

if(date1 == null ) {
  if (date2 == null) {
    return 0;
  } else {
    return /*either -1 or 1 depending on where nulls should end up*/
  }
} else {
  if (date2 == null) {
    return /*either 1 or -1 depending on where nulls should end up*/
  } else {
    return date1.compareTo(date2); //this assumes successful parsing, I'll leave those details for you
  }
}

Upvotes: 2

Related Questions