Search code examples
javasortingcomparable

Array sorting issue: 2 different sorting results


I need to sort a list based on three parameters. This is how I do this.

SortedVehicles = new ArrayList<MyVehicle>(vehicles);
        Collections.sort(SortedVehicles,Collections.reverseOrder());
@Override
public int compareTo(ITSVehicle v) {
    if (this.getCapacity(0) < v.getCapacity(0)) return 1;
    if (this.getCapacity(0) > v.getCapacity(0)) return -1;
    if (this.getCapacity(0) == v.getCapacity(0))
    {
        if (this.getCapacity(1) < v.getCapacity(1)) return 1;
        if (this.getCapacity(1) > v.getCapacity(1)) return -1;
    }
    if (this.getCapacity(1) == v.getCapacity(1))
    {
        if (this.getCapacity(2) < v.getCapacity(2)) return 1;
        if (this.getCapacity(2) > v.getCapacity(2)) return -1;
    }
    return 0;
}

The problem is that I get 2 different sorting results:

Result 1
4490    5.77    306
4490    5.77    300

Result 2
4490    5.77    300
4490    5.77    306

Solution

  • The problem with your approach is most likely that you're comparing Doubles with ==. In most cases new Double(x) == new Double(x) is false since == tests for object equalitiy. You'd have to use equals() here (or unbox yourself).

    Just a small hint: you might want to use the compareTo methods of the primitive wrappers, e.g. like this:

    public int compareTo(ITSVehicle v) {
      result = this.getCapacity(0).compareTo(v.getCapacity(0));
      if( result == 0 ) {
        result = this.getCapacity(1).compareTo(v.getCapacity(1));
      }
      if( result == 0 ) {
        result = this.getCapacity(2).compareTo(v.getCapacity(2));
      }
    
      return result;
    }
    

    Just note that this would break if any capacity was null, but that would be the case with your approach as well (and the exception there would be harder to track since the auto(un)boxing would throw the NPE).

    If they can't be null you might want to consider using primitives instead. Your data already implies that the capacities have different meanings and thus from a design point of view it might be good not to store them in a list etc. (which you do, I assume, and which thus would require you to use Double instead of double).