Collections sort with custom comparator does not work

Question

I have created a custom Comparator to sort an ArrayList of Strings. I have run it through the debugger and watched it comparing and returning values correctly. However, my array is not sorted. Since I am new to Java & Android, there might be something else going on.

After looking at it for a few hours, I can't figure out what .. and since I have been using this site to answer so many other questions, I knew where to come to !

    Collections.sort(allWords, new Comparator<String>(){
        public int compare(String o1, String o2) {
            scoreWord sc1 = new scoreWord((String)o1);
            scoreWord sc2 = new scoreWord((String)o2);
            int i1 = sc1.getScore();
            int i2 = sc2.getScore(); 
            if ( i1 > i2 )
                return 1;
            return 0;
        }

        public boolean equals(String o1, String o2) {
            scoreWord sc1 = new scoreWord((String)o1);
            scoreWord sc2 = new scoreWord((String)o2);
            int i1 = sc1.getScore();
            int i2 = sc2.getScore(); 
            if ( i1 == i2 )
                return true;
            return false;
        }
     });

Solution 1

Your compare method isn't symmetric - it always either returns 1 or 0.

Instead, just delegate to Integer.compare (if it's available in the version of Java you're using), passing in the scores:

public int compare(String o1, String o2) {
    scoreWord sc1 = new scoreWord((String)o1);
    scoreWord sc2 = new scoreWord((String)o2);
    return Integer.compare(i1, i2);
}

Otherwise do it by hand, which is frankly a pain - if you need this in more than one place, I suggest you write your own implementation of Integer.compare to avoid the repetition:

public int compare(String o1, String o2) {
    scoreWord sc1 = new scoreWord((String)o1);
    scoreWord sc2 = new scoreWord((String)o2);
    return i1 > i2 ? 1
         : i1 < i2 ? -1
         : 0;
}

This way you'll have appropriate symmetry:

• a.compareTo(b) < 0 implies b.compareTo(a) > 0
• a.compareTo(b) > 0 implies b.compareTo(a) < 0
• a.compareTo(b) == 0 implies b.compareTo(a) == 0

Solution 2

Try this:

   public int compare(String o1, String o2) {
        scoreWord sc1 = new scoreWord((String)o1);
        scoreWord sc2 = new scoreWord((String)o2);
        int i1 = sc1.getScore();
        int i2 = sc2.getScore(); 
        if ( i1 > i2 ) {
            return 1;
        } else if ( i1 < i2 ) {
            return -1;
        } else {
            return 0;
        }
    }

Solution 3

Your comparator should return something akin to:

Return: a negative integer, zero, or a positive integer as the first argument is less than, equal to, or greater than the second.

Source

Comments

[home]

For what reason do you need the equals(String o1, String o2) method?

[Eng.Fouad]

try compare(Object o1, Object o2) and equals(Object o1, Object o2)

[Richard]

Thanks. This is what university coding gets you... an assumption that sort algorithms require T/F in the compare :(

[home]

@user882173: pls give me a hint, what you mean by "T/F"?

[john ktejik]

can you explain or provide a link to this symmetric property?

[Jon Skeet]

It's in the documentation for Comparator.compare (It's expressed in terms of sign, but that's the same thing.)

[Ravi]

@user882173 true or false