jsr354-ri icon indicating copy to clipboard operation
jsr354-ri copied to clipboard

RoundedMoney instances not rounded

Open marschall opened this issue 5 years ago • 7 comments

When an instance of RoundedMoney is created the rounding operator is not applied. This compounded by the fact that operations assume the instance is rounded so the shortcuts or add, subtract, multiply and divide just return the same, unrounded instance. The same goes for the plus, negate and abs.

I am not sure whether this behavior is intentional or not as the class comment of RoundedMoney does not spell out when the rounding operator should be applied. Rather the class comment seems to be copy and pasted from Money.

marschall avatar Jan 03 '19 07:01 marschall

@atsticks Any idea about that? Is it a bug or a feature that works as intended?

keilw avatar Jan 08 '19 13:01 keilw

I asked a similar question at stackoverflow.

https://stackoverflow.com/questions/52082946/org-javamoney-moneta-roundedmoney-some-operations-result-in-a-rounded-value-so

import java.math.BigDecimal;
import javax.money.CurrencyUnit;
import javax.money.Monetary;
import org.javamoney.moneta.RoundedMoney;

public final class RoundedMoneyRounding
{
    private RoundedMoneyRounding()
    {
    }


    public static void main(final String... args)
    {
        final CurrencyUnit usd = Monetary.getCurrency("USD");
        final RoundedMoney halfcent = RoundedMoney.of(new BigDecimal("0.005"), usd);
        final RoundedMoney zero = RoundedMoney.of(BigDecimal.ZERO, usd);

        System.out.append("A1. 0.005 + 0 = ").println(//
                                                      halfcent.add(zero) //
                                                                      .getNumber().numberValue(BigDecimal.class).toPlainString());

        System.out.append("A2. 0 + 0.005 = ").println(//
                                                      zero.add(halfcent) //
                                                                      .getNumber().numberValue(BigDecimal.class).toPlainString());

        System.out.println("----");

        System.out.append("B1: -0.005 = ").println(//
                                                   halfcent.negate() //
                                                                   .getNumber().numberValue(BigDecimal.class).toPlainString());

        System.out.append("B2: 0.005 * -1 = ").println(//
                                                       halfcent.multiply(new BigDecimal("-1")) //
                                                                       .getNumber().numberValue(BigDecimal.class).toPlainString());

        System.out.println("----");

        System.out.append("C1: 0.005 * 1 = ").println(//
                                                      halfcent.multiply(BigDecimal.ONE) //
                                                                      .getNumber().numberValue(BigDecimal.class).toPlainString());

        System.out.append("C2: 0.005 * 1.1 = ").println(//
                                                        halfcent.multiply(new BigDecimal("1.1")) //
                                                                        .getNumber().numberValue(BigDecimal.class).toPlainString());

        System.out.println("----");

        System.out.append("D1: 0.005 * 2 = ").println(//
                                                      halfcent.multiply(new BigDecimal("2")) //
                                                                      .getNumber().numberValue(BigDecimal.class).toPlainString());

        System.out.append("D2: (0.005 * 2) / 2 = ").println(//
                                                            halfcent.multiply(new BigDecimal("2")).divide(new BigDecimal("2")) //
                                                                            .getNumber().numberValue(BigDecimal.class).toPlainString());
    }
}

Output:

A1. 0.005 + 0 = 0.005
A2. 0 + 0.005 = 0
----
B1: -0.005 = -0.005
B2: 0.005 * -1 = 0
----
C1: 0.005 * 1 = 0.005
C2: 0.005 * 1.1 = 0.01
----
D1: 0.005 * 2 = 0.01
D2: (0.005 * 2) / 2 = 0

(I used moneta 1.3 from Maven Central)

rfcom avatar Feb 14 '19 12:02 rfcom

@marschall I'm not sure, what's the expected behavior here, can you elaborate? Anyway we'll plan that most likely for a brand new JSR, it won't go into 1.4.1 which is due in the next few days.

keilw avatar Jul 19 '20 22:07 keilw

My expectation would be the following:

  1. The class comment documents the contract of the behavior of the class. I believe that's a pretty reasonable expectation. Otherwise users are left guessing and have to resort to reading the source.
  2. I would expect instances of RoundedMoney to always have the rounding applied for the following reasons:
    1. It makes the class easier and less error prone to use as the caller is now no longer responsible for applying the rounding operator upon creating an instance.
    2. It makes the design of the class more consistent are there are now no longer any not rounded instances of RoundedMoney. Arguably this behavior is more intuitive and better matches the expectation of the user.

marschall avatar Jul 20 '20 17:07 marschall

Unfortunately that example (taken from Stackoverflow) is not intuitive and leaves one to guessing when it may apply. Either way, this looks like it's way beyond a quick fix, if there was more configuration required or parameters to RoundedMoney, we'd have to plan something for a new JSR (which everyone is welcome to contribute, I recall you are a JCP member already)

keilw avatar Jul 21 '20 07:07 keilw

Yes, I'm an associate member.

marschall avatar Jul 26 '20 12:07 marschall

Well happy if you'd be willing to help with the next one, this issue IMO will take a bit of time and discussion with other members of a future EG, so we leave it to the next major version (planned to start around the end of the year if all goes well)

keilw avatar Jul 26 '20 16:07 keilw