The Boy Scout Rule

May I raise the so-called “Boy Scout Rule” (programming version)?

“Always leave the code you’re editing a little better than you found it” - Robert C. Martin (Uncle Bob).

Trolling through the codebase and am astounded to find (lots of, just an e.g.)

if(functionThatReturnsBool()) 
    return true; 
else 
    return false;

and that has been there for years! WTF?

How come nobody thought to tweak that a bit?

return functionThatReturnsBool();

How to promulgate the idea that everyone who touches the codebase should fix obvious snafus?

Coccinelle (Semantic Patch) will help you to turn this idea into reality.

Who wrote that? Lol

Or probably the code was left here in this state to eventually expand things when needed.

as example if I would act differently, maybe printing something as error or warning, the code with the if clause is more “extensible” to make such sort of things.

But this is only a guess, probably a “bad guess”.

Regards

Carlo D.

Yes, when I see such a weird thing, I systematically replace with

return !functionThatReturnsBool() != false?false:true;

I don’t like when code can be understood easily by kiddies. :mrgreen: :mrgreen: :mrgreen:

FreddyFreddy I literally searchedfunctionThatReturnsBool in the source and didn’t find it. Did you mean it literally or was that pseudo-code ?

edit: typo

:open_mouth: Yes, that’s pseudo code I guess. Would be a really bad name for an actual function. :slight_smile:

I’ve learnt that is a Really Bad Idea. Causes nothing but pain.

Just one of a number in various forms ..

bool ExpressionParser::isTokenAUnit(const std::string & str)
{
...
    if (status == 0 && token == UNIT)
        return true;
    else
        return false;
}

A perfect opportunity to add six lines of stupid comments to make sure they never figure it out!

Post amended due to the explanation of adrianinsaval below.

Regards

Carlo D.

Doesn’t matter, it’s the same because (status == 0 && token == UNIT) will always return either true or false, instead of an if statement to choose which to return you can just do return status == 0 && token == UNIT

Ah ok, it is similar to write in python (I have copied the condition as in C++, in Python probably is slightly different, I have to check the subtle boolean AND and OR writings of Python)

return True if (status == 0 && token == UNIT) else False

Ok I’ve guessed the point, thanks for correcting me!

Regards

Carlo D.

Representing it’s own style, your variant would at least throw a compile time error :laughing:

Sometimes Python and C++ are similar, and my very limited knowledge of C++ is not helping (Limited knowledge due to some Arduino fiddling, and is not real C++)

my code was Python, I don’t know if “ternary operators” Someone has told me they are called in this way are similar in C++, do you know why it is called ternary?

Good for my part to follow the “boy Scout Rule”.

My two sons are “Scouts” and usually calling them “Boy Scouts” is considerate a joke here in Europe :smiley:

But these considerations are clearly OT.

Probably I have to follow another rule “Think twice before writing a post!” or maybe “three of more times” but now “how to call this rule!” “common sense rule”? :laughing:

Sorry for bothering.

Regards

Carlo D.

because it has three operands 1st if 2nd else 3rd note that in python a ternary operator is redundant too, you can also just do

return status == 0 and token == UNIT

Three parts.

For the same example would be

return status == 0 && token == UNIT ? true : false;

but that would also be unnecesarily complex.

Thanks to all.

Regards

Carlo D.

Also…

You need to be careful of “short circuit” evaluation. If the first term is sufficient to establish the return value, the second term is not evaluated.

This avoids dereferencing undefined pointers for example.

Eg

If (ptr == Null || ptr->status() == false) return false;

Matthew