2

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?

Mohan
  • 155
  • 1
  • 4
  • 10
  • possible duplicate. http://stackoverflow.com/questions/6626437/why-does-my-compare-method-throw-exception-comparison-method-violates-its-gen – Sajith Silva Apr 18 '13 at 06:21
  • possible duplicate of [compare method violates its general contract exception java 7](http://stackoverflow.com/questions/15897892/compare-method-violates-its-general-contract-exception-java-7) – Duncan Jones Apr 19 '13 at 07:50
  • Stop asking the same question multiple times. – Duncan Jones Apr 19 '13 at 07:50

2 Answers2

4

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;
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Thanks Jon. But, why doesn't the code block throw the exception when run in standalone program. – Mohan Apr 18 '13 at 06:22
  • @Mohan: Well your comparison code itself isn't going to check that it's obeying the contract for `compare` - it's only because the sorting code *notices* that you've given it bogus results that you're getting an exception. – Jon Skeet Apr 18 '13 at 06:25
  • Thanks Jon. In the code above if the arraylist has TypeAdapterSort objects with order values 5, 3 and 1 as an example and the Collections.sort method is invoked, exception is not thrown. why is this behavior? – Mohan Apr 18 '13 at 06:41
  • Even when the arraylist has objects with same order value, exception is not thrown when the code block is run in standalone program. I use jre 1.7.07. – Mohan Apr 18 '13 at 06:53
1

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;
Jakub Zaverka
  • 8,816
  • 3
  • 32
  • 48