Inspection Gadgets suggestion for float comparison

I originally tried to email Dave Griffith directly but the email on the Community Wikki seems to no longer be valid. So instead I'll post here since I know he reads the forums:


I was recently reading up on decimal comparisons and some of the problems that can result from such (http://www-106.ibm.com/developerworks/java/library/j-jtp0114/) The InspectionGadgets plugin has an existing inspection that warns if code is using equality (==) comparison of floats/doubles, but the inspection does not offer any suggestions on how to fix the problem. It would be nice if the check could offer an intention to convert a float/double comparison into code that looks something like this:

new Float(a).compareTo(new Float(b)) == 0
new Double(a).compareTo(new Double(b)) == 0

Or in java 1.4+ this can be converted to the much simpler:

Float.compare(a, b)==0;
Double.compare(a, b)==0;


A slightly similar problem can also exists for the BigDecimal class. The equals() method will return true if both BigDecimals have the same number of decimal places but returns false if they don't even if the numbers are actually the same (for instance 0.01 would not be considered equal to 0.010). The compareTo() method does correctly compare these values so an intention that suggests changing 'BigDecimal.equals(BigDecimal)' into 'BigDecimal.compareTo(BigDecimal)==0' would be useful

In addition to the equality comparison for float and doubles, it would be nice if there was an inspection that warned of any float or double comparisons.
For instance:
'a > b' could be changed to ' new Float(a).compareTo(new Float(b)) > 0'
'a < b' could be changed to ' new Float(a).compareTo(new Float(b)) < 0'

6 comments

new Float(a).compareTo(new Float(b)) == 0
new Double(a).compareTo(new Double(b)) == 0


And what's the advantage of this approach over comparing float/double primitives? This obscures the code and you only trade sets of possible errors (NaN comparisons return true, but -0 becomes different than 0). I see no gain in this, but it's very possible I'm missing something :)

Float.compare(a, b)==0;
Double.compare(a, b)==0;


Slightly less obscure, but no advantage either.

A slightly similar problem can also exists for the BigDecimal class.


There was a request for this some time ago, and Dave has already commited the changes ... for the next version of Idea.

0

Well darn it. I now see your point about 0 and -0 and that won't do. I had found a site (that now I can't find again) that recommended using compareTo on floats. It hadn't mentioned anything about the zero problem.

So, should I just avoid comparing floats/doubles then? That's the only reason I can see for the Inspection then...

0


As noted, the .compareTo() solution you describe doesn't actually solve the problem. What I normally suggest is changing a==b to something like Math.abs(b-a)<EPSILON, where epsilon is chosen appropriately, but obviously IG can't automate that because an appropriate constant for EPSILON varies by application. Probably all that can be automated in this case is to warn that floating point equality comparisons are dodgy, and that you should be careful, which is what IG does. I'm not sure what the payoff of reporting floating-point non-equality comparisons, but it would be trivial to add if you provide a reasonable use case.

The BigDecimal .equals()->.compareTo() inspection and quickfix you suggest is already implemented (someone suggested it a while ago), and will ship with the first or second Irida EAP, whenever that is.

Finally, it makes my life easier if IG and IPP feature suggestions are done as Tracker items, rather than comments in the Plugins group. It makes them easier to, um, track.

--Dave Griffith

0

Sorry, I'm usually too busy lurking to make suggestions and wasn't aware of the Tracker.

The Wikki page (http://www.intellij.org/twiki/bin/view/Main/InspectionGadgets) mentioned nothing about submitting a Tracker item and the faq directly suggests contacting you with any ideas. I then only posted here because the email provided on the wikki bounced back.

0

So, should I just avoid comparing floats/doubles then? That's the
only reason I can see for the Inspection then...


1. Stop using floats/doubles, change to BigDecimals if you are doing money/exact calculations.

2. As Dave said, use something like Math.abs(b-a) < EPSILON if you want to compare values. That's because there's no guarantee that n/n == 1 for example - in some cases it's equal to 1.000000...1 or 0.9(9).

3. For compareTo and equals, take in account Nan's, +/-0, +/-infinity.


0


Not a problem. I'll update the wiki.

--Dave Griffith

0

Please sign in to leave a comment.