This follows my post on unnecessary if statements
, and broaches a bigger evil. As Uncle Bob says at the end of this article
: “Boolean arguments loudly declare that the function does more than one thing. They are confusing and should be eliminated.” Same verdict should apply for an enum as a method parameter.
A method with Boolean parameters makes my heart sink – one is bad, more than one says the containing class/app needs urgent surgery it probably will never get. It’s usual to see multiple enum parameters in the same or other methods nearby – after all if Boolean are OK as parameters, you need something for when the conditions exceed two – enum to the rescue!
The defence of this anti-pattern I’ve come across are along the lines of convenience and “well named Boolean and enum arguments are acceptable”. The convenience factor, for me at least, is heavily outweighed by breakage of all sorts of best practices, starting from the S in SOLID principles
. Anyone who cares for their code and wants to save time whenever the method is changed in future, has to make sure that the method is coherent and easily testable. And that is usually not possible when someone starts using Booleans or worse, enums as parameters.As for the proper names, those will probably be outdated by the time the method goes in production. And even with proper names, if they exceed 2/3, just keeping track of the arguments and changes to their values is a big task when tracing the usage through classes. The usage of the arguments will quickly exceed the original plan, if there was one, so the name will make less and less sense to anyone trying to grok
the method/class. It’s difficult to keep track of those usages since the method will usually span multiple pages, sometimes assisted by private methods – after all it has a lot on its plate, which is why it has the Boolean and/or enum parameters.
But above all else, the method (and the class) need refactoring if it has the above mentioned malaise. The time wasted working out what and where the changes are to made when they are needed, has to be experienced to be believed. Having personally seen humongous methods with multiple Boolean AND enum arguments, I can assure you none of those were fully tested/testable and were a pain to work on, every time. Unit tests, if there are any, are quite often as unwieldy as the methods themselves. And herein lies the tragedy – the person aware of these shortcomings may usually arrive on the scene with not enough time/business knowledge/mandate for a rewrite.
To counter such disasters from rolling on or unfolding, be disciplined enough to put in the effort when the first condition raises it’s head. Put in some class to handle the state / behaviour / whatever needs changing, so that the parent class does not have to bother with the logic. It can tell another class to do it’s job. This is possible in domain objects too
– something I’ve found developers reluctant to do. While doing this type of work, please remember object orientation does not stop at inheritance – design patterns are your biggest pals. I regularly open up this webpage
when I have worked out the behaviour needed, just to make sure I am not missing something.When I come across such a method already in use, and lacking the time to do a full refactor, I settle for a partial refactoring. That can easily be justified, if needed, as necessary to ensure that the work I need to do needs to be tested properly. The partial refactor consists of a helper class to pull out the code from the method or area of the method that has to take the new work. The helper class can be injected if possible or a property, instantiated if null. So the original method is still doing what it was doing earlier and calling the new class for the new functionality, where I can write testable code.In some cases, this process has gone slightly differently – the current code has been pulled out of the existing method into the helper class so it only contains a shell where the new code can be brought in and easily tested, with a confirmatory call made to the mock of the helper to ensure the old functionality remains intact.
If time permits, the next intermediary step is to change the signature of the method to take a model instead of multiple parameters. This can be time consuming but has numerous advantages, a topic for a future post. The simplest one is that new additions to logic in the old way too doesn’t change the signature of the method – the model gets a new property. The model can also take logic and be validated separately from parent method. These can clean up a lot of the original code and make it easier to understand.
This process should continue when time permits, and culminate after a proper refactoring that obviates the use of Booleans/enums.