How do you implement compareTo methods cleanly?

Question

Currently I am writing a compareTo method for quadratic functions in the form: ax^2 + bx + c.

a, b, c are integer coefficients that are passed to the class through the constructor.

In the compareTo method, I am supposed to first compare the a-coefficients between two functions, but if they are equal, I compare the b-coefficients. If the b's are equal, I compare the c's.

The method that I came up for this ended up being pretty ugly:

public int compareTo(QuadraticFunction other)
{
    if (a > other.a)
        return 1;
    else if (a < other.a)
        return -1;
    else if (b > other.b)
        return 1;
    else if (b < other.b)
        return -1;
    else if (c > other.c)
        return 1;
    else if (c < other.c)
        return -1;
    else
        return 0;
}

So I was wondering, if you have these "tiered" systems of comparisons (like compare a's before b's before c's), what's the best way to implement them? I can't imagine writing a method like mine if you have to go through 10+ variables.

Oliver Charlesworth · Accepted Answer · 2012-01-15T23:39:41.853

6

For an arbitrary number of coefficients (all of the same type), you should store them in a List (or something similar), rather than individually-named member variables. That allows you to convert your example code into an iteration.

edited Jan 15 '12 at 23:39

answered Jan 15 '12 at 19:14

Oliver Charlesworth

267,707
33
569
680

@LouisWasserman: That's correct. But I think it's fairly safe to assume that one would represent polynomial coefficients with homogeneous types. – Oliver Charlesworth Jan 15 '12 at 23:34
Agreed, but the caveat should be mentioned for future reference. – Louis Wasserman Jan 15 '12 at 23:38

score 2 · Answer 2 · answered Jan 15 '12 at 23:31

2

The Guava Libraries provide an extremely nice tool to do this called ComparisonChain.

Your code would look something like this:

import com.google.common.base.ComparisonChain;
...
public int compareTo(QuadraticFunction other) {
  return ComparisonChain.start()
    .compare(a, other.a)
    .compare(b, other.b)
    .compare(c, other.c)
    .result();
}

answered Jan 15 '12 at 23:31

Louis Wasserman

191,574
25
345
413

Would new Java8 `Comparator` API warrant its own answer, or should we maybe merge it there? – M. Prokhorov Aug 21 '17 at 12:32

Bohemian · Answer 3 · 2012-01-15T22:56:29.003

1

For readability, and to use the built-in compare methods for a, b, c, I would refactor to this:

public int compareTo(QuadraticFunction other) {
    if (a.equals(other.a)) {
        if (b.equals(other.b))
            return c.compareTo(other.c);
        return b.comapreTo(other.b);
    }
    return a.compareTo(other.a);
}

This code assumes the fields are Number. If they are a primitive, either convert them to wrapped type or change a.equals(b) toa == band changea.compareTo(b)toa - b`.

Also note that when an if returns, there is never a need for an else - it's redundant, so remove it.

edited Jan 15 '12 at 22:56

answered Jan 15 '12 at 19:24

Bohemian

412,405
93
575
722

One could argue that retaining the `else` facilitates refactoring. In the OP's case, (s)he might later decide to change the code to `if (cond1) { retval = 1; } else if (cond2) { retval = -1; } else ... return retval;`. – Oliver Charlesworth Jan 15 '12 at 19:42

Mike Samuel · Answer 4 · 2012-01-16T02:02:58.120

0

You can use an idiom like the below which breaks the comparison into clear sections by field, only requires one test per field, and uses the signum method to produce the return values.

Note, the subtraction below works for int, short, char, or byte fields. For long, float, and double fields you have to use separate checks for < and == to avoid overflow/underflow, and loss of precision due to rounding. Be careful of NaN too when comparing floating point values. For Comparable fields, you can just set delta to the result of compareTo after using separate conditions to handle null.

long delta = ((long) a.field1) - b.field1;
if (delta != 0) { return Long.signum(delta); }

delta = ((long) a.field2) - b.field2;
if (delta != 0) { return Long.signum(delta); }

...

return 0;

edited Jan 16 '12 at 02:02

answered Jan 15 '12 at 19:18

Mike Samuel

118,113
30
216
245

That will only work correctly if `a.field1` and co are at most integers. – Voo Jan 15 '12 at 23:48
@Voo, true. The OP says "a, b, c are integer coefficients". – Mike Samuel Jan 16 '12 at 01:34
Ah while reading the question I interpreted "integer coefficient" in the mathematical sense of the word, but yes it's probably more likely he really meant integer int. Still I think a big fat warning is important when using this idiom is always in place: It's been misused for decades so I'm a bit wary with it. – Voo Jan 16 '12 at 01:38
@Voo, Fair enough, added a caveat. – Mike Samuel Jan 16 '12 at 02:03

How do you implement compareTo methods cleanly?

4 Answers4