Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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;
        }
     });
like image 589
Richard Avatar asked Nov 27 '22 03:11

Richard


1 Answers

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
like image 57
Jon Skeet Avatar answered Nov 30 '22 22:11

Jon Skeet