Boolean method parameters are evil. This is the eighth in a series of posts on Code Gardening - the practice of making small improvements as you see them while you are working.
Boolean values are extremely useful - we use them all the time.
Yet I’ve become firmly convinced that they’re overused and that in some places they’re a very bad idea.
Consider this method, from when we discussed code documentation:
This method is easy enough to read, but think about the case when the method is consumed:
Without referring to the actual implementation, the meaning of true is completely opaque. You either magically know what it means, or you have to interrupt your current task to go and find out.
I’ve found this issue crops up almost without fail when methods have bool parameters. There are exceptions, of course, but a bool argument is a strong code smell that should be addressed in the majority of cases.
When there is a single bool parameter, the most likely solution is to create two variations of the method with names that reflect their differing behavior.
First, here’s a variation for when
throwIsMissing is false - note both the new name of the
method to indicate that it might not return a course and the presence of code documentation:
Secondly, here’s a variation for when
throwIfMissing is true, again with an indicative name:
This second method implementation is in terms of the first, guaranteeing consistency of behavior both now and in the future. A completely independent implementation would run the risk of the methods diverging in the future as the system evolved.
With those two methods in place, consumers of the methods are explicit about the required behaviour:
As I’ve discussed this idea with various developers, I’ve found general agreement on the principle but a lot of disagreement on method naming.
Here, I’ve used
FindOptionalCourseByCode, but some dislike those
names because they might be ambiguous, as though “Mandatory Course” and “Optional Course”
are different kinds of course.
A suggested alternative would be more explicit, using names like
One team that I worked on had a global convention that Get methods were for when the thing must
already exist, and Find methods were for when the thing might not exist - they would have used
Another team had a similar convention, but based on the terms Search and Load (attempting
to avoid the issue of developers treating find and get as synonyms). They would have used
Regardless of the naming standard you choose, removing a bool parameter by replacing the method with two related variations will improve your code and make it easier to read and maintain.