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

Inconsistency between #compareTo an isXxx comparison methods #102

Open
marschall opened this issue Nov 11, 2018 · 1 comment
Open

Inconsistency between #compareTo an isXxx comparison methods #102

marschall opened this issue Nov 11, 2018 · 1 comment
Labels

Comments

@marschall
Copy link
Member

Judging from the RI the contract of MonetaryAmount#compareTo seems to be to first compare by currency code (not by currency, even though CurrencyUnit is Comparable) before comparing by numeric value. However all the convenience comparison methods #isGreaterThan, #isGreaterThanOrEqualTo, #isLessThan, #isLessThanOrEqualTo, #isEqualTo are specified to throw MonetaryException if the currency code (not the currency) is different.

This leads to the following behavior:

FastMoney money = FastMoney.of(BigDecimal.valueOf(2L), CHF);

assertTrue(money.compareTo(FastMoney.of(BigDecimal.valueOf(1L), EUR)) < 0); // passes

assertTrue(money.isLessThan(FastMoney.of(BigDecimal.valueOf(1L), EUR))); // throws MonetaryException

This may be an issue with the RI and not the API, as the API gives no guidance on how #compareTo should be implemented. However the API requires that #equals considers the currency unit and that best practices are that a.equals(b) implies a.compareTo(b) == 0 .

@keilw keilw added the analysis label Nov 12, 2018
@stokito
Copy link
Member

stokito commented Feb 10, 2019

A good point.
Comparator can be used for example to sort some amounts in table. And that’s ok to compare amounts with different currencies. So it shouldn’t throw any exceptions for different currencies but instead it should compare by a currency firts.

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

No branches or pull requests

3 participants