How to improve the code around functions like:
RenderGlyphs(true, false, true, false);
It's not easy to reason about those true/false params.
We'd like not only to have more expressive code but to have safer code.
For example, what if you mix two parameters and change their order? The compiler won't help you much!
We could add comments:
RenderGlyphs(glyphs,
/*useChache*/true,
/*deferred*/false,
/*optimize*/true,
/*finalRender*/false);
And although the above code is a bit more readable, still we don't get any more safety.
So what can we do more?
Note: The article originally eppeared on my blog as: Bartek's coding blog: On Toggle Parameters.
Ideas
Here are some ideas that you can use to make such code better:
Small Enums
In theory we could write the following declarations:
enum class UseCacheFlag { True, False };
enum class DeferredFlag { True, False };
enum class OptimizeFlag { True, False };
enum class FinalRenderFlag { True, False };
// and call like:
RenderGlyphs(glyphs,
UseCacheFlag::True,
DeferredFlag::False,
OptimizeFlag::True,
FinalRenderFlag::False);
Using enums is a great approach but have some disadvantages:
- A lot of additional names required!
- Maybe we could reuse some types, should we have some common flags defined in the project? how to organize those types?
- Values are not directly convertible to bool, so you have to compare against
Flag::Trueexplicitly inside the function body.
The required explicit comparison was the reason Andrzej wrote his own little library that creates toggles with conversion to bool.
Initially, I though it's a disappointing that we don't have direct support from the language. But after a while, I changed my mind. The explicit comparison is not that hard to write, so maybe it would be overkill to include it in the language spec? Introducing explicit casts might even cause some problems.
Still, I am not quite happy with the need to write so many tiny enums... And since I am lazy I probably won't apply such rule to all of my code :)
Param Structure
If you have several parameters (like 4 or 5, depends on the context) why don't wrap them into a separate structure?
struct RenderGlyphsParam
{
bool useCache;
bool deferred;
bool optimize;
bool finalRender;
};
void RenderGlyphs(Glyphs &glyphs, const RenderGlyphsParam &renderParam);
// the call:
RenderGlyphs(glyphs,
{/*useChache*/true,
/*deferred*/false,
/*optimize*/true,
/*finalRender*/false});
OK... this didn't help much! You get additional code to manage, and the caller uses almost the same code.
So why could it help?
- It moves the problem to the other place. You could apply strong types to individual members of the structure.
- If you need to add more parameters then you can just extend the structure.
- Especially useful if more functions can share such param structure.
BTW: you could put the glyphs variable also in the RenderGlyphsParam, this is only for example.
Eliminate
We could try to fix the syntax and use clever techniques. But what about using a simpler method? What if we provide more functions and just eliminate the parameter?
It's ok to have one or maybe two toggle parameters, but if you have more maybe it means a function tries to do too much?
In our simple example we could try:
RenderGlyphsDeferred(glyphs,
/*useChache*/true,
/*optimize*/true);
RenderGlyphsForFinalRender(glyphs,
/*useChache*/true,
/*optimize*/true;
We can make the change for parameters that are mutually exclusive. In our example, deferred cannot happen together with the final run.
You might have some internal function RenderGlyphsInternal that would still take those toggle parameters (if you really cannot separate the code). But at least such internal code will be hidden from the public API. You can refactor that internal function later if possible.
So I think it's good to look at the function declaration and review if there are mutually exclusive parameters. Maybe the function is doing too much? If yes, then cut it into several smaller functions.
After writing this section I've noticed a tip from Martin Fowler on Flag Arguments. In the text he also tries to avoid toggles.
You can also read this article from Robert C. Martin's Clean Code Tip #12: Eliminate Boolean Arguments. And more in his book Clean Code: A Handbook of Agile Software Craftsmanship
What's in future C++2z?
There's a paper: Designated Initialization, P0329R0 that might go into C++20.
Basically you could use similar approach as in C99 and name arguments that you pass to a function:
copy(.from = a, .to = b);
There's even a CLang implementation already, see this: Uniform designated initializers and arguments for C++.
Stronger Types
Using small enums or structures is a part of a more general topic of using Stronger Types. Similar problems might appear when you have several ints as parameters or strings...
You can read more about:
- Simplify C++: Use Stronger Types! -
- Type safe handles in C++ — I Like Big Bits
- Strong types for strong interfaces - Fluent C++
- foonathan::blog() - Type safe - Zero overhead utilities for more type safety
- Serialization - BOOST_STATIC_WARNING
One example
Recently, I had a chance to apply some ideas of enum/stronger types to my code. Here's a rough outline:
// functions:
bool CreateContainer(Container *pContainer, bool *pOutWasReused);
void Process(Container *pContainer, bool bWasReused);
// usage
bool bWasReused = false;
if (!CreateContainer(&myContainer, &bWasReused))
return false;
Process(&myContainer, bWasReused);
Briefly: we create a container, and we process it. But the container might be reused (some pool, reusing existing object, etc., some internal logic).
I thought that it doesn't look nice. We use one output flag and then it's passed as input flag to some other function.
What's more, we pass a pointer, so some additional validation should happen. Also, the output parameters are discouraged in modern C++, so it's not good to have them anyway.
How can we do better?
Let's use enums!
enum class ContainerCreateInfo { Err, Created, Reused };
ContainerCreateInfo CreateContainer(Container *pContainer);
void Process(Container *pContainer, ContainerCreateInfo createInfo);
// usage
auto createInfo = CreateContainer(&myContainer)
if (createInfo == ContainerCreateInfo::Err);
return false;
Process(&myContainer, createInfo);
Isn't it better?
There are no outputs via pointer stuff here; we have a strong type for the 'toggle' parameter.
Also, if you need to pass some more information in that CreateInfo enum you can just add one more enum value and process it in proper places, the function prototypes don't have to change.
Of course in the implementation, you have to compare against enum values (not just cast to bool), but itis not difficult and even more verbose.
Summary
Toggle params are not totally wrong, and it's probably impossible to avoid them entirely. Still, it's better to review your design when you want to add third or fourth parameter in a row :)
Maybe you can reduce the number of toggles/flags and have more expressive code?
Some questions to you:
Do you try to refactor toggle parameters?
Do you use strong types in your code?
Call To Action
If you like more programming stories from me, just signup to my weekly newsletter: Bartek's coding blog newsletter
(You'll also get some C++17 bonuses)
Top comments (1)
Great article!
In my last project, we used tagged types to further discriminate types of Strings from each other. The Implementation (in Scala) is straightforward:
We declared them with type aliases to have more human readable types. Taking your examples, that might look like this:
They still can be used like values of the base type:
and here's how we used them with literals:
In case you're unfamiliar with scala, the
asmethod is the one defined in the implicit classTagWrapper. During compile time, this gets translated into completely type safe code.We made good experiences with tagged types, because they are very lightweight and serialize/deserialize well.
They are a good example of getting the benefit of type safety without making the code much harder to write.