Tip of the Week #94: Callsite Readability and bool Parameters

Originally posted as totw/94 on 2015-04-27

By Geoff Romer ([email protected])

Revised 2017-10-25

“Of the many forms of false culture, a premature converse with abstractions is perhaps the most likely to prove fatal to the growth of … vigour of intellect.” — George Boole.

Suppose you come across code like this:

int main(int argc, char* argv[]) {
  ParseCommandLineFlags(&argc, &argv, false);
}

Can you tell what this code does, and in particular what the last argument means? Now suppose you’ve come across this function before, and you know that the last argument has to do with whether command-line flags are left in argv after the call. Can you tell which is true and which is false?

Of course you don’t know, because this is hypothetical, but even in real code, we have better things to do with our brains than memorize the meaning of every function parameter, and better things to do with our time than go look up the documentation for every function call we encounter. We have to be able to make pretty good guesses about what function calls mean, just from looking at the callsite.

Well-chosen function names are the key to making function calls readable, but they’re often not enough. We usually need the arguments themselves to give us some clues about what they mean. For example, you might not know what to make of absl::string_view s(x, y); if you’d never seen string_view before, but absl::string_view s(my_str.data(), my_str.size()); and absl::string_view s("foo"); are much clearer. The trouble with bool parameters is that the argument at the callsite is very often a literal true or false, and that gives the reader no contextual cues about the meaning of the parameter, as we saw in the ParseCommandLineFlags() example. This problem is compounded if there’s more than one bool parameter, because now you have the additional problem of figuring out which parameter is which.

So how can we fix code like that example? One (bad) possibility is to do something like this:

int main(int argc, char* argv[]) {
  ParseCommandLineFlags(&argc, &argv, false /* preserve flags */);
}

The drawbacks of this approach are evident: it’s not clear if that comment is describing the meaning of the parameter or the effect of the argument. In other words, is it saying that we’re preserving the flags, or is it saying that it’s false that we’re preserving the flags? Even if the comment manages to make that clear, there’s still a risk that the comment will get out of sync with the code.

A better approach is to specify the name of the parameter in the comment:

int main(int argc, char* argv[]) {
  ParseCommandLineFlags(&argc, &argv, /*remove_flags=*/false);
}

This is much clearer, and much less likely to fall out of sync with the code. Clang-tidy will even check that the comment has the correct parameter name. A less ambiguous but longer variant is to use an explanatory variable:

int main(int argc, char* argv[]) {
  const bool remove_flags = false;
  ParseCommandLineFlags(&argc, &argv, remove_flags);
}

However, explanatory variable names are not checked by the compiler, and so they may be erroneous. This is a particular problem when you have multiple bool parameters, which might be transposed by the caller.

All these approaches also rely on programmers to consistently remember to add these comments or variables, and to do so correctly (although clang-tidy will check the correctness of parameter-name comments).

In many cases, the best solution is to avoid using bool parameters in the first place, and use an enum instead. For example, ParseCommandLineFlags() could be declared like this:

enum ShouldRemoveFlags { kDontRemoveFlags, kRemoveFlags };

void ParseCommandLineFlags(int* argc, char*** argv, ShouldRemoveFlags remove_flags);

so that the call could look like this:

int main(int argc, char* argv[]) {
  ParseCommandLineFlags(&argc, &argv, kDontRemoveFlags);
}

You can also use an enum class, as described in TotW 86, but in that case you’d want to use a slightly different naming convention, e.g.:

enum class ShouldRemoveFlags { kNo, kYes };
…
int main(int argc, char* argv[]) {
  ParseCommandLineFlags(&argc, &argv, ShouldRemoveFlags::kNo);
}

Obviously, this approach has to be implemented when the function is defined; you can’t really opt into it at the callsite (you can fake it, but for little benefit). So when you’re defining a function, particularly if it’s going to be widely used, the onus is on you to think carefully about how the callsites will look, and in particular to be very skeptical of bool parameters.