inconsistent numeric comparison semantics between BigDecimal and other Numerics

Description

BigDecimal does not have consistent comparison semantics with other numeric types.

user> *clojure-version* {:major 1, :minor 5, :incremental 1, :qualifier nil} user> (== 2.0 2.0M) true user> (== 2 2.0M) false <-- this one is not like the others user> (== 2 2.0) true user> (== 2N 2.0) true user> (== 2 (double 2.0M)) true user> (== 1.0M 1.00M) false ;; potentially surprising

Patch: clj-1118-v7.patch

Approach: Change equiv for BigDecimals so that instead of using BigDecimal.equals(), it uses BigDecimal.compareTo() and checks the return value is equal to 0.

The javadoc for these methods explicitly states that BigDecimal.equals() will treat values that are otherwise equal numerically, but differ in scale, as not equal. They also say that BigDecimal.compareTo() will return 0 for such BigDecimals.

Doing this also changes the behavior of = when comparing BigDecimal values to other numbers. hash should be consistent with =, so now hash should return the same value for all numerically equal BigDecimal values. This patch modifies hasheq() to return the same value for all numerically equal BigDecimal values, by calling BigDecimal.stripTrailingZeros() on them before hashing. This change also will make 1.0M == 1.00M which was not true before.

Screened by: Alex Miller

Environment

None

Attachments

4
  • 22 Oct 2013, 02:01 PM
  • 28 Sep 2013, 01:54 AM
  • 27 Sep 2013, 09:12 PM
  • 15 Aug 2013, 01:29 AM

Activity

Show:

Alex Miller October 22, 2013 at 2:53 PM

Oh right, that is ok so can stay screened.

Andy Fingerhut October 22, 2013 at 2:37 PM

It was not only adding a comment, but removing tests that should not have been there and could potentially mislead people into thinking that Clojure guarantees hash being consistent with = for BigInteger and Float/Double, which by https://clojure.atlassian.net/browse/CLJ-1036#icft=CLJ-1036 it does not.

Regarding the .txt file extensions, first I've heard of the preference. I can make sure future screened tickets don't have this, but changing currently screened tickets would likely lead to confusion about which ones are screened. M-x diff-mode in emacs will force the mode regardless of file name.

Alex Miller October 22, 2013 at 2:01 PM

Andy's last patch just added a comment. I updated a new version that is identical to v6 but has a .patch extension instead. Marking Screened again.

Rich Hickey October 22, 2013 at 1:55 PM

Please don't use .txt as an extension for diffs, it keeps them from loading with appropriate mode in various editors.

v5 was screened but v6 alters it?

Andy Fingerhut September 28, 2013 at 1:54 AM

Patch clj-1118-v6.txt is tiny modification of Alex Miller's clj-1118-make-double-equals-true-for-more-bigdecimals-patch-v5.txt

It removes hash consistency tests for floats and BigIntegers, with a comment explaining why they should not be there.

Completed

Details

Assignee

Reporter

Labels

Approval

Ok

Patch

Code and Test

Priority

Affects versions

Fix versions

Created November 30, 2012 at 7:36 PM
Updated November 23, 2013 at 1:06 AM
Resolved November 23, 2013 at 1:06 AM