1

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
Improve this question
7
  • I assume you're using primitives, aren't you? Commented Dec 3, 2013 at 10:28
  • @Thomas: What do you mean? Commented Dec 3, 2013 at 10:28
  • What data type does getCapacity() return? Commented Dec 3, 2013 at 10:29
  • @Thomas: v.getCapacity() returns Double (not double) Commented Dec 3, 2013 at 10:30
  • 1
    That's the problem, I'll add an answer. Commented Dec 3, 2013 at 10:31

1 Answer 1

Reset to default
2

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).

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Would you suggest to use double instead of Double
@KlausosKlausos see my addition at the end.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.