Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

fixed == to behave correctly with BigDecimal values and added tests for #21

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
1 participant

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] %28println a "==" b tab %28== 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 %28== x y%29)
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 ?

ignore these, they are just whitespaces

@Tirael90 Tirael90 referenced this pull request in clojure-android/clojure Nov 19, 2014

Closed

Android 5.0 - L #3

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment