Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesign #110

Closed
StuporHero opened this issue Mar 8, 2017 · 3 comments
Closed

Redesign #110

StuporHero opened this issue Mar 8, 2017 · 3 comments
Labels

Comments

@StuporHero
Copy link

StuporHero commented Mar 8, 2017

This API is needlessly difficult to use. It lacks a coherent abstraction for what it is actually doing. Ideally, this API would work like the following:

AuthorizeAndCaptureResponse authorizeAndCapture(boolean testMode, String userId, String userKey, String cardNumber, Expiration expiration, double amount) {
    try {
        Gateway gateway = new Gateway(testMode);
        Client client = new Client(userId, userKey);

        AuthorizeAndCaptureRequest request = client.getAuthorizeAndCaptureBuilder()
                                                 .setCardNumber(cardNumber)
                                                 .setExpiration(expiration)
                                                 .setAmount(amount)
                                                 .build();

        return (AuthorizeAndCaptureResponse) client.send(request, gateway);
    } catch(ValidationException e) {
        e.printStackTrace();
        return null;
    }
}

Gateway is responsible for processing requests. Client is responsible for providing builders for all of the request types and for sending authenticated requests to Gateway. The Response objects should provide straightforward access to the results by encapsulating their underlying objects rather than the obnoxious layers that we have to try to wrap our heads around now. Objects like Expiration provide more convenient and intuitive abstractions for things like the expiration date with methods like setMonth(int month) and setYear(int year) or similar constructors like Expiration(int month, int year). As a bonus, these kinds of objects can provide early validation to inform users when the data they've entered makes no sense.

Not only will this type of straightforward abstraction be easier for users to work with, but it should also be easier for you to maintain. I'm considering doing some of this work for you since I don't want to put the code I've written against your current API into production because of how hard it is to read and understand.

Also, you SHOULD NOT provide examples of embedding this kind of code into JSPs since that represents a severe violation of the Separation of Concerns. JSPs should only contain presentation logic and should delegate this kind of work to Servlets which should delegate it to Services which bear responsibility for the business logic.

@gnongsie
Copy link
Contributor

Hi @StuporHero, thanks for this insight. We are looking into your suggestions and will get back to you with a decision.

@chsriniv9
Copy link

Please refer to the ChargeCreditCard sample code.
Note that we have simplified the way of creating request, there has been re-design in sdks and we are deprecating the older design (classes)

@kikmak42
Copy link
Contributor

Closed, re-design of the sdk was done to Controllers & Contract based design, and we are in process of deprecating the older version which was referred in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants