RoundedMoney. Incorrect result from method chain (multiply, divide)

[artbasov]

I've created a small test which is failing in current master/HEAD and in 1.3 version for RoundedMoney but passing for Money (if adopted)

@Test
    public void testMultiplyDivideChain() {
        RoundedMoney actual = RoundedMoney.of(1000.0, "USD");
        Double multiplicand = 15.0;
        BigDecimal expected = new BigDecimal("1000.00")
                .multiply(BigDecimal.valueOf(multiplicand))
                .divide(BigDecimal.valueOf(100), RoundingMode.HALF_EVEN);
        assertEquals(actual.multiply(multiplicand).divide(100), RoundedMoney.of(expected, "USD"));
    }

Is this an intended behavior and if so, is there a way to get the expected result in some other way with RoundedMoney? This issue might repeat the last case from #277 but uses different public API

[stokito]

Interesting, that's definitely a bug and is critical as for me.

[stokito]

Let me add more clear test case. Put it into RoundedMoneyTest.

    @Test
    public void testMultiplyDivideChain() {
        // test that 1000 * 15 / 100 == 150
        double multiplicand = 15.0;
        MonetaryOperator ro = ScaleRoundedOperator.of(2, RoundingMode.HALF_UP);
        RoundedMoney actual = RoundedMoney.of(1000.0, "USD", ro)
            .multiply(multiplicand)
            .divide(100);
        BigDecimal expected = new BigDecimal("1000.00")
            .multiply(BigDecimal.valueOf(multiplicand))
            .divide(BigDecimal.valueOf(100), RoundingMode.HALF_EVEN);
        assertEquals(actual, RoundedMoney.of(new BigDecimal("150.000"), "USD"));
        assertEquals(actual.multiply(multiplicand).divide(100), RoundedMoney.of(expected, "USD"));
    }

    @Test
    public void testMultiplyScaledBigDecimal() {
        BigDecimal num = new BigDecimal("15000");// "15000" intCompact = 15000 scale = 0 precision = 5
        System.out.println(num); //> "15000"
        System.out.println(num.toPlainString()); //> "15000"
        BigDecimal divisor = new BigDecimal("100");
        BigDecimal dec = num.divide(divisor, RoundingMode.HALF_UP);
        System.out.println(dec); //> "150" OK

        num = new BigDecimal("15000").setScale(-3); // "1.5+4" intComp = 15 scale -3 precision = 2
        System.out.println(num); //> "1.5E+4"
        System.out.println(num.toPlainString()); //> "15000"
        divisor = new BigDecimal("100");
        dec = num.divide(divisor, RoundingMode.HALF_UP);
        System.out.println(dec); //> "0E+3" WTF?
    }

So inside of RoundingMoney.divide() we have a number with negative scale and during division it returns a zero. As for me this looks insane

[artbasov]

I've come to the conclusion that it is the scale too but since JSR-354 wording is not restrictive on this I haven't pointed that out

Returns a MonetaryAmount whose value is this / divisor, and whose preferred scale is this.scale() - divisor.scale();

Note that for BigDecimal#divide it states:

Returns a BigDecimal whose value is (this / divisor), and whose scale is this.scale().

[stokito]

https://stackoverflow.com/questions/905795/java-bigdecimal-problems-with-division

[stokito]

ok, I don't have a time to fix it but if you @artbasov can send us PR that would be great. As a workaround you may try to do something like:

RoundedMoney.of(new BigDecimal("1000.00"), "USD") 

Maybe this will create a RoundMoney instance with a proper scale. Not sure if it will works

[keilw]

Is @artbasov a JCP Member at least Associate or could he become one? We'd be happy to consider him and others as Contributors to JSR 354 if you want to.

[stokito]

Please remind me: we can't accept PRs from not a JCP members?

[artbasov]

It seems that we are getting scale = -2 when we are creating a new instance as a result of the multiply. It happens in org.javamoney.moneta.spi.ConvertBigDecimal#stripScalingZeroes with trace:

at org.javamoney.moneta.spi.ConvertBigDecimal.stripScalingZeroes(ConvertBigDecimal.java:136)
      at org.javamoney.moneta.spi.ConvertBigDecimal.access$100(ConvertBigDecimal.java:32)
      at org.javamoney.moneta.spi.ConvertBigDecimal$5.getDecimal(ConvertBigDecimal.java:72)
      at org.javamoney.moneta.spi.ConvertBigDecimal.of(ConvertBigDecimal.java:103)
      at org.javamoney.moneta.spi.MoneyUtils.getBigDecimal(MoneyUtils.java:90)
      at org.javamoney.moneta.spi.MoneyUtils.getBigDecimal(MoneyUtils.java:102)
      at org.javamoney.moneta.RoundedMoney.<init>(RoundedMoney.java:119)
      at org.javamoney.moneta.RoundedMoney.<init>(RoundedMoney.java:87)
      at org.javamoney.moneta.RoundedMoney.multiply(RoundedMoney.java:399)
      at org.javamoney.moneta.RoundedMoneyTest.testMultiplyDivideChain(RoundedMoneyTest.java:522)

Input: intCompact=1500000, scale=2, precision=7, Output: intCompact=15, scale=-3, precision=0 Later in org/javamoney/moneta/spi/MoneyUtils.java:105 we change it to intCompact=15, scale=-3, precision=2

I would reevaluate the need to do stripScalingZeroes in that case. I'll try to play with this later, but I'm not a JCP Member, nor Associate, but I don't mind if someone who is would issue PR with my changes

[artbasov]

@stokito @keilw Could you guys explain what is the difference between Money and RoundedMoney form developer standpoint?

[keilw]

@stokito In the RI and API we should not have IP contributions from a non-JCP member. Eclipse Foundation has similar requirements if you want to contribute to Jakarta EE now. A single line of code could be tolerable, but if your contribution is all over the place, you should sign at least the Associate Agreement (which is no different from those at Apache or Eclipse). Apache won't let you touch any code without that to even start with, ask @atsticks.

[roanbrasil]

I am a JCP member can I contribute?

[keilw]

Yes, although we already proposed MR1 of the JSR, but if you have a PR or similar fix for a problem, we can always release 1.4.1 after that.