I'm looking at you, Java experts
As a professional Java programmer, I often find myself looking at Java blogs, tutorials and news sites. One example that crops up again, and again, is an implementation of the Comparable<T> interface, or a comparator lambda that uses subtraction instead of comparison. Consider the following toy class:package example.bad;
public class Value implements Comparable<Value> {
private final int value;
public Value(int value) {
this.value = value;
}
@Override
public int compareTo(Value o) {
return this.value - o.value;
}
public int getValue() {
return value;
}
}
Even prominent, respected Java experts are guilty of this. It looks perfectly plausible, and it even works if you only call it with values in the range [-1,073,741,824, +1,073,741,824]. But the hallmark of a correct implementation is not plausibility, or "happy-path" success. The hallmark of a correct implementation is that it works with all valid inputs, not some valid inputs. Let's add some tests. If the code works, it just works, right?
package example.bad;
import org.junit.Test;
import static org.junit.Assert.assertTrue;
public class ValueTest {
Value max = new Value(Integer.MAX_VALUE);
Value min = new Value(Integer.MIN_VALUE);
Value zero = new Value(0);
Value one = new Value(1);
Value minusOne = new Value(-1);
@Test
public void happyTest() {
assertTrue(one.compareTo(zero) > 0);
assertTrue(zero.compareTo(one) < 0);
assertTrue(one.compareTo(one) == 0);
}
@Test
public void realTest1() {
assertTrue(max.getValue()+" < "+minusOne.getValue(), max.compareTo(minusOne) > 0);
// AssertionError: 2147483647 < -1
}
@Test
public void realTest2() {
assertTrue(min.getValue()+" > "+one.getValue(), min.compareTo(one) < 0);
// AssertionError: -2147483648 > 1
}
@Test
public void realTest3() {
assertTrue(max.getValue()+" < "+min.getValue(), max.compareTo(min) > 0);
// AssertionError: 2147483647 < -2147483648
}
@Test
public void realTest4() {
assertTrue(min.getValue()+" > "+max.getValue(), min.compareTo(max) < 0);
// AssertionError: -2147483648 > 2147483647
}
}
Looks like it just doesn't work.
And before you object that "well my inputs aren't anywhere near Integer.MAX_VALUE or Integer.MIN_VALUE", remember that bad assumptions kill. Your method might never produce a bogus result... but someone else who reads it might assume that's how you implement a comparison. They might go on to use subtraction to implement comparison in a financial program, or worse yet, an embedded pacemaker, or worse yet, an airplane flight control module, or... you think of the worst possible example you can, and it could end up there.
It's inexcusable, and it needs to stop.
What makes this "sample code" not just dangerous but also incredibly lazy is that Integer.compare(a, b), Long.compare(a, b), etc. have been in the standard library since Java 7. You will never need to implement them yourself. So stop writing sample code that does. Before you kill someone.
Finally, allow me leave you with the easy, and also completely correct, way to write an integer comparison.
@Override
public int compareTo(Value o) {
return Integer.compare(this.value, o.value);
}
Try substituting it for the bogus one and rerunning the tests!
