CODEWITHSUNDEEP
javacollectionscomparator
Mohan
Mohan

Reputation: 155

Comparison method throws general contract exception

Below is a block of code that results in exception as indicated,

Code :

            Collections.sort( arrayList, new Comparator()
            {
                public int compare( Object o1, Object o2 )
                {
                TypeAdapterSort tas1 = ( TypeAdapterSort ) o1;
                TypeAdapterSort tas2 = ( TypeAdapterSort ) o2;
                if ( tas1.order < tas2.order )
                    return -1;
                else
                    return 1;
                }
            } );

Exception :

java.lang.IllegalArgumentException: Comparison method violates its general contract!
                    at java.util.TimSort.mergeLo(TimSort.java:747)
                    at java.util.TimSort.mergeAt(TimSort.java:483)
                    at java.util.TimSort.mergeForceCollapse(TimSort.java:426)
                    at java.util.TimSort.sort(TimSort.java:223)
                    at java.util.TimSort.sort(TimSort.java:173)
                    at java.util.Arrays.sort(Arrays.java:659)
                    at java.util.Collections.sort(Collections.java:217)

When I run the same code as a standalone program, the issue never occurs. What is the issue with the comparator here? Is there a way to reproduce the issue in a standalone code?

This issue occurs only on Java 1.7 as there has been change in the implementation on Arrays.sort & Collections.sort. How to change the above code to avoid the issue?. Also, how to reproduce this issue in a standalone code?

Upvotes: 2

Views: 263

Answers (2)

Jakub Zaverka
Jakub Zaverka

Reputation: 8874

You don't need to return just -1 or 1. The general contract states that the returned number must be negative if the first element is smaller, zero if they are equal, and positive if the second element is smaller. So you can skip the if/else statement and just return:

return tas1.order - tas2.order;

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1502036

The comparator doesn't obey the general contract. Look at this:

if ( tas1.order < tas2.order )
    return -1;
else
    return 1;
}

Now consider two objects which have the same order. Comparing them should return 0.

You'd be better off just delegating to Integer.compare if you're using Java 7:

return Integer.compare(tas1.order, tas2.order);

Or if you're using an earlier version:

return tas1.order == tas2.order ? 0
     : tas1.order < tas2.order ? -1
     : 1;

Upvotes: 4

Related Questions