Raymii.org

Table of Contents

• The bug in question
• Named Booleans
• Name your magic numbers

During a recent code review I found a hard to spot bug, a misplaced parenthesis in an if statement. I often employ a technique I call named booleans, which would have prevented this bug. It's a simple technique, instead of a long if statement, give every comparison a seperate boolean variable with a descriptive name and use those variables is the if statement. This post shows the bug in question, an example of my named booleans technique and another tip regarding naming magic numbers.

Recently I removed all Google Ads from this site due to their invasive tracking, as well as Google Analytics. Please, if you found this content useful, consider a small donation using any of the options below:

I'm developing an open source monitoring app called Leaf Node Monitoring, for windows, linux & android. Go check it out!

Consider sponsoring me on Github. It means the world to me if you show your appreciation and you'll help pay the server costs.

You can also sponsor me by getting a Digital Ocean VPS. With this referral link you'll get $200 credit for 60 days. Spend $25 after your credit expires and I'll get $25!

Do note that most of what I write here is a matter of taste. Some people prefer comments, some people don't mind parentheses, but I find that comments tend to rot. Refactoring often doesn't take the comments into account and sometimes, when copied over a few times, they are plainly wrong.

The bug in question

The bug in question was caught before it hit the master branch, I spotted it during a code review of a new colleagues merge request. I can't show the actual code, but the line below is equivalent:

if ((_someLongNamedVar != FooLongNameEnum::Unknown && _someLongNamedMap.count(_someLongNamedVar)) == 0)

The _someLongNamedMap is a std::map<FooLongNameEnums, std::string> and there is parsing involved before this line. Seasoned C++ developers might already have spotted the issue.

If not, don't worry. The code compiles just fine, but when run, this method doesn't do what was intended. It however does exactly what was asked.

The second-to-last parentheses are placed wrong. The == 0 part has to be moved one parenthesis back:

if ((_someLongNamedVar != FooLongNameEnum::Unknown && _someLongNamedMap.count   (_someLongNamedVar) == 0))

The last statement otherwise doesn't compare to 0, but just evaluates:

_someLongNamedMap.count(_someLongNamedVar)) == 0)

As opposed to:

_someLongNamedMap.count(_someLongNamedVar) == 0))

In C++, only zero is false. Effectively the if statement was inverted.

This code was intended to check if a given item existed in the map. You might wonder why a .count() method is used? Well that is because the .contains() method is only available in C++ 20 and up and .count () has been there since forever. This codebase is compiled using C++ 17. Using .contains() would also have prevented this issue.

Named Booleans

Named booleans is a name I have given this coding technique where you extract every part of an if into seperate booleans explaining what they're intended for in their variable name, like so:

bool someLongNamedVarIsNotUnknown = _parameterCommand != FooLongNameEnum::Unknown;
bool someLongNamedMapCountIsZero = _someLongNamedMap.count(_someLongNamedVar) == 0;

Because the statement is now on its own line, the parentheses are no longer required and the if statement is both shorter and more readable.

Using descriptive names allows the if statement to communicates its intent much more:

if (someLongNamedVarIsNotUnknown && someLongNamedMapCountIsZero)
    return false;
else        
    return true;

When there are more booleans in my if, I often also combine those into another named boolean, trying to communicate the intent even more:

bool validVarButConditionNotMet = (someLongNamedVarIsNotUnknown && someLongNamedMapCountIsZero)

if(validVarButConditionNotMet)
    // do the false thing
else
    // do the true thing

Naming the variables like this not only eliminates comments (which are almost always out of date or not refactored along with the code) but also helps you to remember why certain things are done the way they are when you return to the codebase in the future.

One downside is that this technique prevents short-circuiting, which is often handy with pointer-related code. I often use that to make sure a pointer is not a nullptr before accessing it, because if one part of an if is false, the rest is not even executed. Example:

SomeClassPtr* p;
if(p && p->someMethod)

If p is a nullptr, p->someMethod is never executed. One caveat is that short-circuiting only works on builtin operators, not overloaded operators. Another reason people short circuit is to avoid costly functions, doing a cheaper validation first and only if needed, a costly method. But you can still avoid a costly function turning it into a lambda:

struct Example { int value = 126; };
auto const pointer = std::make_unique<Example>();
auto pointerIsOk = [&pointer] { return pointer != nullptr; };
auto valueIsGood = [&pointer] { return pointer->value == 126; };

if(pointerIsOk() && valueIsGood())
    std::cout << "All is well";

When I need to validate stuff that involves rules with context outside of the code, I often employ this techique. Imagine you're building a shoe recommendation engine and the business has a few indicators that make a shoe match to a user:

bool usersHairColorMatchesThisMonthsAdsColour = _user.hair == HairColour::Red;
bool userFeetSizeFitsInShoe = _user.feetSize <= _requestedShoe.size;
bool shoePriceFitsInUserBudget = _requestedShoe.price <= BudgetHelpers::Calculator(_user);

bool shoeIsProbablyOkayForUser = usersHairColorMatchesThisMonthsAdsColour && userFeetSizeFitsInShoe && shoePriceFitsInUserBudget;

if(shoeIsProbablyOkayForUser)

That if statement could also be way less readable with an ugly comment:

// this months ad campaign color is red 
if(_user.hair == HairColor::Red && _user.feetSize <= _requestedShoe.size && _requestedShoe.price <= BudgetHelpers::Calculator(_user))   

I explicitly choose to name the hair color comparison usersHairColorMatchesThisMonthsAdsColour and not userHasRedHair. The latter one does not indicate why the user has to have red hair. By explicitly naming a condition outside of the scope of the code, it becomes clear why we would check for it. When I come back to this code a few months later, I know right away why, in this case, we check the users hair color, instead of just knowing that we check if, but not why.

I've seen much code that just had a barren bunch of magic numbers and if's scattered all over the place, but in three months from now even you have forgotten the why behind all those if statements.

If this technique already has a name, please send me an email, I'd love to know, I'm not aware of it (yet).

Name your magic numbers

One other technique I also often employ is naming magic numbers. If you have a defined constant, lets say by convention all license plates starting with 42 are from your company, give that constant a descriptive name:

int companyCarLicensePlatePrefix = 42;

Rather than to sprinkle 42 all over your if statements, how much nicer is it to read companyCarLicensePrefix? That saves you a comment (// 42 is our license prefix), it saves you remembering what 42 was in this case and for new people, it's clear what the intent is right away, without having to lookup what 42 could mean in this context. And if it ever changes, you only need to change one variable instead of all over the place.

Tags: blog , bugs , c , c++ , cpp , performance , plt