0

I am having an issue overiding a compareTo methods. The program simulates different employee types, and I have it sorting by employee type perfectly, but can not get it to do a secondary sort by gross pay. Once it sorts by class name/employee type, it then needs to sort by grossPay, which I can obtain by a helper method. Below is the code:

  public int compareTo(Object o) {

    Employee other = (Employee) o;

    if(other instanceof Salaried)
        return -1;

    else if(other instanceof Daily)
         return 1; 

    else
        return 0;
}

I am using Collection.sort() with an arraylist of employess. When I print out I get a great sorted list by employee type, but it should then sort by grossPay.

Erik
  • 47
  • 1
  • 9
  • You may want to take a look at [http://stackoverflow.com/questions/369512/best-way-to-compare-objects-by-multiple-fields](http://stackoverflow.com/questions/369512/best-way-to-compare-objects-by-multiple-fields). It (as far as I understand it) pretty much answers your question. – Michael Jun 17 '12 at 01:56

2 Answers2

6

The compareTo must return results consistent with a total order. Otherwise the sort results are not guaranteed in any way. A total order means that if A<B, then B>A and if A==B, then B==A. In other words, you can switch this and other and the results are consistent. The code you present does not do this even for employee type.

If compareTo isn't consistent with at total order, sort can produce the wrong answer or never terminate.

It's not clear if your system has 3 types of employees or 2. Let's assume it's 2: salaried and daily. Then we need to work through the possiblities:

this     other    result
------------------------
salaried salaried equal
daily    salaried <
salaried daily    >
daily    daily    equal

Only after we've established that this and other are equal in employee type do we take the secondary sort key, which is gross pay.

So one way to code this is:

// Assume this and o have type Daily or Salaried.
public int compareTo(Object o) {
  if (this instanceof Daily && o instanceof Salaried) return -1;
  if (this instanceof Salaried && o instanceof Daily) return +1;
  // The employee types must be equal, so make decision on pay.
  Employee e = (Employee)o;
  return grossPay() < e.grossPay() ? -1 :
         grossPay() > e.grossPay() ? +1 : 0;
}

I'm assuming this is implemented in Employee.

Finally, it would probably be better to implement this sort with a Comparator. The compareTo method should be reserved for the "natural" sort order such as the numerical order of a unique id number that serves as a primary key. This sort criterion doesn't seem to be "natural".

Gene
  • 46,253
  • 4
  • 58
  • 96
3

You can compare grossPay after the comparing of type.Assuming grossPay is a number.

public int compareTo(Object o) {

    Employee other = (Employee) o;
    if(this instanceof Daily && other instanceof Salaried)
        return -1;

    else if(this instanceof Salaried && other instanceof Daily)
        return 1; 

    else
        return this.getGrossPay() - other.getGrossPay();
}
plucury
  • 1,120
  • 1
  • 8
  • 16
  • 2
    Close, but because of the possibility of overflow, you should not use integer subtraction to implement `compareTo`. You could use Integer.compareTo if `getGrossPay()` is already returning an `Integer` value, but for primitives, stick to `<` and `>`. In Java 7, there is the static method `Integer.compare(int, int)`. – Ted Hopp Jun 17 '12 at 02:09
  • Unfortunately this isn't certain to work. If `this` is Salaried and `other` is Salaried, this will return `-1`. If you swap the two operands, it will return `-1` again. So the comparison is not consistent with a total order. The sort may produce the wrong answer or never terminate. – Gene Jun 17 '12 at 02:25
  • @TedHopp Thanks for your advice. I think the value of grossPay should be positive. So it could not overflowed. – plucury Jun 17 '12 at 02:50