views:

1942

answers:

24

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?

+22  A: 

Enums also allow for future modifications, where you now want a third choice (or more).

simon
Like FILE_NOT_FOUND?
Outlaw Programmer
hahahahha I was just going to add that :-)
Orion Edwards
Or Win32's GetMessage(): TRUE, FALSE, or -1.
bk1e
+2  A: 

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.

David Basarab
+60  A: 

Boolean's represent "yes/no" choices. If you want to represent a "yes/no", then use a boolean, it should be self-explanatory.

But if it's a choice between two options, neither of which is clearly yes or no, then an enum can sometimes be more readable.

skaffman
In addition, the method name has to be clear about what the argument yes or no does i.e void turnLightOn(bool) clealy setting true or yes will tunr the light on.
simon
Although in this case I'd maybe rather have turnLightOn() and turnLightOff(), depending on the situation.
skaffman
"turnLightOn(false)" means "don't turn light on"? Confusiong.
Jay Bazuzi
What about `changeLightState( bool )` or something then? :P
poke
How about `setLightOn(bool)`.
Finbarr
+12  A: 

I think you almost answered this yourself, I think the end aim is to make the code more readable, and in this case the enum did that, IMO its always best to look at the end aim rather than blanket rules, maybe think of it more as a guideline i.e. enums are often more readable in code than generic bools, ints etc but there will always be exceptions to the rule.

Tim Jarvis
+6  A: 

Enums are better but I wouldn't call boolean params as "unacceptable". Sometimes it's just easier to throw one little boolean in and move on (think private methods etc.)

Borek
just make the method very descriptive so it is clear what true or yes means.
simon
+3  A: 

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.

Jesse C. Slicer
Some day you will need to add "keep writing if you have free space", and then you will go from bool to enum anyway.
Ilya Ryzhenkov
And then you will have breaking change, or have obsolete overload, or may be something like KeppWritingDataEx :)
Ilya Ryzhenkov
@Ilya or maybe you won't! Creating a possible situation where none currently exists does not negate the solution.
Jesse C. Slicer
Jesse's right. Planning for a change like that is silly. Do what makes sense. In this case, the boolean is both intuitive and clear. http://c2.com/xp/DoTheSimplestThingThatCouldPossiblyWork.html
Derek Park
@Derek, in this case, boolean is not even needed, because DataAvailable always returns true :)
Ilya Ryzhenkov
+2  A: 

It depends on the method. If the method does something that is very obviously a true/false thing then it is fine, e.g. below [though not I am not saying this is the best design for this method, it's just an example of where the usage is obvious].

CommentService.SetApprovalStatus(commentId, false);

However in most cases, such as the example you mention, it is better to use an enumeration. There are many examples in the .NET Framework itself where this convention is not followed, but that is because they introduced this design guideline fairly late on in the cycle.

Greg Beech
+2  A: 

IMHO it seems like an enum would be the obvious choice for any situation where more than two options are possible. But there definitely ARE situations where a boolean is all you need. In that case I would say that using an enum where a bool would work would be an example of using 7 words when 4 will do.

Jurassic_C
+1  A: 

Booleans make sense when you have an obvious toggle which can only be one of two things (i.e. the state of a light bulb, on or off). Other than that, it's good to write it in such a way that it's obvious what you're passing - e.g. disk writes - unbuffered, line-buffered, or synchronous - should be passed as such. Even if you don't want to allow synchronous writes now (and so you're limited to two options), it's worth considering making them more verbose for the purposes of knowing what they do at first glance.

That said, you can also use False and True (boolean 0 and 1) and then if you need more values later, expand the function out to support user-defined values (say, 2 and 3), and your old 0/1 values will port over nicely, so your code ought not to break.

Dan Udey
+1  A: 

It does make things a bit more explicit, but does start to massively extend the complexity of your interfaces - in a sheer boolean choice such as appending/overwriting it seems like overkill. If you need to add a further option (which I can't think of in this case), you can always perform a refactor (depending on the language)

Jennifer
What about prepending as a third possible option? ;-))
Yarik
+5  A: 

I would not agree that it is a good rule. Obviously, Enum makes for a better explicit or verbose code at some instances, but as a rule it seems way over reaching.

First let me take your example: The programmers responsibility (and ability) to write good code is not really jeopardized by having a Boolean parameter. In your example the programmer could have written just as verbose code by writing:

dim append as boolean = true
file.writeData( data, append );

or I prefer more general

dim shouldAppend as boolean = true
file.writeData( data, shouldAppend );

Second: The Enum example you gave is only "better" because you are passing a CONST. Most likely in most application at least some if not most of the time parameters that are passed to functions are VARIABLES. in which case my second example (giving variables with good names) is much better and Enum would have given you little benefits.

csmba
Although I agree that boolean parameters are acceptable in many cases, in case of this writeData() example, the boolean parameter like shouldAppend is very inappropriate. The reason is simple: it is not immediately obvious what is the meaning of False.
Yarik
+19  A: 

Use the one that best models your problem. In the example you give, the enum is a better choice. However, there would be other times when a boolean is better. Which makes more sense to you:

lock.setIsLocked(True);

or

enum LockState { Locked, Unlocked };
lock.setLockState(Locked);

In this case, I might choose the boolean option since I think it's quite clear and unambiguous, and I'm pretty sure my lock is not going to have more than two states. Still, the second choice is valid, but unnecessarily complicated, IMHO.

jbourque
in your example i'd rather have two methods. lock.lock() lock.release() and lock.IsSet, but it all depends on what makes the most sense to the consuming code.
Robert Paulson
That's a fair comment, but I think it also illustrates the larger point that there are many ways to model a given problem. You should use the best model for your circumstances, and you should also use the best tools that the programming environment provides to fit your model.
jbourque
I totally agree :) ,I was just commenting on the particular pseudocode offering yet another take. I agree with your answer.
Robert Paulson
+7  A: 

Remember the question Adlai Stevenson posed to ambassador Zorin at the U.N. during the cuban missile crisis?

"You are in the courtroom of world opinion right now, and you can answer yes or no. You have denied that [the missiles] exist, and I want to know whether I have understood you correctly.... I am prepared to wait for my answer until hell freezes over, if that's your decision."

If the flag you have in your method is of such a nature that you can pin it down to a binary decision, and that decision will never turn into a three-way or n-way decision, go for boolean. Indications: your flag is called isXXX.

Don't make it boolean in case of something that is a mode switch. There is always one more mode than you thought of when writing the method in the first place.

The one-more-mode dilemma has e.g. haunted Unix, where the possible permission modes a file or directory can have today result in weird double meanings of modes depending on file type, ownership etc.

Thorsten79
+8  A: 

Booleans may be OK in languages that have named parameters, like Python and Objective-C, since the name can explain what the parameter does:

file.writeData(data, overwrite=true)

or:

[file writeData:data overwrite:YES]
Chris Lundie
IMHO, writeData() is a bad example of using a boolean parameter, regardless of whether named parameters are or are not supported. No matter how you name the parameter, the meaning of False value is not obvious!
Yarik
+6  A: 

There are two reasons I've run into this being a bad thing:

  1. Because some people will write methods like:

    ProcessBatch(true, false, false, true, false, false, true);

This is obviously bad because it's too easy to mix up parameters, and you have no idea by looking at it what you're specifying. Just one bool isn't too bad though.

  1. Because controlling program flow by a simple yes/no branch might mean you have two entirely different functions that are wrapped up into one in an awkard way. For instance:

public void Write(bool toOptical);

Really, this should be two methods:

public void WriteOptical();
public void WriteMagnetic();
  • because the code in these might be entirely different; they might have to do all sorts of different error handling and validation, or maybe even have to format the outgoing data differently. You can't tell that just by using Write(), or even Write(Enum.Optical); (though of course you could have either of those methods just call internal methods WriteOptical/Mag if you want).

I guess it just depends. I wouldn't make too big of a deal about it except for #1.

Sam Schutte
Very good points! Two boolean parameters in one method look terrible, indeed (unless you are lucky to have named parameters, of course).
Yarik
This answer might benefit from some reformatting, though! ;-)
Yarik
+3  A: 

Enums have a definite benefit, but you should't just go replacing all your booleans with enums. There are many places where true/false is actually the best way to represent what is going on.

However, using them as method arguments is a bit suspect, simply because you can't see without digging into things what they are supposed to do, as they let you see what the true/false actually means

Properties (especially with C#3 object initializers) or keyword arguments (a la ruby or python) are a much better way to go where you'd otherwise use a boolean argument.

C# example:

var worker = new BackgroundWorker { WorkerReportsProgress = true };

Ruby example

validates_presence_of :name, :allow_nil => true

Python example

connect_to_database( persistent=true )

The only thing I can think of where a boolean method argument is the right thing to do is in java, where you don't have either properties or keyword arguments. This is one of the reasons I hate java :-(

Orion Edwards
+3  A: 

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?

Robert Paulson
A: 

It really depends on the exact nature of the argument. If it is not a yes/no or true/false then a enum makes it more readable. But with an enum you need to check the argument or have acceptable default behaviour since undefined values of the underlying type can be passed.

CheeZe5
+3  A: 

While it is true that in many cases enums are more readable and more extensible than booleans, an absolute rule that "booleans are not acceptable" is daft. It is inflexible and counter-productive - it does not leave room for human judgement. They're a fundamental built in type in most languages because they're useful - consider applying it to other built-in-types: saying for instance "never use an int as a parameter" would just be crazy.

This rule is just a question of style, not of potential for bugs or runtime performance. A better rule would be "prefer enums to booleans for reasons of readability".

Look at the .Net framework. Booleans are used as parameters on quite a few methods. The .Net API is not perfect, but I don't think that the use of boolean as parameters is a big problem. The tooltip always gives you the name of the parameter, and you can build this kind of guidance too - fill in your XML comments on the method parameters, they will come up in the tooltip.

I should also add that there is a case when you should clearly refactor booleans to an enumeration - when you have two or more booleans on your class, or in your method params, and not all states are valid (e.g. it's not valid to have them both set true).

For instance, if your class has properties like

public bool IsFoo
public bool IsBar

And it's an error to have both of them true at the same time, what you've actually got is three valid states, better expressed as something like:

enum FooBarType { IsFoo, IsBar, IsNeither };
Anthony
A: 

The use of enums instead of booleans in your example does help make the method call more readable. However, this is a substitute for my favorite wish item in C#, named arguments in method calls. This syntax:

var v = CallMethod(pData = data, pFileMode = WriteMode, pIsDirty = true);

would be perfectly readable, and you could then do what a programmer should do, which is choose the most appropriate type for each parameter in the method without regard to how it looks in the IDE.

C# 3.0 allows named arguments in constructors. I don't know why they can't do this with methods as well.

MusiGenesis
An interesting idea. But would you be able to reorder parameters? Omit parameters? How would the compiler know which overload you were binding to if this were optional? Also, would you have to name all parameters in the list?
Drew Noakes
A: 

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.

Drew Noakes
+1  A: 

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.

Pascal Thivent
Doesn't that mean that a function that accepts an ASCII string of length N does 128^N things?
detly
@delty Is this a serious comment? If yes, do you code an if on all possible values of a String often? Is there any possible comparison with the case of a boolean argument?
Pascal Thivent
A: 

Some rules thatyou colleage 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!
Alex Worden
A: 

Booleans values true/false only. So it is not clear what it represent. Enum can have meaningful name, e.g OVERWRITE, APPEND, etc. So enums are better.

fastcodejava