Jump to content
Mike Torrettinni

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

Recommended Posts

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?

 

Share this post


Link to post
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?

 

  • Like 2
  • Thanks 1

Share this post


Link to post
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?

Share this post


Link to post

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);

 

  • Like 6

Share this post


Link to post
Posted (edited)

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 by Dalija Prasnikar
  • Like 3
  • Thanks 1

Share this post


Link to post
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.

Share this post


Link to post

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.

  • Like 7

Share this post


Link to post
Posted (edited)

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 by Mike Torrettinni

Share this post


Link to post
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. 

  • Like 3
  • Thanks 1

Share this post


Link to post
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? 🙂

 

Share this post


Link to post
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? 

Share this post


Link to post
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. 

  • Like 2

Share this post


Link to post
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.

Share this post


Link to post
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.

 

  • Haha 3

Share this post


Link to post
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. 

  • Haha 1

Share this post


Link to post
Posted (edited)

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 by Arnaud Bouchez
  • Like 1
  • Thanks 1

Share this post


Link to post
Posted (edited)

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 by Pat Foley
typo
  • Like 1

Share this post


Link to post

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

  • Like 1
  • Thanks 1

Share this post


Link to post

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)

 

  • Like 1

Share this post


Link to post
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.

Share this post


Link to post
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

  • Like 2

Share this post


Link to post
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.

Share this post


Link to post
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 🙂

  • Like 1
  • Haha 1

Share this post


Link to post

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

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

 

  • Like 2

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×