v5 Result Object Proposal #813
Conversation
| // handle error | ||
| } | ||
| ``` | ||
| ## Single Result Object With Types |
There was a problem hiding this comment.
This feels the best to me, but it also reads similar to Swift so I think that makes sense. It sounds like this is also what other folks have moved to so would be a good move to modernize our approach.
Is there a world in which we'd consider offering 2 similar approaches in v5 and in a future v6 go with 1? In iOS v6 we offer both completions and async/await with a plan to go to only async/await in v7. I am not sure if something like this is feasible on Android though or would over complicate things.
There was a problem hiding this comment.
It would be a bit of a docs and development burden, but I could see the case for including the single result callback pattern and the listener pattern (since listener was the suggested v4 approach) to ease integration burden and confirm which approach merchants prefer. But that might create a whole mess of docs (you'd need to invoke different methods if you set the listener vs not)
There was a problem hiding this comment.
Ah gotcha - that makes sense. We just document 1 for iOS but we can kinda just call the callbacks from the async/await so it's not a ton of extra code lift. Whereas in Android it seems like we'd be adding a ton of extra work for ourselves.
There was a problem hiding this comment.
@sarahkoop - After our mob PR review, I also vote for this option. We can take a look at the past few inbounds in Slack & GH to see if they are mostly Kotlin (probably).
Historically we've been more conservative, but I think we're at a point now we can be more aggressive and go for this. We could also introduce it module-by-module and put out beta releases to see if people use it and have feedback.
| This approach relies on the Kotlin sealed class, so requires the `PaymentAuthResult` and | ||
| `PaymentResult` objects to live in `BraintreeCore`. It also requires the launchable result | ||
| objects to all extend a single class (ex: `LaunchableRequest`). This reduces code duplication in | ||
| our SDK, but requires casting by the merchants (ex: PaymentMethodNonce is returned instead of |
There was a problem hiding this comment.
requires casting by the merchants
Will this casting still be required once fully converted to Kotlin?
There was a problem hiding this comment.
Yes but Kotlin has smart casting so for Kotlin merchants it's as simple as checking if the object is PaymentResult.Success and then it's automatically cast to that type. So the Kotlin code snippet below won't change but it's already relatively clean.
Java merchants will always have the cumbersome type-checking/unwrapping.
| the Java integration becomes somewhat complex with casting. This approach aligns with how other | ||
| payment SDKs handle results. | ||
|
|
||
| This approach relies on the Kotlin sealed class, so requires the `PaymentAuthResult` and |
There was a problem hiding this comment.
so requires the
PaymentAuthResultandPaymentResultobjects to live inBraintreeCore
Does this mean the payment-method specific return types will have to live in Core? Like SEPADirectDebitNonce?
There was a problem hiding this comment.
No since the payment method nonces all inherit from PaymentMethodNonce (which lives in core), these result objects will just return a PaymentMethodNonce and the specific module clients can pass the module-specific nonce type to the result creation.
If we re-write the entire SDK in Kotlin, these result types could also become module specific (i.e. PayPalPaymentResult) and could return a PayPalAccountNonce specifically rather than a PaymentMethodNonce).
|
Closing this with decision to go with single result objects |
Summary of changes
Checklist
[ ] Added a changelog entryAuthors