add/subtract loose the scale when value is zero

[glureau]

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.

kotlin-multiplatform-bignum/bignum/src/commonMain/kotlin/com/ionspin/kotlin/bignum/decimal/BigDecimal.kt

Lines 1561 to 1580 in 1f61d2a

	private fun computeMode(other: BigDecimal, op: ScaleOps): DecimalMode {
	return if (decimalMode == null ||
	decimalMode.isPrecisionUnlimited ||
	other.decimalMode == null ||
	other.decimalMode.isPrecisionUnlimited
	)
	DecimalMode.DEFAULT
	else {
	DecimalMode(
	max(decimalMode.decimalPrecision, other.decimalMode.decimalPrecision),
	decimalMode.roundingMode,
	if (decimalMode.usingScale && other.decimalMode.usingScale)
	when (op) {
	ScaleOps.Max -> max(decimalMode.scale, other.decimalMode.scale)
	ScaleOps.Min -> min(decimalMode.scale, other.decimalMode.scale)
	ScaleOps.Add -> decimalMode.scale + other.decimalMode.scale
	} else
	-1
	)
	}

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!

[ionspin]

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.

[ionspin]

Sorry didn't have time this weekend to look into it, hopefully next one :(

[ionspin]

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

kotlin-multiplatform-bignum/bignum/src/commonMain/kotlin/com/ionspin/kotlin/bignum/decimal/BigDecimal.kt

Lines 1053 to 1066 in 1f61d2a

	fun scale(scale: Long): BigDecimal {
	if (scale < 0)
	throw ArithmeticException("Negative Scale is unsupported.")
	val mode = if (decimalMode == null) {
	if (scale == -1L) {
	DecimalMode.DEFAULT
	} else {
	DecimalMode(0, RoundingMode.ROUND_HALF_AWAY_FROM_ZERO, scale)
	}
	} else {
	DecimalMode(decimalMode.decimalPrecision - decimalMode.scale, decimalMode.roundingMode, scale)
	}
	return BigDecimal(significand, exponent, mode)
	}

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.

kotlin-multiplatform-bignum/bignum/src/commonMain/kotlin/com/ionspin/kotlin/bignum/decimal/BigDecimal.kt

Lines 76 to 97 in 1f61d2a

	init {
	if (_decimalMode != null && _decimalMode.usingScale) {
	val wrk = applyScale(_significand, _exponent, _decimalMode)
	if (wrk.isZero().not()) {
	significand = wrk.significand
	exponent = wrk.exponent
	val newPrecision = significand.numberOfDecimalDigits()
	precision = newPrecision
	decimalMode = _decimalMode.copy(decimalPrecision = newPrecision)
	} else {
	significand = wrk.significand
	exponent = wrk.exponent.times(_decimalMode.decimalPrecision + _decimalMode.scale)
	precision = _decimalMode.decimalPrecision + _decimalMode.scale
	decimalMode = _decimalMode.copy(decimalPrecision = precision)
	}
	} else {
	significand = _significand
	precision = _significand.numberOfDecimalDigits()
	exponent = _exponent
	decimalMode = _decimalMode
	}
	}

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?