checkout / frames-android

Frames Android: making native card payments simple
https://www.checkout.com/docs/integrate/sdks/android-sdk
MIT License
47 stars 32 forks source link

PIMOB:2159 - Creating component factory and public interfaces #236

Closed chintan-soni-cko closed 9 months ago

chintan-soni-cko commented 9 months ago

Issue

PIMOB-2159

Proposed changes

Note: This pr is for base for public interfaces. Adding unit tests will be part of the next following PR for core work.

Test Steps

If there's any functionality change, please list a step by step guide of how to verify the changes, and/or upload a screen recording for any visible changes.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you choose the solution you did and what alternatives you considered, etc...

chintan-soni-cko commented 9 months ago

I found slightly confusing for the naming here, as we have CVVComponentFactory, ComponentApi, ComponentApiClient, ComponentMediator, InternalComponentMediator.

Is there any naming hierarchy we are following here? Maybe we could use some simplification here.

@jheng-hao-lin-cko refer diagram and we can discuss how we find it is confusing and what we can simplify here. We provided public interfaces which totally hides the implementation details and it following the existing Frames archietecture. for instance, refer CheckoutApiServiceFactory

jheng-hao-lin-cko commented 9 months ago

I found slightly confusing for the naming here, as we have CVVComponentFactory, ComponentApi, ComponentApiClient, ComponentMediator, InternalComponentMediator. Is there any naming hierarchy we are following here? Maybe we could use some simplification here.

@jheng-hao-lin-cko refer diagram and we can discuss how we find it is confusing and what we can simplify here. We provided public interfaces which totally hides the implementation details and it following the existing Frames archietecture. for instance, refer CheckoutApiServiceFactory

The reason I found confusing is for instance, the API as a name is generic, it could be anything. While Factory is a clear self-explaining name which create an item. If this has been agreed with @ehab-al-cko then I think there's no problem. Below is some of my suggestions feel free to see if they work for you.


Personally I would probably renaming the class to be something like this to make them deadly simple:

So in this case:

val mediatorFactory = CVVComponentMediatorFactoryProvider.create(publicKey, environment, context)
val mediator = mediatorFactory.create(cvvComponentConfig)

// render component
mediator.CVVComponent()

// tokenisation
mediator.createToken(request)

Merchants can easily know the CVVComponentMediatorFactoryProvider provides a CVVComponentMediatorFactory, and CVVComponentMediatorFactory can creates CVVComponentMediators.