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

Calling FastMoney.with(MonetaryOperator) results in a Arithmetic exception #415

Open
Dudeplayz opened this issue May 21, 2024 · 6 comments

Comments

@Dudeplayz
Copy link

I am converting USD to EUR. I use FastMoney, but when applying the example code:

var EUR_CONVERTER = MonetaryConversions
        .getExchangeRateProvider(ExchangeRateType.ECB, ExchangeRateType.IMF).getCurrencyConversion("EUR");
var bd = BigDecimal(0.657243).setScale(5, RoundingMode.HALF_UP);
var sourceMoney = FastMoney.of(bd, currency);
sourceMoney.with(EUR_CONVERTER);

I get java.lang.ArithmeticException: 0.9207255317189946 can not be represented by this class, scale > 5. I am expecting here, that the rounding scale of FastMoney is applied. Otherwise, what would be the correct way of handling that?

@keilw keilw added the analysis label May 23, 2024
@keilw
Copy link
Member

keilw commented May 23, 2024

Will have a look when there is time.

In the meantime, have you tried using Money instead of FastMoney for souceMoney?

@keilw keilw added the Exchange label May 23, 2024
@Dudeplayz
Copy link
Author

Will have a look when there is time.

In the meantime, have you tried using Money instead of FastMoney for souceMoney?

Yes, thats working. I have done this workaround (kotlin):

    fun roundedFastMoneyInEUR(sourceCurrency: String, sourceAmount: Double): FastMoney {
        if (sourceCurrency == EUR_CONVERTER.currency.currencyCode) {
            return roundedFastMoney(sourceCurrency, sourceAmount)
        } else {
            val sourceMoney = Money.of(sourceAmount, sourceCurrency).with(EUR_CONVERTER)
            return FastMoney.of(scaleDown(sourceMoney.numberStripped), EUR_CONVERTER.currency)
        }
    }

    private fun scaleDown(value: BigDecimal): BigDecimal {
        return value.setScale(5, RoundingMode.HALF_UP)
    }

The issue occurred after updating from 1.4.2 to 1.4.4, before the exact same code worked.

@keilw
Copy link
Member

keilw commented May 23, 2024

This could have been related to some hardcoded scaling e.g. before formatting, which we addressed in 1.4.3/1.4.4.
Glad it works, but what is the advantage of returning FastMoney (after you already used Money and BigDecimal) instead of the API type MonetaryAmount?

I left it open, but feel free to close it if the workaround, e.g. using Money addresses the problem.

@Dudeplayz
Copy link
Author

This could have been related to some hardcoded scaling e.g. before formatting, which we addressed in 1.4.3/1.4.4.

What is the assumption regarding such currency conversions, when scaling issues are resulting from it? Is FastMoney in this case always incompatible/unusable for this case?

Glad it works, but what is the advantage of returning FastMoney (after you already used Money and BigDecimal) instead of the API type MonetaryAmount?

Good point. I assumed it is faster in later usage, but mainly I only do one or two sorting operations on it later on. I changed my code already to MonetaryAmount, which makes sense. Just to be sure: from where comes the main performance difference between Money and FastMoney, is it related to its operations or to its creation etc. because my 'roundedFastMoney' operation also converts it to BigDecimal, scales it to 5 digits and creates the FastMoney with it.

@keilw
Copy link
Member

keilw commented May 24, 2024

FastMoney stores the value internally as a combination of long variables. That is why rounding issues may occur in some situations, passing a BigDecimal is fine, it will be stored the way it does and therefore the only overhead is before you create the instance.

@Dudeplayz
Copy link
Author

Ok thanks for the clarification. I have already changed my code to use MonetaryAmount and using the BigDecimal directly. You were right, I created additional overhead because I already used BigDecimal and converted it to FastMoney.

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

2 participants