Is it really that bad to Use boolean parameters to make your functions do two things?

[Mike Torrettinni 198]

Just interested how others view this implementation:

processOrder(dontUseWidget: Boolean) {
  if not dontUseWidget then {
    processOrderWithWidget();
  } else {
    processOrderWithoutWidget();
  }
}

Nick Hodges describes here: https://medium.com/nickonsoftware/twenty-one-ways-to-be-a-crappy-software-developer-c69e4b39c5df- see #20 Use boolean parameters to make your functions do two things

 

I use such code similar as abode, or in examples like:

procedure EnableDisableControls(aEnabled: boolean = True);
begin
  MainForm.Control.Enabled := aEnabled;
  ...
end;

and a few other instances.

 

I'm just curious how others see this example, is anybody else using it or is it really that bad example that it should be nuked out of any project?

 

[Anders Melander 1783]

10 minutes ago, Mike Torrettinni said:

processOrder(dontUseWidget: Boolean)

Apart from the braces I don't see a problem with that example. It's a convenience or wrapper function and while it's not clear from the context the function doesn't appear to do two completely different things based on the value.

 

5 minutes ago, Mike Torrettinni said:

procedure EnableDisableControls(aEnabled: boolean = True);

A kitten dies every time you do that.

 

Remember that anyone reading the code that uses the function will have no idea that the parameter name makes the intent clear.

EnableDisableControls(True); // WTH?
EnableDisableControls(False); // WTF?

 

[Mike Torrettinni 198]

5 minutes ago, Anders Melander said:

Apart from the braces I don't see a problem with that example. It's a convenience or wrapper function and while it's not clear from the context the function doesn't appear to do two completely different things based on the value.

 

A kitten dies every time you do that.

 

Remember that anyone reading the code that uses the function will have no idea that the parameter name makes the intent clear.

EnableDisableControls(True); // WTH?
EnableDisableControls(False); // WTF?

 

I'm not sure I see the distinction between Nick's example and mine. What am I not seeing?

[Lars Fosdal 1792]

Two things:

1. I really try my best to avoid negations in booleans

2. I try to name methods that do stuff like verbs and where possible put the parameter in context of the name

 

Hence

processOrder(aUseWidget: Boolean) {
  if aUseWidget then {
    processOrderWithWidget();
  } else {
    processOrderWithoutWidget();
  }
}

And

SetControlsEnabled;  // Implicit True
SetControlsEnabled(True); // Explicit
SetControlsEnabled(False);

 

[Dalija Prasnikar 1396]

There are two problems with using Boolean parameters.

 

First, is when like in Nick's example you are literally calling two separate functions to do something based on boolean parameter. If there is common logic shared inside the method, where making two distinct methods would violate DRY principle, then it is fine to have parameter that will make your method do two things.

 

Another problem using Boolean parameters, when you have valid reason to make method do two things is that boolean on its own is very rarely descriptive. 

 

EnableDisableControls(True);

Does the above call enable or disable controls? 

 

If you need to make method do two things, it is better to use enum with two values that will give you more descriptive code on the call site.

 

In your example if the method would be called EnableControls, then using Boolean parameter would be self descriptive. It is pretty obvious what following line does:

 

EnableControls(True);

 

On the other hand boolean in following method makes code unreadable

ListFiles(const APath: string; IncludeSubFolders: Boolean) 

 

Can you tell what following code does, unless you know ListFiles declaration by heart?

ListFiles(Path, True);

 

In such case enum, is much better option even though it would have only two states.

 

ListFiles(Path, optIncludeSubFolders);

 

Edited January 8, 2021 by Dalija Prasnikar

[Lars Fosdal 1792]

Two books that are gold for picking up good coding habits - even this long after they were written:

Code Complete 2nd Edition by Steve McConnell

Framework Design Guidelines by Krzysztof Cwalina & Brad Abrams (Third Edition)

[Mike Torrettinni 198]

20 minutes ago, Dalija Prasnikar said:

In such case enum, is much better option even though it would have only two states.

 

ListFiles(Path, optIncludeSubFolders);

That looks like a sensible solution... I'm just thinking out loud: so many new enums need to be created. For example EnableControls, EnableMenus, EnableFeatures... they all accept boolean, and I do understand the purpose of using enums, but I have to define new enum type for every single True/false parameter.

[David Heffernan 2345]

Use some judgement. You don't need to get rid of all booleans. Code like:

SetControlsEnabled(True);
SetMenusEnabled(True);
SetFeaturesEnabled(True);

is perfectly fine. Does it make sense when you read it? Yes, of course it does.

 

What you need to watch out for is code like:

EnumerateFrogs(True);

Nobody reading that can infer what the argument does.

[Mike Torrettinni 198]

Thanks all for the feedback I already started making some naming changes!

It was just interesting that Nick put this into a blog like it's some really bad practice, while is more about naming than anything else.

 

Edited January 8, 2021 by Mike Torrettinni

[David Heffernan 2345]

10 minutes ago, Mike Torrettinni said:

Thanks all for the feedback I already started making some naming changes!

It was just interesting that Nick put this into a blog like it's some really bad practice, while is more about naming than anything else.

 

It feels like Nick put this example into the blog just for the sake of something to write, to get to magical over 20, but not even number. 😉

Nick's point is fine. I think you just misunderstood it to mean that you needed to replace all boolean args.

 

Following somebody else's recipe without really grasping the issue won't make your code any better. It will just give you work to do and make it likely that you will introduce bugs to your code? If you don't have comprehensive tests for all of your code then you are in big danger of that. 

[Mike Torrettinni 198]

1 minute ago, David Heffernan said:

Nick's point is fine. I think you just misunderstood it to mean that you needed to replace all boolean args.

 

Following somebody else's recipe without really grasping the issue won't make your code any better. It will just give you work to do and make it likely that you will introduce bugs to your code? If you don't have comprehensive tests for all of your code then you are in big danger of that. 

I agree, but it's Nick Hodges... how are you not gonna follow his advice, even if it's a little out-there? 🙂

 

[David Heffernan 2345]

8 minutes ago, Mike Torrettinni said:

how are you not gonna follow his advice

You weren't following his advice. You misunderstood what he said.

 

I already said this. Did you also misunderstand what I said? 

[David Heffernan 2345]

10 minutes ago, Mike Torrettinni said:

it's Nick Hodges

Names mean nothing. You have to understand what people say, and judge it for yourself. Stop following people blindly based on reputation. Try to develop your own critical assessment of what you read. 

[Mike Torrettinni 198]

2 minutes ago, David Heffernan said:

You weren't following his advice. You misunderstood what he said.

 

I already said this. Did you also misunderstand what I said? 

True, that's why I opened this topic, to get some clarification, some sense of it.

[Attila Kovacs 629]

Quote

Twenty-one ways to be a Crappy Software Developer
#20 Use boolean parameters to make your functions do two things

*YAAAWWWN*

    with GzipFile(fileobj=output_file, mode='wt', compresslevel=9) as gzip_out:
        gzip_out.write(contents)

Sounds like a rule from the one thousand fifty nine thing where Delphi still sucks blog.

 

[David Heffernan 2345]

2 minutes ago, Mike Torrettinni said:

True, that's why I opened this topic, to get some clarification, some sense of it.

It's strange then that you don't seem to read what people post. I get a sense sometimes that you ask questions but have already decided what the answer is. 

[Arnaud Bouchez 407]

If a method does two diverse actions, define two methods.

 

If a method performs an action which is something on/off or enabled/disabled then you can use a boolean, if the false/true statement is clearly defined by the naming of the method.

 

If a method performs something, but with a custom behavior, don't use boolean (or booleans) but an enumeration or even better a set.

It will be much more easy to understand what it does, without looking into the parameter names, and it will be more open to new options/behaviors.

 

function TMyObject.SaveTo(json: boolean): string;
// what is the behavior with json=false?

function TMyObject.SaveToJson(expanded: boolean): string;
// what does SaveToJson(true/false) mean without knowing the parameter name?

function TMyObject.SaveToJson(expanded, usecache: boolean): string;
// what does SaveToJson(true/false, true/false) mean without knowing the parameters names?

type
  TMyObjectSaveToJsonOptions = set of (sjoExpanded, sjoUseCache);
  
function TMyObject.SaveToJson(options: TMyObjectSaveToJsonOptions): string;
// you understand what does SaveToJson([]) or SaveToJson([sjoExpanded]) or SaveToJson[sjoExpanded, sjoUserCache]) mean

 

Edited January 8, 2021 by Arnaud Bouchez

[Pat Foley 51]

I think using verbs is better.

 

//old way

procedure runTimer(bRuntimer);
  timer.enabled := bRuntimer;

// better two procedures
Procedure Run
   timer.enabled := True;

Procedure Stop
   timer.enabled := False;

// later expand the functionality
Procedure Step
   timer.enabled := False
   timer.timer  // to walk through code  
// 
// 

Nick Hodges in his book says always program against an interface.  I say always program against a 5 inch X 3 inch UX or 3 X 5 UI.

 

Also consider this for codebase

  T: Boolean

Inside procedure What

      T := T  and B or A

 Doing this preserves the T incoming state.  This latching allows T's state set elsewhere be used reducing the tests in What.       

 

Edited January 8, 2021 by Pat Foley
typo

[Stano 143]

I also highly recommend the book "Code Complete 2nd Edition by Steve McConnell"

[Fr0sT.Brutal 900]

I use Booleans as options for functions from time to time. Yes they add some guess work but defining a special enum type for every function is too hard for me. I hope I'll never deal with a library that would force me to do something like

FindFiles('c:\', TFindFilesOptions.Subdirs)

 

[David Heffernan 2345]

8 minutes ago, Fr0sT.Brutal said:

FindFiles('c:\', TFindFilesOptions.Subdirs)

You really think that an enumerated by, with a meaningful name, carries more information than:

FindFiles('c:\', True);

I think you are in a large minority if that is your view.

[Fr0sT.Brutal 900]

5 minutes ago, David Heffernan said:

You really think that an enumerated by, with a meaningful name, carries more information than:

FindFiles('c:\', True);

I think you are in a large minority if that is your view.

Hmm I guess some of us has had too little sleep at night because I don't get your point. Of course properly named parameters of enum type carry more info than plain boolean. But they involve too much overhead and pollute namespace so I prefer a little bit of mystery the booleans bring

[Mike Torrettinni 198]

3 hours ago, Fr0sT.Brutal said:

Of course properly named parameters of enum type carry more info than plain boolean. But they involve too much overhead and pollute namespace so I prefer a little bit of mystery the booleans bring

I agree. if It's a simple standalone method, I don't bother to create it's own enum, like for Enable/Disable Control. But when part of a feature, in it's own unit(s), then I can add a few enums without even blinking, of course most of them are of limited scope within a feature.

[Fr0sT.Brutal 900]

12 hours ago, Mike Torrettinni said:

I can add a few enums without even blinking, of course most of them are of limited scope within a feature

You mean something like

TFileSearcher.FindFiles('c:\', TFileSearcher.TFindFilesOptions.Subdirs);

? God when ppl ask for adding some features from other languages to Delphi I guess they don't mean this Javism 🙂

[Lars Fosdal 1792]

After doing a lot of SQL, I find myself wanting to name parameters in Delphi too...

TFileSearcher.FindFiles('c:\', aRecursive := True);