ionspin / kotlin-multiplatform-bignum

A Kotlin multiplatform library for arbitrary precision arithmetics
Apache License 2.0
339 stars 40 forks source link

BigDecimal.scale not working as BigDecimal from java.math #258

Closed MaxMalina closed 1 year ago

MaxMalina commented 1 year ago

Hey! It looks like an issue! BigDecimal scale by default is set to impossible value -1L. That produce behaviour which different from java.math.BigDecimal. For example: java.math.BigDecimal(1).scale() returns 1 While here: BigDecimal.fromDouble(1.0).scale returns -1

Same with scale over one: java.math.BigDecimal.valueOf(1.23).scale() returns 2 While here: BigDecimal.fromDouble(1.23).scale returns -1

ionspin commented 1 year ago

Hi @MaxMalina, thanks for reporting, at the moment this is not really an issue because that is the expected value. Let me give you a short introduction why that is.

When I first implemented this library, for BigDecimal I chose to use a class called DecimalMode to handle rounding, precision, etc. Scale was added much later as a contribution by @skolson and we had to make some tradeoffs so that now both approaches can coexist. One of the tradeoffs was that scale value of -1 signals that scale was not set and that the decimal mode is the one that is valid.

You can see a snippet of code that uses it here https://github.com/ionspin/kotlin-multiplatform-bignum/blob/281d0e79ddc48883134bdbf91224c967bcbe6aee/bignum/src/commonMain/kotlin/com/ionspin/kotlin/bignum/decimal/DecimalMode.kt#L99

Another thing is that this library while inspired in parts by Java big number implementation was never meant to be completely compatible or a drop-in replacement, although they are quite similar.

With all that said, and I mentioned this several times, I hate how I initially implemented DecimalMode and it's going to be rewritten and probably deprecated in favor of using just scale.

I'm going to close this issue, as scale currently work as expected, but I have created a project and a feature request issue to rework the DecimalMode/Scale API so it's more intuitive to use. Once this is implemented I could imagine that scale will behave like java scale and return values that you mentioned. Here is the new issue #259