Search code examples
javacomparable

Abnormal phenomenon for Comparable<T> in Java


When I write a RankedObject class which implements Comparable<RankedObject>:

public static class RankedObj implements Comparable<RankedObj>      {
    int i;
    public RankedObj(int i)     {
        this.i=i;
    }

    @Override
    public String toString() {
        return "RankedObj [i=" + i + "]";
    }


    @Override
    public int compareTo(RankedObj o) {
        if(this.i == 3)     {  // '3' will always be the smallest item
            return -1;
    }
    return this.i-o.i;
}

I want to make the list of RankedObject "in ascending order on the whole, at the same time, the number 3 to always be the smallest one". When testing, I found the order of the original list really affects the result, and the result is not correct in some cases. Namely:

//----CASE 1----
List<RankedObj> list = new ArrayList<>();
list.add(new RankedObj(11));
list.add(new RankedObj(1));
list.add(new RankedObj(12));
list.add(new RankedObj(3));
list.add(new RankedObj(8));
Collections.sort(list);
System.out.println(list);

//output (as I intend)
[RankedObj [i=3], RankedObj [i=1], RankedObj [i=8], RankedObj [i=11], RankedObj [i=12]]

//----CASE 2----
List<RankedObj> list = new ArrayList<>();
list.add(new RankedObj(11));
list.add(new RankedObj(12));
list.add(new RankedObj(3));
list.add(new RankedObj(8));
list.add(new RankedObj(1));
Collections.sort(list);
System.out.println(list);

//output (not what my Comparable intends)
[RankedObj [i=1], RankedObj [i=3], RankedObj [i=8], RankedObj [i=11], RankedObj [i=12]]

Could anyone tell my why? as well as how can I realize the purpose of making the list in ascending order on the whole while '3' being the smallest item? Thanks a lot!

P.S. According to my test using a Comparator in which the code is the same as the code in Comparable, the result is the same:

Collections.sort(list, new Comparator<RankedObj>() {
    @Override
    public int compare(RankedObj o1, RankedObj o2) {
        if(o1.i == 3)       {
            return -1;
    }
    return o1.i-o2.i;
    }
});

Solution

  • Your comparison isn't symmetric.

    Consider these two calls:

    RankedObj x = new RankedObj(1);
    RankedObj y = new RankedObj(3);
    System.out.println(x.compareTo(y)); // -2
    System.out.println(y.compareTo(x)); // -1
    

    They should both give an answer of 0, or give answers with opposite signs. Instead, they'll both return a negative value - because in the first call, o1.i is 1 (not 3) so it will fall back to o1.i - o2.i - which is still negative.

    Additionally, you have a problem that if you have a very strongly negative value, when you subtract a strongly positive value from it, the subtraction will overflow.

    You need to fix it to be symmetric and to avoid overflow. This should do it:

    @Override
    public int compareTo(RankedObj o) {
        // Handle equality first
        if (this.i == o.i) {
            return 0;
        }
        // Handle the magic value correctly on *both* sides
        if(this.i == 3) {
            return -1;
        }
        if (o.i == 3) {
            return 1;
        }
        // Fall back to normal comparsions
        return Integer.compare(i, o.i);
    }