Currently reading Clean Code, and the authors seem to imply that any function with an arity greater than 3 should be avoided at all costs.
I am unfortunately running into an issue where I am faced with a class method with an arity of 4 (5 if you count self)
def start_job(self, operation, name, external_id, format):
...
All of the parameters can vary, and are necessary to start the job. However, if I think about encapsulating all of those parameters in some sort of class
class Job(object):
def __init__(self, operation, name, external_id, format):
but then all of those parameters would be necessary for the __init__, and would still need to expose the underlying structure of the object to the start_job function. At this point, it would seem like feature envy as well and would make sense to change the start_job function over to the Job class and just call it start.
Any ideas on how to avoid this smell?
1
This really isn’t answerable without knowing the exact case you are trying to solve. The reason they call them “code smells” is not because the “smell” itself is bad but rather because it indicates that there is something else wrong, a metaphorical pile of crap underneath generating that smell.
As such, the answer generally is not to directly attack the smell itself. That’s no better than spraying the crap with lysol. The answer is to try to figure out what in the underlying design pushed you to use a method with four parameters. Maybe examining the smell will bring this out, or maybe you’ll decide that it was just a whiff of compost and maybe not so bad.
In this case, four parameters isn’t much of a stink. It’s when you reach ten or fifteen that things stink to high heaven.
One way to see if this is a real issue is to look at the code inside the function. Is it a set of elif blocks, basically a case statement controlled by one of your parameters? If so, maybe the method should be broken up along those lines.
Are you passing the same set of parameters to a set of methods? Then maybe you should have a class controlling those values with members for each of the original methods.
Are some of the parameters combined in particular ways inside the method to make a single value? Maybe it could be pulled up out of the function. (For instance, are you creating a filename that looks like name + ‘.’ + extension where extension depends on format?
Does a parameter almost always take a particular value? Then make it a default. While not reducing the “arity”, it does reduce the cognitive load required to use the function.
1
If all of these parameters are always required (never optional), vary independently, have no sensible sub groups, and have widely varying values then it will be tough to do better than you’ve already done. Named Arguments can make the long list less confusing to read.
If that’s not exactly the case then there may be room for improvement. Ask yourself:
Are all the parameters required or are some optional? If optional they should have a default. Make attributes and use a property or setter to change the default. Only keep required ones in __init___.
Are any of the parameters related? If I know two of them can I do anything useful? If so make an object for them that does this useful thing and see if it can also help you do your thing.
If I know one of them can I learn what the other is? If so you’re probably providing redundant information. Figure out what you can live without.
Do any of the parameters have a limited number of states? For example are there only 5 things operation can be? Maybe 5 methods would be a better idea, or even 5 classes.
There are a lot of patterns that can help with this problem. The GoF construction patterns, in particular builder and parameter object. Also the python version of Josh Bloch’s builder might help. Whether it’s 4 or 14 parameters you should always look for a way to simplify how your code is used.
Do the members of Job logically cohere into an object? Yes.
What can you do with a Job? Start it, stop it, query its state.
Sounds like you’ve got a bona fide object there with start(), stop(), query() instance methods.