1

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;                       
                }
            });
        }
    }
Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
sTg
  • 4,313
  • 16
  • 68
  • 115
  • so your object has a method like `getManufacturedDate()` ? – Leviand Oct 17 '18 at 09:56
  • try date.before(otherDate) and date.after(otherDate) instead. – curiouscupcake Oct 17 '18 at 09:56
  • 1
    @Leviand - I have a property manufacturedDate for which i am having its setter and getter – sTg Oct 17 '18 at 09:57
  • 1
    `null` values should be considered to be on the beginning or on the end of the list? – Flown Oct 17 '18 at 10:13
  • Describe your actual problem—better than saying “the sorting is not happening properly”. Further, explain what you mean with “The list does not returns values…” – Holger Oct 17 '18 at 10:15
  • I recommend you avoid the `SimpleDateFormat` class. It is not only long outdated, it is also notoriously troublesome. Today we have so much better in [`java.time`, the modern Java date and time API](https://docs.oracle.com/javase/tutorial/datetime/). – Ole V.V. Oct 17 '18 at 10:22
  • Did you search before asking? Always do that. There are other questions quite similar to this one. Start here: [Sort objects in ArrayList by date?](https://stackoverflow.com/questions/5927109/sort-objects-in-arraylist-by-date). – Ole V.V. Oct 17 '18 at 10:35
  • @OleV.V. : If you check my issue it is somewhat similar i agree with the post but when it is not working and i have put my efforts in searching as well as working i had put up this question. I am not sure how come this is duplicate where the solution itself is not working. #sad – sTg Oct 17 '18 at 10:55
  • @Holger - When i say sorting is not happening properly it means the list is neither in ascending or descending it comes up with some random order. if there are no null values it works properly. My question is not duplicate . – sTg Oct 17 '18 at 10:57
  • 1
    You should [edit] these information into your question. I did not vote for closing as duplicate. Unfortunately, Stackoverflow lists users flagging for other issues (like missing sufficient problem description) as if these users voted for the same thing when a question is closed. But besides that, the other Q&A *does* describe how to handle `null` and date parsing exceptions. – Holger Oct 17 '18 at 11:06

4 Answers4

2

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
  }
}
Thomas
  • 87,414
  • 12
  • 119
  • 157
  • 4
    …or better not doing this manually, when you can use something like `Comparator.comparing(CarDataTransferObject::getManufacturedDate, Comparator.nullsFirst(Comparator.comparing(string -> { try { return sdf1.parse(string); } catch (ParseException e) { log.error("Error inside sortList method"+e); return null; } })))` – Holger Oct 17 '18 at 10:24
  • @Holger yeah, that's the better way though maybe not as clear for someone struggling with the basic concept :) – Thomas Oct 17 '18 at 10:43
  • thanks for such a wonderful explaination @Thomas. Have accepted this answer .. :) – sTg Oct 17 '18 at 12:08
1

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]

Leviand
  • 2,745
  • 4
  • 29
  • 43
  • "With filter you can remove the null elements." - I have the impression what nulls should not be filtered though. – Thomas Oct 17 '18 at 10:45
  • @Leviand I would have accepted this as my answer. Issues with the answer. manufacturedDate is a String. and i cant ignore null values as i need to display them at the beginning while ascending and at the last while descending. – sTg Oct 17 '18 at 10:48
  • @sTg now should be fine, according to new specifications :) – Leviand Oct 17 '18 at 11:06
1

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.

Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
0

Try this:

 returnVal = sdf1.parse(object1.getManufacturedDate()).getTime() - sdf1.parse(object2.getManufacturedDate()).getTime();
Kishita Variya
  • 810
  • 9
  • 19