kotlin-multiplatform-bignum
kotlin-multiplatform-bignum copied to clipboard
add/subtract loose the scale when value is zero
Describe the bug
Operations like addition or subtraction are loosing the scale when one of the operand value is zero with a scale of zero.
To Reproduce
println((BigDecimal.parseString("1").scale(0) + BigDecimal.parseString("2").scale(1)).scale) // 1
println((BigDecimal.parseString("0").scale(1) + BigDecimal.parseString("2").scale(1)).scale) // 1
println((BigDecimal.parseString("0").scale(0) + BigDecimal.parseString("2").scale(1)).scale) // -1 : KO, expected 1
println((BigDecimal.parseString("1").scale(0) - BigDecimal.parseString("2").scale(1)).scale) // 1
println((BigDecimal.parseString("0").scale(0) - BigDecimal.parseString("2").scale(1)).scale) // -1 : KO, expected 1
Expected behavior A clear and concise description of what you expected to happen.
Platform
- tested on JVM, as the code is checking with
== ZERO
I presume it's impacting every platforms.
Additional context I read a bit the code, I presume it's related to this initial condition, as decimalMode.isPrecisionUnlimited is true (because I don't really need to define the precision here) then I don't calculate the best scale. https://github.com/ionspin/kotlin-multiplatform-bignum/blob/1f61d2a172237c7ee3aa7771a016749ae750481b/bignum/src/commonMain/kotlin/com/ionspin/kotlin/bignum/decimal/BigDecimal.kt#L1561-L1580
I'm open to push a PR if there's a clear way to go. I guess I could change this DEFAULT with a .copy(scale = ...)
but no idea if it's the right approach.
PS : Thanks for your work, greatly appreciated!
Hi @glureau, thanks for reporting and for offering help, it's appreciated! I'll give the issue a look over the weekend and get back to you.
Sorry didn't have time this weekend to look into it, hopefully next one :(
Hi @glureau,
I got some time to look into this, it seems that the original issue is that when scale was applied to zero, it would incorrectly have a decimalPrecision set to 0 instead of 1, and that signaled isPrecisionUnlimited
as true, which I think is not correct.
If you take a look at what happens when scale is applied, in last line of this block we are creating a new BigDecimal
https://github.com/ionspin/kotlin-multiplatform-bignum/blob/1f61d2a172237c7ee3aa7771a016749ae750481b/bignum/src/commonMain/kotlin/com/ionspin/kotlin/bignum/decimal/BigDecimal.kt#L1053-L1066
And then if we follow what happens next we can see that we end up in line 88 that sets the precision to 0, and that I think is not correct.
https://github.com/ionspin/kotlin-multiplatform-bignum/blob/1f61d2a172237c7ee3aa7771a016749ae750481b/bignum/src/commonMain/kotlin/com/ionspin/kotlin/bignum/decimal/BigDecimal.kt#L76-L97
I think the fix should go there, and hardcode the precision in line 88 (when wrk is determined to be equal to zero) to a value of 1, because I think precision of zero is irrelevant as long as it's not 0.
Also I think that multiplication to calculate the exponent is pointless since the significand is zero, but I can't recall the reason why I wrote it like that, maybe it was a copy-paste from some case where it had meaning.
Would you like to look into this and submit a PR?