mikvor / money-conversion

Money conversion and arithmetics utility classes.
64 stars 17 forks source link

Bug in multiplication #14

Open alensiljak opened 9 years ago

alensiljak commented 9 years ago

Here is a test case. Real-world situation is a calculation of a percentage of the total sum.

        // allocation is 30%
        double allocation = 30;
        // total value
        Money totalValue = MoneyFactory.fromString("7281.6612");
        // money value of 30% allocation
        double value = allocation * totalValue.toDouble() / 100;
        Money moneyValue = MoneyFactory.fromDouble(value);

        // calculate the percentage of the money value
        double currentAllocationD = moneyValue.multiply(100)
            .divide(totalValue.toDouble(), Constants.DEFAULT_PRECISION)
            .toDouble();

        Money currentAllocation = moneyValue.multiply(100)
            .divide(totalValue.toDouble(), Constants.DEFAULT_PRECISION);

        // it should be 30, as set initially.
        assertThat(currentAllocationD).isEqualTo(30);
        assertThat(currentAllocation.toString()).isEqualTo("30");

The wrong number comes out of the multiplication with 100.

mikvor commented 9 years ago

Alen, I can fix this and other bug in about a week. I'm in the middle of a major relocation.

alensiljak commented 9 years ago

No, problem, Mikhail. There is a workaround for the bug where, instead of

double value = allocation * totalValue.toDouble() / 100;
Money moneyValue = MoneyFactory.fromDouble(value);

I can simply reverse the operations

Money moneyValue = totalValue.multiply(allocation).divide(100, Constants.DEFAULT_PRECISION);

and the result is correct. And if I have some spare time, I might have a shot at adding isZero() and/or some of the other helper methods. Cheers

archenroot commented 6 years ago

Is there any future work planned on this high perormance bigdecimal replacement?

ratcashdev commented 6 years ago

submitted PR #17 (currently a test-case only) so that everyone is aware of this bug until fixed.

ratcashdev commented 6 years ago

see https://github.com/mikvor/money-conversion/pull/17 for the fix.