Reputation: 4424
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
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
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.
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.
Oracle tutorial: Date Time explaining how to use java.time
.
Upvotes: 1
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
Reputation: 810
Try this:
returnVal = sdf1.parse(object1.getManufacturedDate()).getTime() - sdf1.parse(object2.getManufacturedDate()).getTime();
Upvotes: 0
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