Closed Bug 1316015 Opened 3 years ago Closed 3 years ago
[findbugs] [FE] Test for floating point equality
58 bytes, text/x-review-board-request
* 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?
It looks like we already have such a method in FloatUtils: https://dxr.mozilla.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/FloatUtils.java#15-17
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+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/4ca9f9807277 Fixed by replacing '==' & '!=' float comparisons with FloatUtils.fuzzyEquals(). r=sebastian
You need to log in before you can comment on or make changes to this bug.