Another "Comparison method violates its general contract!"

Question

I'm being asked to investigate a bug as part of my internship. A piece of code is throwing

java.lang.IllegalArgumentException: Comparison method violates its general contract!

A custom Comparator is comparing two custom classes by looking at long member variables of said custom class:

return v1 > v2 ? -1 : v1 < v2 ? 1 : 0;

The equals method for this custom class looks at a String member variable of this custom class. We're having a hell of a time reproducing the behavior. My knee-jerk reaction was to replace the return statement in the custom Comparator with return v2.compareTo(v1);, but my team is skeptical that this will address the problem. Can anyone offer any insight?

Arrays.sort(anArray, new Comparator<ACustomClass>() {
  @Override
  public int compare(ACustomClass o1, ACustomClass o2) {
    long v1 = o1.getALong();
    long v2 = o2.getALong();
    return v1 > v2 ? -1 : v1 < v2 ? 1 : 0;
  }});

Answer 1

I cannot see anything obviously wrong with the comparator as presented. (And I am sceptical about the proposed fixes: they "smell" of voodoo programming to me.)

But if the ACustomClass class's aLong attribute was mutable ... and it changed while you were sorting ... then this could cause the sort code to think that the comparator was violating the contract.

So ... check to see if this could be a concurrency issue, where one thread is mutating the objects in an array that another thread is trying to sort.

---

We spent quite a bit of time on this w/ the debugger...lots of different test cases. Couldn't get the behavior to be reproduced.

I would treat that as evidence that points to a concurrency issue ...

Answer 2

I think you're barking up the wrong tree here. That piece of code does not throw that exception. I would want to see a stack trace, and I would also be looking at the ACustomClass.equals() method. Unless it tests the getAlong() results for equality, and nothing else, it doesn't agree with this Comparator, and one of them is therefore wrong, if used in a context where consistency with equals is required, such as a sorted collection, which is more probably where the exception is arising from.

You can address the scepticism experimentally. Unless they can come up with an actual formal reason why it can't work, you are certainly entitled to try it.