maple_shaft
maple_shaft

Reputation: 10463

Sort by date descending comparator not working as expected

Trying to make sense of the following output:

public class CommunicationComparator implements Comparator<Communication> {
    @Override
    public int compare(Communication comm1, Communication comm2) {
        long t1 = comm1.getDate().getTime();
        long t2 = comm2.getDate().getTime();
        return (int) (t2 - t1);
    }
}

The method getDate() returns a java.sql.Timestamp.

Here is the output before the sort:

for (Communication n : retVal) {
    System.out.println(n.getDate().toString());
}

2012-10-03 10:02:02.0
2012-10-07 03:02:01.0
2012-10-08 13:02:02.0
2012-10-09 03:02:00.0
2012-11-26 10:02:05.0
2012-11-28 11:28:11.0
2012-12-03 12:03:01.0
2012-12-06 15:03:01.0
2012-12-13 14:03:00.0
2012-12-28 11:03:00.0
2012-12-28 13:49:21.0

And after:

Collections.sort(retVal, new CommunicationsComparator());

2012-12-13 14:03:00.0
2012-12-06 15:03:01.0
2012-12-03 12:03:01.0
2012-11-28 11:28:11.0
2012-10-09 03:02:00.0
2012-10-08 13:02:02.0
2012-11-26 10:02:05.0
2012-10-07 03:02:01.0
2012-10-03 10:02:02.0
2012-12-28 13:49:21.0
2012-12-28 11:03:00.0

Any ideas why the bottom two objects might not be sorted correctly? I am using the MySQL JDBC implementation of this Timestamp.

Upvotes: 5

Views: 17956

Answers (5)

Preetam Kumar
Preetam Kumar

Reputation: 446

In java 8 and above we can Do this in a very clean and simple manner by:

list.stream().max(Comparator.comparing(YourCustomPojo::yourDateField))

This works with any Pojo having field type with support of compareTo(). java.sql.Timeastamp and java.util.Date comes with out of the box support for this method.

check the java doc here

Upvotes: 0

James McMinn
James McMinn

Reputation: 429

The difference between the last 2 dates and earlier dates will overflow integer.

Perhaps a better solution would be to compare the values, rather than subtract them.

    long t1 = comm1.getDate().getTime();
    long t2 = comm2.getDate().getTime();
    if(t2 > t1)
            return 1;
    else if(t1 > t2)
            return -1;
    else
            return 0;

Upvotes: 18

MrSmith42
MrSmith42

Reputation: 10151

My first idea is that the problem is an overflow. t1 and t2 are longs. The different may not fit in an int.

I would check that.

If compare on second-level is good enough for you, you should try:

return (int) ((t2 - t1)/1000);

This does not guarantee that there will be no overflows. I would at least add a test.

I think the best answer is not mine. My favorite is:

    if(t2 > t1)
        return 1;
    else if(t1 > t2)
        return -1;
    else
        return 0;

Upvotes: 4

Peter Lawrey
Peter Lawrey

Reputation: 533660

You can use

return Long.compare(t2, t1);

but you are better off comparing the dates.

return comm2.getDate().compareTo(comm1.getDate());

Upvotes: 6

gogognome
gogognome

Reputation: 759

If the difference is bigger than about 25 days, an overflow occurs. (An int cannot represent a bigger time difference in milliseconds than about 25 days). This will make the comparison incorrect.

This can be solved by changing the return statement into:

return Long.signum(t2 - t1);

Upvotes: 6

Related Questions