3

I am in the process of upgrading our project to java 7. I ran into the Illegal Argument exception for Collections.sort(). I know the cause of the exception is the new Timsort in java 7 (I did go throw though all the questions raised previously on this issue). Now I need to modify the compare logic to overcome the exception. Here is my compare method

if (o1.isLookup() && !o2.isLookup()) {
    return -1;
}
if (!o1.isLookup() && o2.isLookup()) {
    return 1;
}

if (o1.dependsOn(o2)) {
    return 1;
}
if (o2.dependsOn(o1)) {
    return -1;
}
return 0;
  1. I tried overriding the equals() method , with the same logic as compare, thinking if equals and compare return the same result it should resolve the issue; but it didn't work as expected .

  2. When I split the compare method into two with separate comparators as shown below then the sort(using both comparators) does not throw any exception. What could be the possible reason?

Code Below:

protected Comparator<EntityWrapper> getComparator2() {      
    return new Comparator<EntityWrapper>() {
        public int compare(EntityWrapper o1, EntityWrapper o2) {
            if (o1.dependsOn(o2.entityClass)) {
                // This depends on otherWrapper
                return 1;
            }
            if (o2.dependsOn(o1.entityClass)) {
                // OtherWrapper depends on this
                return -1;
            }
            return 0;
        }
    };
}

protected Comparator<EntityWrapper> getComparator1() {
    return new Comparator<EntityWrapper>() {
public int compare(EntityWrapper o1, EntityWrapper o2) {
        if (o1.isLookup() && !o2.isLookup()) {
            return -1;
        }
        if (!o1.isLookup() && o2.isLookup()) {
            return 1;
        }
        return 0;
    };
}
IS_EV
  • 988
  • 2
  • 15
  • 29
  • 2
    is it possible that `o1.dependsOn(o2)` and `o2.dependsOn(o1)` are both true? – Henry Jan 09 '14 at 20:17
  • 1
    It is hard to comment unless we see what `dependsOn` and `isLookup` method do here. – Rohit Jain Jan 09 '14 at 20:19
  • Can't speak for your specific example, but make sure your comparison function respects the concept of Strict Weak Ordering. That is, if a < b, then b should be > a and stuff like that. – Cubicle Dragon Jan 09 '14 at 20:20
  • 4
    @Henry probably nailed it for your specific case. If o1 and o2 both depend on each other, your comparator will return that o1 is greater than o2 if called with (o1, o2) but will return that o2 is greater if called with (o2, o1), which totally confuses the sorting algorithm. – Cubicle Dragon Jan 09 '14 at 20:22
  • @Henry: IF that was the case then when I did a sort based on getComparator2(), the sort should have thrown the exception; but it didn't. This is what is adding to my confusion. – IS_EV Jan 09 '14 at 20:34
  • 1
    Post [**SSCCE**](http://sscce.org) that clearly shows the problem. – PM 77-1 Jan 09 '14 at 20:44
  • @paulb: What are all possible `EntityWrapper` values? – Karol S Jan 09 '14 at 22:39
  • Second example is missing the `public int compare(...) {` line, and the matching `}`. – keshlam Jan 09 '14 at 23:26
  • @keshlan : Typo.. this is corrected..thanks – IS_EV Jan 09 '14 at 23:43
  • @KarolS : Its a class which holds reference to other classes : private Class> entityClass; – IS_EV Jan 09 '14 at 23:45
  • I believe I have figured out the problem ; there is one scenario where o1.dependsOn(o2) and o2.dependsOn(o1) is returning true .... I am thinking of adding a condition to the comparator where in if o1.dependsOn(o2) and o2.dependsOn(o1) is returning true , then return 0. – IS_EV Jan 09 '14 at 23:48
  • @paulb: is there a chance that none of the classes depend on each other *and* either none or both are lookup? – Karol S Jan 11 '14 at 20:05
  • @KarolS yes and yes.. when none of the classes depend on each other then we treat them as equal.. the last condition for dependency is checked if only both classes are lookup or non lookup – IS_EV Jan 14 '14 at 00:53
  • possible duplicate of [Comparison method violates its general contract! Java 7 only](http://stackoverflow.com/questions/7849539/comparison-method-violates-its-general-contract-java-7-only) – eltabo Jan 15 '14 at 08:37
  • @eltabo: yes , I knew for a fact that my comparison method was violating the comparable contract from the exception that was getting thrown, but I had questions on the way my code was behaving under different scenarios and also thought some one could throw some light on my specific scenario from their experience.. – IS_EV Jan 15 '14 at 18:47

1 Answers1

0

Aren't you missing ".entityClass" such as:

if (o1.isLookup() && !o2.isLookup()) {
    return -1;
}
if (!o1.isLookup() && o2.isLookup()) {
    return 1;
}

if (o1.dependsOn(o2.entityClass)) {
    return 1;
}
if (o2.dependsOn(o1.entityClass)) {
    return -1;
}
return 0;
xchiltonx
  • 1,946
  • 3
  • 20
  • 18