CODEWITHSUNDEEP
javasortingcollections
Priyansu Singh
Priyansu Singh

Reputation: 139

Cannot compare user-defined objects for sorting (Java 8)

I have following classes- 

public class Pair
{
    public double value;
    int id;

    Pair(){
        value=0;
        id=0;
    }
    Pair(double value, int id){
        this.value=value;
        this.id=id;
    }
}

PairComparator class to be used for implementing Comparator class

public class PairComparator implements Comparator<Pair>{
    @Override
    public int compare(Pair p1, Pair p2){
        return (int)(p2.value-p1.value);
    }
}

I'm trying to sort a Pair collection as follows- 

List<Pair> list= new ArrayList<Pair>();
list.add(new Pair(5.2, 4));
list.add(new Pair(3.4, 5));
list.add(new Pair(10.3, 3));

Collections.sort(list, new PairComparator());

However I'm getting following error-

`Exception in thread "main" java.lang.IllegalArgumentException: Comparison method violates its general contract!

Exception in thread "main" java.lang.IllegalArgumentException: Comparison method violates its general contract!
at java.util.TimSort.mergeHi(TimSort.java:895)
at java.util.TimSort.mergeAt(TimSort.java:512)
at java.util.TimSort.mergeCollapse(TimSort.java:435)
at java.util.TimSort.sort(TimSort.java:241)
at java.util.Arrays.sort(Arrays.java:1512)
at java.util.ArrayList.sort(ArrayList.java:1454)
at java.util.Collections.sort(Collections.java:175)
at com.research.priyanshuid.recommendation_system.MatrixRecalculation.main(MatrixRecalculation.java:59)

Upvotes: 1

Views: 701

Answers (3)

Tagir Valeev
Tagir Valeev

Reputation: 100249

In Java-8 you can do this safely and without additional PairComparator class:

Collections.sort(list, Comparator.comparingDouble(p -> p.value));

Upvotes: 4

Vinay Rao
Vinay Rao

Reputation: 1284

You usually get this error when the comparator violates the transitivity - check this answer.

When using double in a Comparator, you should avoid using the substraction logic. Since you're converting the result into an int, it would be rounded and thus would break transitivity while comparing values like 5.1, 5.2 etc. Check this answer for details.

You're better off using a comparator logic as so:

public int compare(Pair p1, Pair p2) {
   if (p1.value < p2.value) return -1;
   if (p1.value > p2.value) return 1;
   return 0;
}

As mentioned by sarilks - if you're expecting NaN values better use in-built compare function - 

public int compare(Pair p1, Pair p2){
    return Double.compare(p1.value, p2.value);
}

Upvotes: 5

Saril Sudhakaran
Saril Sudhakaran

Reputation: 1129

Since you are comparing Double values , its safer to use Double.compare

 public int compare(Pair p1, Pair p2){
    return Double.compare(p1.value, p2.value);
}

Upvotes: 4

Related Questions