remove joda dependency that was triggering an `UnsupportedOperationEx… #286
Conversation
…ception`. Avoid returning null as fallback for the introductoryPriceText.
| @@ -83,72 +77,65 @@ public SkuDetails(JSONObject source) throws JSONException | |||
| priceLong = source.optLong(Constants.RESPONSE_PRICE_MICROS); | |||
| priceValue = priceLong / 1000000d; | |||
| priceText = source.optString(Constants.RESPONSE_PRICE); | |||
| subscriptionPeriod = parsePeriod(source.optString(Constants.RESPONSE_SUBSCRIPTION_PERIOD, "P")); | |||
| subscriptionFreeTrialPeriod = parsePeriod(source.optString(Constants.RESPONSE_FREE_TRIAL_PERIOD, "P")); | |||
| haveTrialPeriod = subscriptionFreeTrialPeriod.toStandardSeconds().getSeconds() != 0; | |||
There was a problem hiding this comment.
can we avoid renaming existing fields?
There was a problem hiding this comment.
I haven't renamed these fields, this diff is due to the line changes, in fact I have removed the haveTrialPeriod inserted in the previous PR #281 because it's pointless if we don't use joda, and here it's where it was crashing, because of the toStandardSeconds() that's not working for all formats.
There was a problem hiding this comment.
Seem like this field could be useful. We can determine its value using TextUtils.isEmpty(subscriptionFreeTrialPeriod) for instance
Other then this, LGTM
| subscriptionPeriod = parsePeriod(source.optString(Constants.RESPONSE_SUBSCRIPTION_PERIOD, "P")); | ||
| subscriptionFreeTrialPeriod = parsePeriod(source.optString(Constants.RESPONSE_FREE_TRIAL_PERIOD, "P")); | ||
| haveTrialPeriod = subscriptionFreeTrialPeriod.toStandardSeconds().getSeconds() != 0; | ||
| subscriptionPeriod = source.optString(Constants.RESPONSE_SUBSCRIPTION_PERIOD, "P"); |
There was a problem hiding this comment.
this is from the last PR you merged https://github.com/anjlab/android-inapp-billing-v3/pull/281/files#diff-82bb12704ee8c87bd183b059a6972bffR86, where the guys set a default P for period.
There was a problem hiding this comment.
I guess P was an argument to a parse method, not actually a default value
There was a problem hiding this comment.
I am happy to use the default as I did in my PR https://github.com/anjlab/android-inapp-billing-v3/pull/282/files#diff-82bb12704ee8c87bd183b059a6972bffR77, what he did was using the fallback "P" instead of the default empty "", and I left it because I thought it was more convenient to have this default for who needs this param. I will just use the default ref. https://github.com/stleary/JSON-java/blob/master/JSONObject.java#L1372
There was a problem hiding this comment.
Also here a ref. of how a subscriptionPeriod looks like: https://github.com/anjlab/android-inapp-billing-v3/pull/281/files#diff-ebc6d11b564dc13ba02eb68f29e058ffR9
anjlab#286) * remove joda dependency that was triggering an `UnsupportedOperationException`. Avoid returning null as fallback for the introductoryPriceText. * remove fallback for some skuDetails fields. * insert booleans for checking whether there is a trialPeriod/introductoryPeriod or not. * insert missing tests from previous PR.
UnsupportedOperationExceptionIn general it's a good approach avoiding to add dependencies to libraries whenever possible
nullas fallback forintroductoryPriceText, but rather returns the default to avoid potential crash.