Feature/exchange rate providers #147
Conversation
| listExchangeRateProvider.setOnPreferenceChangeListener(new Preference.OnPreferenceChangeListener() { | ||
| @Override | ||
| public boolean onPreferenceChange(Preference preference, Object newValue) { | ||
| PrefsUtil.edit().putString(PrefsUtil.EXCHANGE_RATE_PROVIDER, newValue.toString()).commit(); |
There was a problem hiding this comment.
If setOnPreferenceChangeListener is invoked, doesn't it imply that the value already has been changed and this line changes it again?
There was a problem hiding this comment.
Yes, but I want to update the exchange rates when changing the provider.
If I don't have that line here, it will update the exchange rates before the provider is switched and therefore fetching the rates from the wrong provider.
There was a problem hiding this comment.
Or in other words: The value is not changed until true is returned. That's why I have to do it earlier.
| * @param response a JSON response that comes from Blockchain.info | ||
| * @return | ||
| */ | ||
| public JSONObject parseBlockchainInfoResponse(JSONObject response) { |
There was a problem hiding this comment.
Maybe an easier way to parse the Json would be by using Gson.
see e.g. https://stackoverflow.com/questions/38636254/how-to-convert-json-to-java-object-using-gson
But works as well as we do it now 👍
| Iterator<String> iter = rates.keys(); | ||
| while (iter.hasNext()) { | ||
| String rateCode = iter.next(); | ||
| if (AppUtil.getInstance(mContext).getCurrencyNameFromCurrencyCode(rateCode) == null) { |
There was a problem hiding this comment.
Hmm, not sure but this loads every iteration the whole json list from resources. Maybe a little cost expensive.
Would be nice if we can load the list once and then use it.
Description
Added option to change exchange rate provider. (In Advanced Settings)
For now only Coinbase is added, but should be simple to add more providers now.
Separated exchange rate providers from monetary util into its own.
Replaced a lot of strings with constants.
Motivation and Context
Address issue #104
How Has This Been Tested?
On my S9
Screenshots (if appropriate):
Types of changes
Checklist: