clojure/clojure — pull request #21 by andrea-chiavazza (2012-04-15 18:26:30+00:00)
Title: fixed == to behave correctly with BigDecimal values and added tests for

The behaviour of == is not correct.

The problem is in this method in Numbers.java:
public boolean equiv(Number x, Number y){
return toBigDecimal(x).equals(toBigDecimal(y));
}

The behaviour we currently have is:
user=> (let [ones [1 1.0 1N 1M 1.0M 1.00M] ] (doseq [a ones b ones] (println a "==" b \tab (== a b) )))
1 == 1	true
1 == 1.0	true
1 == 1N	true
1 == 1M	true
1 == 1.0M	false
1 == 1.00M	false
1.0 == 1	true
1.0 == 1.0	true
1.0 == 1N	true
1.0 == 1M	true
1.0 == 1.0M	true
1.0 == 1.00M	true
1N == 1	true
1N == 1.0	true
1N == 1N	true
1N == 1M	true
1N == 1.0M	false
1N == 1.00M	false
1M == 1	true
1M == 1.0	true
1M == 1N	true
1M == 1M	true
1M == 1.0M	false
1M == 1.00M	false
1.0M == 1	false
1.0M == 1.0	true
1.0M == 1N	false
1.0M == 1M	false
1.0M == 1.0M	true
1.0M == 1.00M	false
1.00M == 1	false
1.00M == 1.0	true
1.00M == 1N	false
1.00M == 1M	false
1.00M == 1.0M	false
1.00M == 1.00M true

I propose we change the method to be:
public boolean equiv(Number x, Number y){
return toBigDecimal(x).compareTo(toBigDecimal(y)) == 0;
}
This makes the previous expression return all true, and == should also be transitive.

In particular, (== 1.0M 1.00M) will be true rather than false, which is much more in the spirit of ==.
The difference between 1.0M and 1.00M should be checked by calling the scale() method, which makes sense because it is quite an unusual thing to do.

I also noticed in test_clojure/numbers.clj the lines:
; TODO:
; ==
; and more...

So I thought of also adding the test:

(deftest equality
(are [x y] (== x y)
42 42
42 42.0
42 42N
42 42M
42 42.0M
42 42.00M
42.0 42
42.0 42.0
42.0 42N
42.0 42M
42.0 42.0M
42.0 42.00M
42N 42
42N 42.0
42N 42N
42N 42M
42N 42.0M
42N 42.00M
42M 42
42M 42.0
42M 42N
42M 42M
42M 42.0M
42M 42.00M
42.0M 42
42.0M 42.0
42.0M 42N
42.0M 42M
42.0M 42.0M
42.0M 42.00M

1.23 1.23
1.23 1.23M
1.23M 1.23
1.23M 1.23M )

(are [x y] (not (== x y))
12 12.1
1.23 123
34 3.4
1.23 1.234
123N 345N
123 345N
123N 345
12.34M 456N
12.34M 4.56
12.34 4.56M
12 4.56M
12M 4.56
12.34M 1.234M ))

I think it would be really nice to get this fixed before the 1.4 release.
Can anybody defend the current behaviour against the one I propose ?