tags:

views:

40

answers:

2

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?

+1  A: 

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.

martin clayton
+1  A: 

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.

Grodriguez