Greg On Dynamics Ax

Random thoughts about development using Dynamics AX and .Net

Refactoring a long parameter list of boolean flags

with 3 comments

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);
About these ads

Written by gregondax

February 26, 2010 at 9:15 am

3 Responses

Subscribe to comments with RSS.

  1. 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

  2. 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


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Follow

Get every new post delivered to your Inbox.

%d bloggers like this: