CODEWITHSUNDEEP
javaandroidsortingcollectionscomparator
Richard
Richard

Reputation: 968

Collections sort with custom comparator does not work

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;
        }
     });

Upvotes: 5

Views: 25040

Answers (3)

Jon Skeet
Jon Skeet

Reputation: 1502076

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

Upvotes: 20

Austin Hanson
Austin Hanson

Reputation: 22040

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

Upvotes: 2

home
home

Reputation: 12538

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;
        }
    }

Upvotes: 12

Related Questions