scream icon indicating copy to clipboard operation
scream copied to clipboard

Change as many bool as possible to enums (where useful)

Open bartgol opened this issue 2 years ago • 1 comments

Bool's can be cryptic when found at the call place. Take the function

void allocate_vector (std::vector<int>& v, const int n, const bool init_to_random);

As the name says, if the last input is true, we init the entries to random values. Pretty clear. However, this is how the code looks at the call site:

std::vector<int> vec;
allocate_vector(vec,10,true);

Not so clear anymore. The issue can be overcome by inserting inline comments, such as

allocate_vector(vec,10, /* init_to_random = */ true);

but I would argue that's a bit clunky. Compare that with

enum InitType {
  InitDefault,
  InitRandom
};
void allocate_vector(std::vector<int>& vec, int n, InitType);
...
std::vector<int> vec;
allocate_vector(vec,10,InitRandom);

It gets even better if you use strong enums:

enum class InitType {
  Default,
  Random
};
void allocate_vector(std::vector<int>& vec, int n, InitType);
void allocate_vector_bad(std::vector<int>& vec, int n, bool init_to_random);
...
std::vector<int> v;
allocate_vector (v,10,InitType::Random); // ok, works
allocate_vector (v,InitType::Random,10); // Compiler error, good.
allocate_vector_bad(v,10,true);                // ok, works
allocate_vector_bad(v,true,10);                // ARGH! This works too!

The last example might be silly, but I did run into something similar with a very well developed linear algebra package, where a mistake at the call site ended up passing a pointer to a bool. Had that bool been a strong enum, the compiler would have barked, and the bug caught right away.

bartgol avatar Nov 08 '21 23:11 bartgol