Refactor handler signatures to improve readability
As described in https://gitlab.com/stp-team/systemtestportal-webapp/blob/master/docs/adr/0009-dependency-inversion.md#decision-outcome under "Negative consequences", the handlers have a lot of parameters, which leads to unreadable code.
An example for this is the handler for the test execution of a test-case.
func CaseExecutionPost(protocolLister test.ProtocolLister, caseProtocolStore CaseProtocolStore,
sequenceProtocolAdder SequenceProtocolAdder,
progress progressMeter, testSequenceVersion *test.SequenceVersion, testCaseGetter handler.TestCaseGetter,
testCaseLister handler.TestCaseLister, listAdder handler.TaskListAdder, listGetter handler.TaskListGetter,
activityStore handler.Activities, issueGetter handler.IssueGetter, issueAdder handler.IssueAdder) http.HandlerFunc {
...
return func(w http.ResponseWriter, r *http.Request) {
...
}
}
In total, 12 parameters are passed to this function.
Suggestion: Instead of passing the parameters directly to the function, the handlers can be defined as a struct, which contains all the needed Getters, Adders, Listers, etc...
The execution-handler would be defined as:
type CaseExecutionHandler struct {
ProtocolLister test.ProtocolLister
CaseProtocolStore CaseProtocolStore
SequenceProtocolAdder SequenceProtocolAdder
Progress progressMeter
TestCaseGetter handler.TestCaseGetter
TestCaseLister handler.TestCaseLister
ListAdder handler.TaskListAdder
ListGetter handler.TaskListGetter
ActivityStore handler.Activities
IssueGetter handler.IssueGetter
IssueAdder handler.IssueAdder
}
The function to handle the test-case execution would then look like this:
func (caseExecutionHandler *CaseExecutionHandler) ExecutionPost() http.HandlerFunc {
return func(writer http.ResponseWriter, request *http.Request) {
// TODO
}
}
Access to the getters, setters, listers etc. in the handler-function can be made with caseExecutionHandler.ProtocolLister.GetCaseExecutionProtocol()
.
Positive effects:
- Readable code
- When new Getters/Setters are needed in the handlers, they can be easily defined in the struct instead of having to change the parameters in the handler package and in the routing package
- The linter is happy
Negative effects:
- Every handler needs to be changed (a lot of work)
- Handler-tests need to be changed
@niklaskhf You worked with the code for a long time now. Do you think this would make it easier to understand the code and introduce new functionality? I think leaving the handlers like this makes it very, very, very difficult to work with them in the long term.