Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RoundedMoney instances not rounded #277

Open
marschall opened this issue Jan 3, 2019 · 7 comments
Open

RoundedMoney instances not rounded #277

marschall opened this issue Jan 3, 2019 · 7 comments

Comments

@marschall
Copy link
Member

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.

@keilw keilw added the analysis label Jan 8, 2019
@keilw
Copy link
Member

keilw commented Jan 8, 2019

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

@rfcom
Copy link

rfcom commented Feb 14, 2019

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)

@keilw
Copy link
Member

keilw commented Jul 19, 2020

@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.

@marschall
Copy link
Member Author

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.

@keilw
Copy link
Member

keilw commented Jul 21, 2020

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 keilw added the deferred label Jul 21, 2020
@keilw keilw added this to the .Next milestone Jul 21, 2020
@marschall
Copy link
Member Author

Yes, I'm an associate member.

@keilw
Copy link
Member

keilw commented Jul 26, 2020

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants