Add priceFormatter to StoreProduct#1028
Conversation
| private lazy var formatter: NumberFormatter = { | ||
| @objc public override var priceFormatter: NumberFormatter? { |
There was a problem hiding this comment.
had to remove lazy here because this is a computed var.
I think an optimal version of this would be to use a private static var, so we can check if it's initialized and only do it once, then listen for storefront changes.
the public var would be the same, it would just redirect internally to the static var.
| configurePurchases() | ||
| var completionCalled = false | ||
| var receivedError: Error? = nil | ||
| var receivedOfferings: Offerings? = nil |
There was a problem hiding this comment.
I still don't like this at all 😅 in a strongly typed language we never put the type in variable names. This is like doing completionCalledBoolean. The fact that this is a "Maybe" (like Haskell calls it) is already encoded in the type.
There was a problem hiding this comment.
yeah, I hear ya. I find it redundant often too.
I do find it useful for callbacks in particular, though, and as an ocasional reminder to consider nullability. It might still be the python coder in me that won't let go.
There was a problem hiding this comment.
The compiler will remind you :P
|
|
||
| @objc public override var localizedTitle: String { underlyingSK2Product.displayName } | ||
|
|
||
| @objc public override var priceFormatter: NumberFormatter? { |
There was a problem hiding this comment.
Maybe document that this creates a new formatter every time, which can be slow?
There was a problem hiding this comment.
good call. I'll add a TODO: as well as a reminder to tackle it before shipping ideally
| let priceFormatter = try XCTUnwrap(storeProduct.priceFormatter) | ||
| let productPrice = storeProduct.price as NSNumber | ||
|
|
||
| expect(priceFormatter.string(from: productPrice)) == "$4.99" |
There was a problem hiding this comment.
Not for this PR
It would be nice to have tests that ensure this formats in other locales too. The new Test Plans in Xcode makes that a lot easier.
Otherwise if we changed the implementation to hardcode this to Dollars it would still pass.
There was a problem hiding this comment.
I think I can do that with StoreKitTest, I'll take a look
Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com>
Fixes #1013
Will follow up with another PR for the new methods that already format the price, figured smaller PRs was probably better.
Also does some small cleanup for outdated comments after the AvailabilityChecks PR