Java comparator issue when passed to Collections.sort

Question

When testing with less load in local it worked fine.

private static class CoordinateComparator implements Comparator<Coordinate> {

    @Override
    public int compare(Coordinate o1, Coordinate o2) {
        return o1.x <= o2.x ? -1 : 1;
    }

}

Here x is primitive and it was giving runtime error when tests were run. Under heavy load it was breaking.

Then i changed comparator to:-

private static class CoordinateComparator implements Comparator<Coordinate> {

    @Override
    public int compare(Coordinate o1, Coordinate o2) {
        return o1.x.compareTo(o2.x);
    }

}

In this case x is Integer. Then it started working fine. Any ideas or thoughts why this was happening. I was passing this comparator to Collections.sort(array, comp)

It was during a competitive coding. They just gave RE not exact error. — Ashutosh Jha
– Ashutosh Jha, Commented Mar 19, 2020 at 9:40
I tested the same problem after accounting for o1.x == o2.x return 0. And it worked fine. — Ashutosh Jha
– Ashutosh Jha, Commented Mar 19, 2020 at 9:53

Eran · Accepted Answer · 2020-03-19 09:40:56Z

6

public static <T> void sort(List<T> list, Comparator<? super T> c) would throw IllegalArgumentException if the comparator is found to violate the Comparator contract.

In your code, the first compare method is inconsistent for the case where o1.x is equal to o2.x. It will return either -1 or 1 depending on the order in which the instances are compared. It should return 0 in this case.

You can fix it as follows:

public int compare(Coordinate o1, Coordinate o2) {
    return o1.x < o2.x ? -1 : o1.x > o2.x ? 1 : 0;
}

Though your o1.x.compareTo(o2.x) alternative seems cleaner to me.

edited Mar 19, 2020 at 9:40

answered Mar 19, 2020 at 9:34

Eran

395k57 gold badges726 silver badges793 bronze badges

Sign up to request clarification or add additional context in comments.

8 Comments

Lino Over a year ago

Why not simply write o1.x - o2.x?

Eran Over a year ago

@AshutoshJha it might, since your Comparator is violating the contract.

Eran Over a year ago

@Lino if there's no danger of numeric overflow, you can certainly write that (I'm not sure what's the type of x and what range of values it may contain).

Eran Over a year ago

@Lino depends on how large and how small these integers can be. For example, if you subtract Integer.MAX_VALUE from any negative value, you'll get numeric overflow.

Holger Over a year ago

@AshutoshJha how can you say "it didn't matter to me", when you are going to pass this comparator to a sort or search routine, you didn't write, so you don't know what matters to these routines?

|

Collectives™ on Stack Overflow

Java comparator issue when passed to Collections.sort

1 Answer 1

8 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

8 Comments

Your Answer

Sign up or log in

Post as a guest

Related