NL.17: Consider braces also for single-statement if

[GiovanniDicanio]

It's better under code maintenance to have single-statement if with braces, e.g.:

if (0 < x) {
    ++x;
}

Errors can happen if people start with code containing single-statement if-s without braces, and then this code is extended with additional statements, and the braces are not added by the maintainer.

It's just better to have the braces there from the beginning, even for single-statement if; it reduces potential for errors/bugs.

[mikebmcl]

My opinion is that an if statement without braces should be deprecated in both C and C++ and eventually removed. These days, saving a few bytes here and there in source files is insignificant when weighed against the bugs that this has allowed (especially in combination with accidental semi-colons). Indeed, I expect that compilers would become a tiny bit faster if braces were mandatory since they would be looking for a brace and fail if a non-whitespace, non-brace character was found versus allowing for either a brace or a character that began a valid statement.

Regardless, the omission of braces following if statements is a known source of bugs and as such I believe that these guidelines should draw attention to it just the same as they draw attention to other known sources of bugs that are (currently) syntactically correct.

[galik]

I actually suspect it is safer to not use braces round single statement ifs.

That may seem like a contrary opinion to some. I used to religiously put braces everywhere assuming it would prevent bugs. But i have never actually encountered a bug created in this way except for in beginners code.

Nowadays I tend to think that if you always assume an if will have braces then you run the risk of complacency when someone then forgets to (or deliberately chooses not to) put the braces. I think there is a danger people will "train" themselves to assume they can just keep adding statements after an if.

On the other hand, if there is no expectation for braces on single if statements people "train" themselves to pay attention to the if. They get used to spotting a single statement if from a block if. I feel that makes it harder to make a mistake. Actually adding a second statement to a single statement if looks immediately wrong.

When modifying code, a programmer should mentally walk through the logic of the code several times. A person used to differentiating between single statement ifs and block ifs is much more likely to spot missing braces than a person who habitually always uses blocks.

Other people's experience may differ but I suspect it i actually rather rare to find a bug caused by this kind of error.

[alphaONE2]

@galik: I guess you've never heard of the Apple "goto fail;" bug. This was a serious security bug in the iOS and OS X SecureTransport API, which was ultimately caused by code of the form

if (...)
    goto fail;
    goto fail;

The bug caused SecureTransport to skip authenticating SSL/TLS connections against the remote certificate to verify that the client is actually connected to whom it thinks it is. This opened the door for potential man-in-the-middle attacks for millions of OS X and iOS users.

[rianquinn]

Sounds like apple should have worried more about testing then. I agree with @galik. To add, an analysis tool should have caught that type of bug. Adding:

if (blah)
{
    do_something
}

everywhere kills readability as well.

[GiovanniDicanio]

"Readability" here is very subjective. I don't find code with braces around single-statement if "unreadable" at all. And I do prefer code safety and robustness under maintenance to very subjective "readability".

Il domenica 21 agosto 2016, Rian Quinn notifications@github.com ha scritto:

Sounds like apple should have worried more about testing then. I agree with @galik https://github.com/galik. To add, an analysis tool should have caught that type of bug. Adding:

if (blah) { do_something }

everywhere kills readability as well.

[GiovanniDicanio]

In addition RE your readability argument, to me it's irrelevant here, as I believe there are quality C++ programmers who consider your style of having matching braces vertically aligned with the open brace on its own line very readable, and other quality C++ programmers who think your way of using braces is unreadable as it wastes precious vertical space, and they prefer 1TBS.

All in all, it's always possible to have:

if (blah) { do_something; }

Il domenica 21 agosto 2016, Giovanni Dicanio xxx@xxx.xxx ha scritto:

"Readability" here is very subjective. I don't find code with braces around single-statement if "unreadable" at all. And I do prefer code safety and robustness under maintenance to very subjective "readability".

Il domenica 21 agosto 2016, Rian Quinn <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> ha scritto:

Sounds like apple should have worried more about testing then. I agree with @galik https://github.com/galik. To add, an analysis tool should have caught that type of bug. Adding:

if (blah) { do_something }

everywhere kills readability as well.

[gdr-at-ms]

We did consider these arguments when the rule was originally proposed. Clearly, each organization, each team will feel differently about where the braces go, how many of them must be there. Eventually, people will use tooling to enforce the particular layout they want for their projects. The one here is just a suggestion.

[GiovanniDicanio]

To conclude, I'd like to say I do agree that where to put the braces (Allman, K&R, ...) is very subjective, but suggesting to always using braces even for single-statement ifs is a different thing: this is just a "good default" suggestion, a good piece of advice, that would save C++ programmers and maintainers bugs and headaches.

I think those C++ core guidelines should suggest "good defaults", and using braces also for single-statement ifs is certainly a safe good default, IMO, so this is why I think it should be suggested in those guidelines.

[pserwa-exida]

MISRA C++ 2008 requires it:

Rule 6–4–1 (Required) An if ( condition ) construct shall be followed by a compound statement. The else keyword shall be followed by either a compound statement, or another if statement. (...) Example if ( test1 ); // Non-compliant - accidental single null statement { x = 1; } if ( test1 ) { x = 1; // Compliant - a single statement must be in braces }

[MikeGitb]

I really don't see, what possible benifit leaving out the braces would have. Considering that this causes real bugs during refactoring and the overhead is minimal I'm somewhat dissapointed that this didn't make it into the guidelines.

[tlanc007]

Yes, I'm disappointed this didn't make it, too.

At my work this was a requirement. At the time, when it was put in place, I thought it kind of a silly requirement and it didn't feel clean ascetically. I had been programming in C/C++ for years and never had an issue.

So at work I followed the rule because I had to. But for my own personal coding, I continued with single statement ifs. Then last year, I got careless and got burned. So now, compound statement ifs for me.

Its one of the harder cases to track down and then it isn't always obvious what the intent was.