Closed Bug 1316015 Opened 3 years ago Closed 3 years ago

[findbugs] [FE] Test for floating point equality

Categories

(Firefox for Android :: General, defect, P3)

All
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: sebastian, Assigned: swaroop.rao, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [lang=java][good first bug])

Attachments

(1 file)

* Test for floating point equality in org.mozilla.gecko.home.activitystream.topsites.CirclePageIndicator.onDraw(Canvas)

* Test for floating point equality in org.mozilla.gecko.home.HomeBanner.handleHomeTouch(MotionEvent)

"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."
To start, set up a build environment - you can see the instructions here:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build

Then, you'll need to upload a patch - see:
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "sebastian" and you can find me and other helpful folks in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC
I would like to take this up if nobody's working on it. I have a build ready to go and I understand what changes need to be made. Based on the context / logic, I feel that we don't need to use BigDecimal mainly because a) we don't need the arbitrary precision and b) it could hinder performance. So, I'm thinking of implementing tolerance-based comparison. For this, I plan to define a static final variable for holding the tolerance. Since there are at least two cases where a floating point comparison is being done and they are in two separate classes, I would like to define this static final variable in a common class. I did a quick search for a suitable class but couldn't find one. Can you please suggest a class that I could use to define this constant in?
Flags: needinfo?(s.kaspari)
Assignee: nobody → swaroop.rao
Status: NEW → ASSIGNED
Comment on attachment 8811806 [details]
Bug 1316015 - Fixed by replacing '==' & '!=' float comparisons with FloatUtils.fuzzyEquals().

https://reviewboard.mozilla.org/r/93762/#review94078
Attachment #8811806 - Flags: review?(s.kaspari) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4ca9f9807277
Fixed by replacing '==' & '!=' float comparisons with FloatUtils.fuzzyEquals(). r=sebastian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4ca9f9807277
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.