Refactoring a long parameter list of boolean flags
If you have had the need to have a method accept many boolean arguments, you might design the method to accept a number of optional boolean parameters, such as:
public void print(boolean _proforma = false, boolean _hideFooter = false, boolean _hideHeader= false, boolean _archive = false)
This is not the best design. For a start if you are happy to accept all the defaults but want to archive the printout you’ll need to call the method like this:
print(false, false, false, true);
It also suffers from what refactoring practitioners call a “bad smell” – this one being a long parameter list, which makes it harder to read and understand, and gets worse over time as new arguments are added.
A refactoring to improve the design might be to introduce a (single) settings object to replace the parameters.
If you consider that to be overkill you may want to consider this alternative: replace the parameters with a single base Enum type:
Each enumeration element then has a value that maps to the two base binary. So the values for this base enum would be:
- Proforma – 1
- HideFooter – 2
- HideHeader – 4
- Archive – 8
This enables us to declare our print method like so:
public void print(DEV_PrintOptions _opts)
{
if ((_opts & DEV_PrintOptions::Proforma) == DEV_PrintOptions::Proforma)
// Do proforma stuff
if ((_opts & DEV_PrintOptions::HideFooter) == DEV_PrintOptions::HideFooter)
// Hide the footer
if ((_opts & DEV_PrintOptions::HideHeader) == DEV_PrintOptions::HideHeader)
// Hide the header
if ((_opts & DEV_PrintOptions::Archive) == DEV_PrintOptions::Archive)
// Archive the printout
}
Which allows the method to be called in a more flexible way, specifying only the options you wish to override, for example:
// print out proforma and archive it (accepting the default to print the footer and header): print(Dev_PrintOptions::Proforma | Dev_PrintOptions::Archive);

You may simplify testing a bit:
if (_opts & DEV_PrintOptions::Proforma)
// Do proforma stuff
The result will either be 0 (false) or a 1 bit somewhere (true).
Jan B. Kjeldsen
February 26, 2010 at 12:50 pm
I going agree with you that
:print(false, false, false, true);
isn’t very readable at all. But to use a bit mask as parameter is not very intuitive.
Every programmer which sees that syntax
:public void print(DEV_PrintOptions _opts) will think
will be confused, if he doesn’t analyze the method itself to understand he must pass a bitmask.
In such case i rather would suggest to create a parameter object.
But anyway a interesting post.
Patrick
March 3, 2010 at 1:38 pm
Patrick, you raise a good point. If it is used then it needs to be well communicated.
There are examples of this technique being used in standard API’s see The Regexp.Match method overload which uses the RegexOptions enumerator (http://msdn.microsoft.com/en-us/library/system.text.regularexpressions.regexoptions.aspx).
gregondax
March 4, 2010 at 12:52 pm