Background
We have a request object that contains information. That specific object has a field called partnerId which determines what we are going to do with the request.
A typical approach would be a gigantic if/then/else:
function processRequest( request ){
if( request.partnerId === 1 ){
//code here
}else if( request.partnerId === 23 ){
//code here
}
//and so on. This would be a **huge** if then else.
}
This approach has two main problems:
- This function would be huge. Huge functions are a code smell (explaining why next) but mainly they become very hard to read and maintain very quickly.
- This function would do more than one thing. This is a problem. Good coding practices recommend that 1 function should do only 1 thing.
Our solution
To bypass the previous problems, I challenged my co-worker to come up with a different solution, and he came up with a function that dynamically builds the name of the function we want to use and calls it. Sounds complicated but this code will clarify it:
const functionHolder = {
const p1 = request => {
//deals with request
};
const p23 = request => {
//deals with request
};
return { p1, p23 };
};
const processRequest = request => {
const partnerId = request.partnerId;
const result = functionHolder[`p${partnerId}`](request);
return result;
};
Problems
This solution has advantages over the previous one:
- There is no main function with an huge gigantic if then else.
- Each execution path is not a single function that does one thing only
However it also has a few problems:
- We are using an object
functionHolderwhich is in reality useless.p1andp23don't share anything in common, we just use this object because we don't know how else we can build the function's name dynamically and call it. - There is no
elsecase. If we get an incorrect parameter the code blows. - Out eslint with rule
non-used-varscomplains thatp1andp23are not being used and we don't know how to fix it ( http://ift.tt/2erflr6 ).
The last problem, gives us the impression that perhaps this solution is not so great. Perhaps this pattern to avoid an if then else has some evil to it that we are yet to find.
Questions
- Is there any other pattern we can use to avoid huge if then else statements ( or switch cases )?
- Is there a way to get rid of the
functionHolderobject? - Should we change the pattern or fix the rule?
Looking forward to any feedback!
Aucun commentaire:
Enregistrer un commentaire