Test for floating point equality. (FE_FLOATING_POINT_EQUALITY)

Question

I am using a findbugs in an ANT script and I can't figure out how to fix two of my errors. I have read the documentation, but don't understand. Here are my errors and the code that goes with them:

Error 1: Test for floating point equality. (FE_FLOATING_POINT_EQUALITY)

private boolean equals(final Quantity other) {
    return this.mAmount == convertedAmount(other);
}

Error 2: EQ_COMPARETO_USE_OBJECT_EQUALS

public final int compareTo(final Object other) {
    return this.description().compareTo(((Decision) other).description());
}

I've read the documentation for the ComparesTo issue that states

It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."

and also the docs regarding the floating point equality

This operation compares two floating point values for equality. Because floating point calculations may involve rounding, calculated float and double values may not be accurate. For values that must be precise, such as monetary values, consider using a fixed-precision type such as BigDecimal. For values that need not be precise, consider comparing for equality within some range, for example: if ( Math.abs(x - y) < .0000001 ). See the Java Language Specification, section 4.2.4.

I don't get it though. Can anyone please help?

Answer 1

Problem 1:

For the FE_FLOATING_POINT_EQUALITY issue, you should not be comparing two float values directly with the == operator, since due to tiny rounding errors, the values might be semantically "equal" for your application even if the condition value1 == value2 does not hold true.

In order to fix this, modify your code as follows:

private boolean equals(final Quantity other) {
    return (Math.abs(this.mAmount - convertedAmount(other)) < EPSILON);
}

Where EPSILON is a constant that you should define in your code, and represents small differences that are acceptable to your application, e.g. .0000001.

Problem 2:

For the EQ_COMPARETO_USE_OBJECT_EQUALS issue: It is strongly recommended that wherever x.compareTo(y) returns zero, x.equals(y) should be true. In your code you have implemented compareTo, but you have not overriden equals, so you are inheriting the implementation of equals from Object, and the above condition is not met.

In order to fix this, override equals (and perhaps hashCode) in your class, so that when x.compareTo(y) returns 0, then x.equals(y) will return true.

Answer 2

For the floating point warning, you should bear in mind that floats are an inexact type. A standard reference oft given for this (which is worth reading once perhaps) is:

What Every Computer Scientist Should Know About Floating-Point Arithmetic by David Goldberg.

Because floats are not exact values - even if they look the same when rounded up to a few decimals - they can differ very slightly, and fail to match.

The Comparable interface expects a certain behaviour by its implementor; the warning is telling you you are not adhering to that, and offering suggested actions.

Answer 3

I do not agree with the answers above. Equals and compareTo are the wrong place to introduce epsilons in floating point comparisons.

Floating point values can be compared exactly by equals and compareTo, just using the "==" operator.
If your application, uses floats that are a result of calculation, need to compare these values with the epsilon approach, it should do that only in that place where this is needed. E.g in a mathematical line intersection method.
But not in equals and compareTo.

This warning is very misleading. It means comparing two floats where at leats one is a result of an calculation might give unexpected result. However, often such floats, to compare, are not a result of a calculation, like

static final double INVALID_VALUE = -99.0;
if (f == INVALID_VALUE)

where f is initialized with INVALID_VALUE, will in java always work perfectly. But findbugs and sonarcube will still complain.

So just add an ignore filter to findbugs, asuming you have two classes MyPoint2D and Myrectangle2D

<Match>
        <OR>
            <Class name="~.*\.MyPoint2D" />
            <Class name="~.*\.MyRectangle2D" />
        </OR>
        <Bug code="FE" />
        <Justification author="My Name" />
        <Justification
            text="Floating point equals works (here)." />
    </Match>