Are booleans as method arguments unacceptable? [cl

Question

A colleague of mine states that booleans as method arguments are not acceptable. They shall be replaced by enumerations. At first I did not see any benefit, but he gave me an example.

What's easier to understand?

file.writeData( data, true );

Or

enum WriteMode {
  Append,
  Overwrite
};

file.writeData( data, Append );

Now I got it! ;-)
This is definitely an example where an enumeration as second parameter makes the code much more readable.

So, what's your opinion on this topic?

Answer 1

To me, neither using boolean nor enumeration is a good approach. Robert C. Martin captures this very clearly in his Clean Code Tip #12: Eliminate Boolean Arguments:

Boolean arguments loudly declare that the function does more than one thing. They are confusing and should be eliminated.

If a method does more than one thing, you should rather write two different methods, for example in your case: file.append(data) and file.overwrite(data).

Using an enumeration doesn't make things clearer. It doesn't change anything, it's still a flag argument.

Answer 2

If the method asks a question such as:

KeepWritingData (DataAvailable());

where

bool DataAvailable()
{
    return true; //data is ALWAYS available!
}

void KeepWritingData (bool keepGoing)
{
   if (keepGoing)
   {
       ...
   }
}

boolean method arguments seem to make absolutely perfect sense.

Answer 3

Some rules that your colleague might be better adhering to are:

• Don't be dogmatic with your design.
• Choose what fits most appropriately for the users of your code.
• Don't try to bash star-shaped pegs into every hole just because you like the shape this month!

Answer 4

A Boolean would only be acceptable if you do not intend to extend the functionality of the framework. The Enum is preferred because you can extend the enum and not break previous implementations of the function call.

The other advantage of the Enum is that is easier to read.

Answer 5

Enums can certainly make the code more readable. There are still a few things to watch out for (in .net at least)

Because the underlying storage of an enum is an int, the default value will be zero, so you should make sure that 0 is a sensible default. (E.g. structs have all fields set to zero when created, so there's no way to specify a default other than 0. If you don't have a 0 value, you can't even test the enum without casting to int, which would be bad style.)

If your enum's are private to your code (never exposed publicly) then you can stop reading here.

If your enums are published in any way to external code and/or are saved outside of the program, consider numbering them explicitly. The compiler automatically numbers them from 0, but if you rearrange your enums without giving them values you can end up with defects.

I can legally write

WriteMode illegalButWorks = (WriteMode)1000000;
file.Write( data, illegalButWorks );

To combat this, any code that consumes an enum that you can't be certain of (e.g. public API) needs to check if the enum is valid. You do this via

if (!Enum.IsDefined(typeof(WriteMode), userValue))
    throw new ArgumentException("userValue");

The only caveat of Enum.IsDefined is that it uses reflection and is slower. It also suffers a versioning issue. If you need to check the enum value often, you would be better off the following:

public static bool CheckWriteModeEnumValue(WriteMode writeMode)
{
  switch( writeMode )
  {
    case WriteMode.Append:
    case WriteMode.OverWrite:
      break;
    default:
      Debug.Assert(false, "The WriteMode '" + writeMode + "' is not valid.");
      return false;
  }
  return true;
}

The versioning issue is that old code may only know how to handle the 2 enums you have. If you add a third value, Enum.IsDefined will be true, but the old code can't necessarily handle it. Whoops.

There's even more fun you can do with [Flags] enums, and the validation code for that is slightly different.

I'll also note that for portability, you should use call ToString() on the enum, and use Enum.Parse() when reading them back in. Both ToString() and Enum.Parse() can handle [Flags] enum's as well, so there's no reason not to use them. Mind you, it's yet another pitfall, because now you can't even change the name of the enum without possibly breaking code.

So, sometimes you need to weigh all of the above in when you ask yourself Can I get away with just an bool?

Answer 6

Sometimes it's just simpler to model different behaviour with overloads. To continue from your example would be:

file.appendData( data );  
file.overwriteData( data );

This approach degrades if you have multiple parameters, each allowing a fixed set of options. For example, a method that opens a file might have several permutations of file mode (open/create), file access (read/write), sharing mode (none/read/write). The total number of configurations is equal to the Cartesian products of the individual options. Naturally in such cases multiple overloads are not appropriate.

Enums can, in some cases make code more readable, although validating the exact enum value in some languages (C# for example) can be difficult.

Often a boolean parameter is appended to the list of parameters as a new overload. One example in .NET is:

Enum.Parse(str);  
Enum.Parse(str, true); // ignore case

The latter overload became available in a later version of the .NET framework than the first.

If you know that there will only ever be two choices, a boolean might be fine. Enums are extensible in a way that won't break old code, although old libraries might not support new enum values so versioning cannot be completely disregarded.

---

EDIT

In newer versions of C# it's possible to use named arguments which, IMO, can make calling code clearer in the same way that enums can. Using the same example as above:

Enum.Parse(str, ignoreCase: true);