RubyMoney / google_currency

Ruby Money::Bank interface for the Google Currency exchange data
http://rubymoney.github.com/google_currency
MIT License
182 stars 90 forks source link

incorrect conversion of IDR to USD #34

Closed lenidot closed 9 years ago

lenidot commented 9 years ago

The gem gives an incorrect conversion rate from IDR to USD. The USD to IDR rate is correct.

irb> Money.new(10000000, :IDR).exchange_to(:USD) =>

irb> Money.new(1000, :USD).exchange_to(:IDR) =>

The wrong result isn't really the gem's fault. Google's web service is rounding it's result to 4 decimal places, which isn't enough precision for a currency like IDR (where the exchange rate is currently around 12950:1).

$ GET 'http://www.google.com/finance/converter?a=1&from=IDR&to=USD'

1 IDR = 0.0001 USD The resulting behaviour is nasty because apps that use such conversion silently get an incorrect exchange rate. Options for a fix: 1. ask Google to increase the precision of their service and/or 2. when Google returns an exchange rate of 0.0001, the gem could lookup the reverse rate and then swap the numerator and denominator or 3. when Google returns an exchange rate of 0.0001, the gem could ignore it. Better that clients get no rate than an incorrect one.
semmons99 commented 9 years ago

is option 4, use a different service? :smile:

I'm not sure how much we want to guess at what Google is doing all the time vs asking them to fix. Thoughts?

yinquanteo commented 9 years ago

The other thing we could do is to put a disclaimer in the README about the precision of Google's conversion.

Google's service actually doesn't convert for exchange rates greater than 4 decimal places. E.g. 1 Vietnamese Dong to USD: https://www.google.com/finance/converter?a=1&from=VND&to=USD

semmons99 commented 9 years ago

That's probably the right choice. Would you make a PR?

  • shane

On Tue, Apr 21, 2015 at 5:57 PM, Yinquan (Yink) Teo notifications@github.com wrote:

The other thing we could do is to put a disclaimer in the README about the precision of Google's conversion.

Google's service actually doesn't convert for exchange rates greater than 4 decimal places. E.g. 1 Vietnamese Dong to USD: https://www.google.com/finance/converter?a=1&from=VND&to=USD

Reply to this email directly or view it on GitHub: https://github.com/RubyMoney/google_currency/issues/34#issuecomment-94955413

yinquanteo commented 9 years ago

@semmons99 I'll merge the PR if the phrasing works?

semmons99 commented 9 years ago

looks good

Tarang commented 9 years ago

I don't think using the disclaimer is very helpful. I've created a pull request #37 with the ability to use # 2 as @lenidot suggests. This uses an inverse conversion (e.g USD to IDR instead of IDR to USD, then uses the reciprocal which carries a bit more precision)

This should be more precise with the whitelisted currency codes.

P.S it was great having team firststep meet you, Leni!

yinquanteo commented 9 years ago

What happens then if we do a JPY to IDR conversion? Using the approach you propose, we'd end up using the inverse of the 1 IDR to JPY rate which will have a lower precision than the original 1 JPY to IDR rate. What happens if the exchange rate for your whitelisted currencies change dramatically?

I feel like this approach presupposes some knowledge of the underlying currencies. I am more comfortable with taking a more general approach.

I have 2 suggestions for a general approach. We can detect if the conversion rate has 4 decimal places but has 2 or fewer significant digits\ (e.g. 0.0042) and then do one of the following:

  1. Have a "precise mode" that would throw an error if * is satisfied. No error thrown if * not satisfied.
  2. Use the inverse conversion you suggested only if \ is satisfied.

@semmons99 thoughts?

semmons99 commented 9 years ago

I like option 2

Tarang commented 9 years ago

@semmons99 So be it. How does this sound:

1) Check for 2 significant digits 2) Requery with an inverse exchange rate

This should:

a) Solve the issue of presupposed knowledge b) Ensure precision for all currencies no matter of the combination used in the request.

I could refashion the PR to incorporate these changes if agreed?

semmons99 commented 9 years ago

Perfect!

On Thu, May 14, 2015 at 12:04 PM, Tarang Patel notifications@github.com wrote:

@semmons99 So be it. How does this sound: 1) Check for 2 significant digits 2) Requery with an inverse exchange rate This should: a) Solve the issue of presupposed knowledge b) Ensure precision for all currencies no matter of the combination used in the request.

I could refashion the PR to incorporate these changes if agreed?

Reply to this email directly or view it on GitHub: https://github.com/RubyMoney/google_currency/issues/34#issuecomment-102085646