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

Currency converter broken #29

Closed
NickAnderegg opened this issue Nov 4, 2017 · 7 comments
Closed

Currency converter broken #29

NickAnderegg opened this issue Nov 4, 2017 · 7 comments

Comments

@NickAnderegg
Copy link

I'm suddenly not able to do all the currency conversions I'm used it. I've used the currency converter several times a day for years now, and it's suddenly failing to recognize my most used conversion, USD to CAD.

image

image

@madsb
Copy link

madsb commented Nov 4, 2017

Many cryptocurrencies are also broken. BTC/LTC works, ZEN/RLC/GUP etc. are broken.

@deanishe
Copy link
Owner

deanishe commented Nov 4, 2017

Not the same problem as the last issue? #28

@madsb
Copy link

madsb commented Nov 4, 2017

Adding the APP_KEY fixed it for me - thanks!

@NickAnderegg
Copy link
Author

Yup, I missed that issue. It might be helpful to have a notice or something when someone tries a conversion with the new version, because I'm guessing not everyone will check GitHub!

@deanishe
Copy link
Owner

deanishe commented Nov 5, 2017

It might be helpful to have a notice or something

I’ve thought about how best to do that. The way the workflow is currently built means it doesn’t know any currencies until they’re loaded from the cache … which doesn’t exist until APP_KEY is set.

Maybe add some extra logic to the “unknown unit” error that checks it against the currency list, and shows a different notice if the unit is a currency.

@NickAnderegg
Copy link
Author

You could probably get away with hard coding a list of just a handful of the most common currencies to catch >95% of users. Like there's probably not a huge contingent of users who are converting New Zealand Dollars into Swiss Francs or South African Rand into Indian Rupees.

It'll probably be fine to just check something like:

  • USD
  • CAD
  • GBP
  • EUR
  • AUD
  • JPY
  • CNY

I'd best the majority of exchange rate looks up will involve at least one of those currencies and cover most users.

@deanishe
Copy link
Owner

deanishe commented Nov 6, 2017

the most common currencies

I don’t think that would work: the workflow short circuits to failure if the first currency isn’t recognised.

It’s not a big deal: it already contains a list of all currencies, they just aren’t fed into the conversion library unless an exchange rate is available.

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