4

I was working on Comparable interface and found that it is throwing IllegalArgumentException when i put size=50 in the program below and is working fine when i put size =5.

    public class Test {
    public static void main(String[] args) {
        int size = 50;
        Test compareTest = new Test();
        compareTest.test(size);
    }

    public void test(int size) {
        List<TestObject> requests = new ArrayList<TestObject>();
        for (int index = 0; index < size; index++) {
            TestObject request = new TestObject();
            request.value = index;
            requests.add(request);
        }
        Collections.sort(requests);
    }

}

class TestObject implements Comparable<TestObject> {
    public int value;

    public int compareTo(TestObject req) {
        if (value % 3 == 0) {
            return -1;
        } else if (value % 3 == 1) {
            return 0;
        }

        return 1;
    }
}

I am not sure about the root cause of this problem, can some one help me on this.

Exception stack trace is given below.

Exception in thread "main" java.lang.IllegalArgumentException: Comparison method violates its general contract!
    at java.util.ComparableTimSort.mergeLo(ComparableTimSort.java:744)
    at java.util.ComparableTimSort.mergeAt(ComparableTimSort.java:481)
    at java.util.ComparableTimSort.mergeCollapse(ComparableTimSort.java:406)
    at java.util.ComparableTimSort.sort(ComparableTimSort.java:213)
T-Bag
  • 10,916
  • 3
  • 54
  • 118
  • 1
    I think there's another question that needs to be asked. You say you're "working on" a `Comparable` interface, but a `Comparable` is supposed to compare two objects and you wrote one that doesn't even look at one of the objects. The question is, how were you trying to sort your list, what order did you want the list to come out in, and how do you write a valid comparator to achieve what you want? – ajb Jun 02 '17 at 13:16

2 Answers2

6

You violate the Comparable contract.

Actually you don't compare two objects between them but you compare only the value field of the current TestObject object according to the result of modulo of 3. You don't use the TestObject object passed as parameter in the compareTo() method.

Suppose you have a List of two TestObject objects with 3 as value field

The two objects will return -1 :

    if (value % 3 == 0) {
        return -1;
    } 

But according to the rule :

The implementor must ensure sgn(x.compareTo(y)) == -sgn(y.compareTo(x)) for all x and y. (This implies that x.compareTo(y) must throw an exception iff y.compareTo(x) throws an exception.)

Assuming the first object is x and the second object is y.
If y.compareTo(x) returns a negative number (such as -1), so x.compareTo(y)should return a positive number (such as 1).

I was working on Comparable interface and found that it is throwing IllegalArgumentException when i put size=50 in the program below and is working fine when i put size =5

In fact when you violate the Comparable contract, the results are not predictable. It may work with some specific values, don't work with other specific values.
It may work in a specific JVM version and don't work in another one.
Trying to understand why it fails or it successes for a specific value may be interesting but it is really not helpful.
Trying to understand the contract in order to respect it is much better. Because today it works is in way but tomorrow in a future implementation, it could change. Only the API is a guarantee.

davidxxx
  • 125,838
  • 23
  • 214
  • 215
  • Technically, it doesn't have to return 1; it can return any positive number, because only the `sgn` is checked. – ajb Jun 02 '17 at 05:58
  • 3
    @ShowStopper Because you got lucky. Timsort doesn't try to verify that your comparison method obeys the contract in all cases. It only notices it when it has a reason to. Most sort algorithms don't even check; they just behave badly. In any case, I don't know what you mean by "working fine", except for not throwing an exception; your `compareTo` doesn't define an order since it doesn't compare two objects, so I don't know what order you would expect the sorted array to look like. – ajb Jun 02 '17 at 06:01
  • @ajb--this is what i want to know, could you please explain the same in your answer – T-Bag Jun 02 '17 at 06:03
  • @ShowStopper The same what? I don't know what you want explained. – ajb Jun 02 '17 at 06:04
  • 1
    If `y.compareTo(x)` returns -1, `x.compareTo(y)` can return 100 and still obey the contract. It doesn't have to return 1 as you said in your post. (Although in practice, it usually would.) – ajb Jun 02 '17 at 06:05
  • @Show Stopper In fact when you violate the compare contract, the results are not predictable. It may work in this case, don't work in other cases. It may work in a specific JVM and not work in another one. Trying to understand why it fails for a specific case is not helpful. Trying to understand the contract in order to respect it is much better. – davidxxx Jun 02 '17 at 06:06
  • @ajb It is true conceptually. I have just used the values of the OP in its `compareTo()`. I will modify to be more general. Thank you – davidxxx Jun 02 '17 at 06:08
2

This error comes up because compareTo is not a legal comparison function.

compareTo must obey the mathematical rules for equivalence (equality) and ordering:

(1) x.compareTo(x) must return 0.

(2) If x.compareTo(y) returns 0, y.compareTo(x) returns 0; and if x.compareTo(y) returns >0, y.compareTo(x) returns <0 and vice versa.

(3) If x.compareTo(y) and y.compareTo(z) both return >0, then x.compareTo(z) also returns >0; similarly if they are both 0, or if they are both <0 (transitivity). The above is simplified a bit.

Your comparison function doesn't even look at its argument; compareTo should be comparing one object to another, but your function ignores the second argument completely. This makes it clear that your comparison function will violate all three of the conditions.

Timsort sometimes notices when the contract is violated; if it's already looked at how compareTo works on certain values, it may deduce that it should behave a certain way on values it's seen before. If compareTo returns something that it doesn't expect, it throws an exception. Most sort algorithms don't look for this kind of violation. But if they're given a comparison function that doesn't obey the rules, the result could be a rather random order, or it could throw the algorithm into an infinite loop.

ajb
  • 31,309
  • 3
  • 58
  • 84
  • It is working fine only for size < 47 but size >= 47 not working. I also don't find out the reason. – Ravindra Kumar Jun 02 '17 at 08:19
  • @Ravindra I already explained the reason in my comments above, and so did davidxxx. When you don't have a correct comparator, the behavior is unpredictable. It might throw an exception and it might not. Please don't waste time trying to find a "reason". The fact is that the code uses a `compareTo` that doesn't compare two elements, and that means there's no way to tell what the algorithm will do. – ajb Jun 02 '17 at 13:13