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

new ExchangeRate() Value round to 4 digits only. Loss of fraction occurs #43

Closed
v0id24 opened this issue Jul 6, 2016 · 2 comments
Closed
Milestone

Comments

@v0id24
Copy link

v0id24 commented Jul 6, 2016

Hi!
In most cases base currency have bigger value than quote currency.
But in some situations (money order between two people with different currencies; or simply nonstandard pair where quote currency and base currency are overturned) ExchangeRate.Value is much less than 1 and loss of fraction occurs.

Maybe rounding should be more than 4 digits or to add exception if ExchangeRate.Value is less than 1 to prevent losses.

@v0id24
Copy link
Author

v0id24 commented Jul 6, 2016

The workaround for me now is
ExchangeRate exchangeRate = new ExchangeRate(baseCurrency, quoteCurrency, baseCurrencyRate / quoteCurrencyRate);
if (exchangeRate.Value < 1m)
{
exchangeRate = new ExchangeRate(quoteCurrency, baseCurrency, quoteCurrencyRate / baseCurrencyRate);
}

@RemyDuijkeren RemyDuijkeren added this to the v0.7.0 milestone Jul 6, 2016
@RemyDuijkeren
Copy link
Owner

RemyDuijkeren commented Jul 6, 2016

Good point. I was under the assumption that exchange rates are always given in 4 decimals. It looks like some exchangerateproviders provide rates for the "quote" currency up to 6 decimal places.

I think the ExchangeRate type should be changed to allow 6 decimals or even better 6 significant figures.

BTW ExchangeRate with a value of less then 1, should not be a problem in itself. Rounding of money can happen and is allowed, so far as I know.

RemyDuijkeren pushed a commit that referenced this issue Sep 9, 2016
@RemyDuijkeren RemyDuijkeren mentioned this issue Sep 9, 2016
RemyDuijkeren pushed a commit that referenced this issue Sep 9, 2016
* Changing to .NET core project, Fixed #44 
* Changed test projects to .NET Core
* Projects adjustments for building .net core.
* Fixed #43 
* Fixed #42
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