CollActionteam / collaction_backend

Backend code for CollAction
6 stars 4 forks source link

[Research/Discussion] Use method chaining to avoid duplicate logic and unmaintainable code #104

Open rubenhorn opened 2 years ago

rubenhorn commented 2 years ago

From Slack:
Thoughts on using a page out of Java's book with the class StringBuilder (and maybe method chaining) to consistently and maintainably implement request handling on the backend? Example:

func handler(ctx context.Context, req events.APIGatewayV2HTTPRequest) (events.APIGatewayV2HTTPResponse, error) {
   handlerChain := new(HandlerChain)
            .append(validateRequestMyHandler) // Check parameters
            .append(authentiate) // (optional) Store user info in ctx
            .append(processRequestMyHandlerPart1)
            .append(processRequestMyHandlerPart2) // There could be multiple steps to processing a request
            .append(processRequestMyHandlerPart3)
   return handlerChain.handle(ctx, req)
}

Every handler only invokes the next one if there is no error, otherwise it returns the error response. This way we can return an error using shared code without adding unrelated logic to the handlers.

Please discuss! 🙂

Qalifah commented 2 years ago

Sounds good. From the example you gave, I noticed the *MyHandler type is not used within the method, where does it come in?

rubenhorn commented 2 years ago

Sounds good. From the example you gave, I noticed the *MyHandler type is not used within the method, where does it come in?

It's not required. I updated the example code.
Thanks for pointing it out.

mrsoftware commented 2 years ago

this is a good idea, but what is wrong with the current approach?

rubenhorn commented 2 years ago

I think this might help reduce duplicate code or errors when doing generic stuff such as acquiring user info (aka authentication/authorization) or API version validation, since it would contain it in a single function call respectively.

Currently we do have to put

user, error := getUser();
if(error) {
   return getHttpErrorResponse(error);
}
//...

...which may be less readable/maintainable.

mrsoftware commented 2 years ago

it is a valid reason, but the common approach for this kind of work is middleware. your suggestion is like middleware but middleware is before the handler not inside the handler.

rubenhorn commented 2 years ago

Yes, that was the idea, but I am not sure if this is supported by AWS SAM.

mrsoftware commented 2 years ago

Yes, that was the idea, but I am not sure if this is supported by AWS SAM.

I don't know, but If was sam not supporting middleware we can create a structure like a method Channing/middleware in the main function, out side handler for auth or ...