Multiple boolean arguments - why is it bad?

Question

This seems to be a commonly encountered and brought up code smell, and is a common feature in multiple coding design books which I wasn't fully able to understand.

Say I have a function

def train_model(model,useSGD,initWithZeros):
    
    if initWithZeros:
        model.initWeights(method='zeros')
    else:
        model.initWeights(method='random')

    if useSGD:
        model.train(optimizer='SGD')
    else: 
        model.train(optimizer='Adam')

    return model

Such a function seems to violate the Single Responsibility Principle and does different things based on it's argument flags, which I understand is bad practice.

To resolve this however, how would one refactor this? Do you create 4 functions now?

trainModelWithZerosInitOptimSGD(...)
trainModelWithZerosInitOptimAdam(...)
trainModelWithRandomInitOptimSGD(...)
trainModelWithRandomInitOptimAdam(...)

Isn't this worse for maintenance or readability? Is there any benefit of doing it like this, for this case?

Further, if I want to add a method to save model weights, called model.save_weights(...), I would need to add it to all the 4 functions, as compared to just one place?

Was hoping to learn what a good way would be to deal with these scenarios. Thanks!

Answer 1

The obvious problem

To resolve this however, how would one refactor this? Do you create 4 functions now?

  trainModelWithZerosInitOptimSGD(...)
  trainModelWithZerosInitOptimAdam(...)
  trainModelWithRandomInitOptimSGD(...)
  trainModelWithRandomInitOptimAdam(...)

What you've done here is the equivalent of rewriting:

int Add(int a, int b)

into:

int OnePlusOne()
int OnePlusTwo()
int TwoPlusOne()
int OnePlusThree()
// ...

Which is clearly the opposite of reusability.

What's the point of the advice?

Whenever you deal with any kind of good practice advice, my advice to you is to understand the reason, don't just apply the letter of the rule. Applying things without understanding them leads to many misunderstandings and code that is just as bad as (if not worse than) the original.

So why are multiple boolean arguments bad? Well, very simply put, because it's really difficult to understand the values used in such a method call once it is written.

myModel = train_model(myModel, true, false)

It is not at all obvious what these boolean settings do. Let's use another example. Tell me what we're doing here:

var result = RaiseToPower(2, 3);

Are we calculating 2³ or 3²? Well, you might be able to reason that the left-to-right order is preserved, so (2,3) must mean 2³. Yeah, that sounds reasonable. But what about this?

var result = CalculateRoot(3, 2);

Are we calculating √3 (also written as ²√3) or ³√2? Here, we come to an interesting issue. Either we retain the left-to-right nature of the parameters (which means we're doing ³√2 here), or we retain the "base number, then power" order that we used in our RaiseToPower signature (which means we're doing ²√3 here).

In either case, we are now breaking one of the natural assumptions that we made about RaiseToPower. It's always going to confuse someone.

The key here is one of reasonable readability. If there is a better way to communicate the meaning of the values, then it should be used. The goal here is to maximize readability without making it too hard to write at the same time.

Solving the core problem (solution 1)

For example, C# allows you to name your parameters. This means that you can use the name that is defined in the method signature in the method call, e.g.:

myModel = trainModel(myModel, useSGD: true, initWithZeros: false);

This already resolves the issue of readability without actually getting rid of the boolean parameters themselves. Is it the best solution? Maybe not, but I would consider this sufficiently readable.

A broader best practice solution (solution 2)

However, if focus on other good practice advice, in this case inversion of control, the solution to that would inherently also solve the boolean parameter issue.

Skipping the lesson on inversion of control, the solution entails passing the weight and optimizer into the method, rather than a flag value which then selects the related weights/optimizer:

def train_model(model, weights, optimizer):
    model.initWeights(weights)
    model.train(optimizer)

    # some really fancy logic

    return model

To be honest, your particular example is rendered to nothing when this advice is applied. I added this suggested solution anyway because it is great in scenarios where the method contains more logic than just the flag-checking logic (i.e. the some really fancy logic that I added to the example)

As a matter of fact, your example method's purpose is intrinsically tied to the passing in of boolean flags, to the point where its existence itself violates the boolean argument guideline.

Finding the right pattern (solution 3)

I think in this case the builder pattern is a more appropriate solution for you.

In essence, instead of doing this:

myModel = train_model(myModel, true, false)

Your consumer should be doing something like this:

myTrainedModel = (
    myModel
    .train()
    .withWeights('random')
    .withOptimizer('SGD')
    .run()
)

I have also opted to skip the whole stringly typed parameter discussion here, I'm not a fan of these string values but this is out of scope of the question you asked.

Answer 2

Functions with a bunch of boolean parameters are often "helper" functions, as in your example, that just end up delegating to other functions based on the values of those parameters. You get to write:

train_model(model, true, false)

instead of:

model.initWeights(method='random')
model.train(optimizer='SGD')

At first glance, it seems like the helper function leads to fewer lines of code. However, it's arguably harder to read because I now need to know what each boolean parameter actually does. Named parameters are marginally better, but even so, you're less likely to remember that initWithZeros=false translates to method='random' six months down the line. If you (and especially other people working on your codebase) end up needing to look up what each parameter does every time you call train_model, then that's not a good abstraction. I'd rather just make explicit calls to the model than save a couple lines of code by introducing boolean parameters.

Answer 3

Prioritize client usage for simplicity over implementation issues.

Look to the callers/clients to see how they use these functions. If they all always pass constant values (true/false), then probably best to offer separate methods rather than the boolean parameters.

However, if certain clients pair an SGD and ZeroInit value together and use that for a duration (i.e. multiple times), then create an object that represents the state of SGD and init bound together for the client and pass that as parameter.

Or maybe create an object that represents the model + an sgd + an zeroinit value together, and have the client hold and use that object.  In this scenario the client has to hold onto only one variable, and can forget about the extra parameters during later method calls.

The goal is to simplify things for the consuming client as much as possible, and make things less error prone (as in passing the wrong boolean value in one of several calls).

Answer 4

In my view, if a method does a large, complex thing such as train an entire ML model, then tweaking various little aspects of that thing isn't violating the SRP.

People usually argue against boolean arguments because they make the method call very hard to read. The go-to language for ML (as in your example) is Python, which has named parameters, which takes the force out of that argument. In languages that don't have named parameters, the better solution is usually to declare specific TrainingDetails class with named fields which you can then pass to the training.

Answer 5

If you have a function call

F(true, false, true);

the reader is guessing what these parameters mean. Or checking the documentation. This is error prone and time consuming.

There are languages where parameters are named. If you read

F(visible: true, textVisible: false, changeAll: true);

You have a much much better idea what’s going on. At this point, having multiple functions to call becomes pointless.

Answer 6

I know I'm coming late to this party but I can't help myself.

Regardless of the number, and regardless of the language, even a single Boolean argument has issues. It can force you to write if code rather than use polymorphism. Maybe you like if's. Maybe you don't care about separating code into different files so it can hide from changes that don't impact it. But if you do care understand that a Boolean argument isn't helping.

As for multiple Booleans, yes combinational explosion can quickly make blindly breaking these into separate methods silly. But the refactoring replace conditional with polymorphism isn't about methods. It's about classes. And classes have the power to tame this problem.

def train_model(model, optimizer, init_method)
    model.initWeights(method=init_method)
    model.train(optimizer=optimizer)
    return model

No combinational explosion. Just sending dependencies off to the things that depend on them.

---

I need to clear up another misunderstanding. The builder pattern actually comes in two district kinds. The builder the Gang of Four introduced us to is not the same thing as the builder that Joshua Bloch gave us. The Bloch Builder is a Java thing. It's the one used to simulate named / keyword arguments which you don't need in languages that have them already (like C# or Python).

Even if your language doesn't have named arguments, and even if you've sworn off the Bloch Builder, there's still a simple way to handle the confusion caused by a line of Boolean arguments.

useSGD = true
initWithZeros = false
myModel = train_model(myModel, useSGD, initWithZeros)

But you don't see this very much in Python (or C#) because they have those lovely named arguments.

myModel = train_model(myModel, useSGD=true, initWithZeros=false)

---

Polymorphism is hardly the only way to handle this issue. If these choices are really configuration consider externalizing them in some way like making them Feature Flags. If you really like your ifs consider this method of making your choices:

def train_model(model){
  if featureIsEnabled("initWithZeros"): 
    model.initWeights(method='zeros')
  else:
    model.initWeights(method='random')

  if featureIsEnabled("useSGD"):
    model.train(optimizer='SGD')
  else: 
    model.train(optimizer='Adam')

This method is usually preferred when these choices need to be made with different deployments or builds but by people who'd rather not be messing around in the code.

Answer 7

Your example is basically just boilerplate, or a "macro". It has no function of its own, and no particular benefit.

def train_model(model,useSGD,initWithZeros):

  if initWithZeros:
      model.initWeights(method='zeros')
  else:
      model.initWeights(method='random')

  if useSGD:
      model.train(optimizer='SGD')
  else: 
      model.train(optimizer='Adam')

  return model

As you noticed, this seems kind of superfluous; it combines things that should not be combined. What happens if you add a third optimizer? Not only will you have to touch this method (in addition to whatever else you need), but suddenly you need to change the type of these arguments, and so on and forth.

To resolve this however, how would one refactor this?

Possibility 1

Instead of

train_model(model, true, false)

You could simply write

model.initWeights(method='zeros')
model.train(optimizer='Adam')

There is no particular reason to encapsulate those two calls (unless there actually is a reason you haven't told us about). Assuming all 4 variations of the two booleans in this example are legal, of course. And even if some combination is illegal (for example, forgetting to initialize the weights before calling the train method), you could add exception handling for this case in train.

If you have a great many calls like this in your application, you indeed can write individual methods for it as you mention - which would not be a grand architectural decision, but just a way to make code shorter.

Possibility 2

An alternative would be to do it like this:

def train_model(model, method, optimizer): 
   model.initWeights(method=method)
   model.train(optimizer=optimizer)

If you wish to make it impossible to forget how to write the string, you could switch to constants instead (i.e., have compiler constants METHOD_ZEROS, METHOD_RANDOM and so on and forth).

This still has the drawback of combining things that should maybe not combined together, but would at least be a little better (i.e. less boilerplate code).

Possibility 3

The OO way would be to encapsulate the weights and training optimizer in their own classes (Python: modules), with a common abstract base class. And then use some more or less involved way to instantiate those classes and bring them together, i.e.

weights = ZeroWeights()
training = AdamTraining()
model.train(weights, training)

If those classes basically simply select an algorithm and require no state of their own, you can use the Singleton pattern to avoid creating superfluous objects, as well.

Final words

So it's up to you, there is a plethora of possibilities (and probably even more than I listed). All of them are somewhat a matter of taste, and depend on the usage of your code - whether it's more of a one-off; whether the functions are called millions of times vs. only one over the lifetime of the application, and so on and forth.

Answer 8

Why are boolean arguments bad? At the locations in the source that call methods that have boolean arguments, e.g. train_model(model, true, true), you cannot know at-a-glance what each boolean value means. You either have to remember, or re-check the API every single time. This is tedious and prone to error. Designing an API based on boolean arguments is also inflexible, because one day you may realise one of the parameters needs more than two values, or you may need to add an additional parameter, at which point every single consumer has to be updated too to fit the new signature.

You can replace boolean flag arguments with enum arguments. This makes the forgettable true/false litterings more immediately meaningful and readable:

train_model(model, true, false)
train_model(model, false, true)

etc. becomes

train_model(model, Optimizer.SGD, InitialWeights.ZEROS)
train_model(model, Optimizer.ADAM, InitialWeights.RANDOM)

for example.

I first heard of this solution because of the (in)famous Clean Code by Robert C. Martin, but from looking around online it still seems to be a generally well-liked pattern.

---

However, the best solution for your problem cross-language is normally to pass a configuration/options object that collates all of the configuration parameters like initWithZeros and useSGD, and gives them names and documents them. In Python, kwargs are normally still the best way to do this, it's a bit awkward in this language compared to others to make and use real options objects.

def train_model(model, *, useSGD=true, initWithZeros=true):

interface TrainingOptions {
  useSGD?: boolean;
  initWithZeros?: boolean;
}

function trainModel(model: Model, options: TrainingOptions)

public class TrainingOptions {
    public bool UseSGD { get; set; } = true;
    public bool InitWithZeros { get; set; } = true;
}

public Model(TrainingOptions options)

Answer 9

Your example is probably best handled by a builder of some sort as a couple of people have pointed out. But I’d like to address what is wrong with it.

Parameters are always either (a) data or (b) configuration.

Mixing up the parameter position either way is a problem, although that can mainly be addressed by named arguments in languages that support that.

Regardless, presumably if data, it’s the right data under the circumstance, and can be easily verified, so not really a major problem.

But configuration is another matter, it has several problems with it. It’s more likely to be used for sub-configuration, either alone or in combination with some other value, and it’s inherently exponential. Making it harder to test and verify at best, impossible to fully test at worst.

As configuration, one part of the program is telling another part of the program how to behave. Which introduces a tight coupling between the two. Which means that something which isn’t necessarily up to date or fully knowledgeable about what is the best decision is making it. And that is presuming it was applied to the “right” configuration in the first place. With a boolean, you have a 50% chance of getting the expected behavior if it’s applied to the wrong configuration, making it easy to get wrong and not notice it. The more boolean configurations, the more likely that misapplying it will pass unnoticed.

Answer 10

I think this is less of a problem in modern IDEs and the advice is somehow dated.

Most modern IDEs would show a function call like

train_model(Model,true,false)

actually as

train_model(Model,useSGD:true,initWithZeros:true)

using a different font for useSGD and initWithZeros.

So while on plain text, it is hard to read, it isn't really a problem any more.

Answer 11

I agree with the other answers that the helper/wrapper method might not be necessary at all, calling initWeights() and train() separately is barely longer and much clearer than a single train_model(…) call. The boolean arguments are hard to understand, and naming them doesn't really improve things imo.

I would instead just forward the arguments and add default values:

def train_model(model, initMethod = 'random', optimizer = 'Adam'):
    model.initWeights(method=initMethod)
    model.train(optimizer=optimizer)
    return model

Instead of train_model(…, initWithZeros=true) you'd write train_model(…, initMethod='zeros'), instead of train_model(…, useSGD=true) you'd write train_model(…, optimizer='SGD').