CTO and co-founder of Signal Sciences. Author and speaker on software engineering, devops, and security.

Never Use Naked If Statements

For C, PHP, Java and Javascript programmers, naked if-statements are code landmines.

For many programming languages based on the C-syntax style, the body of an if-statement is delimited by braces:

if (foo == 1) {
    delete_everything();
}

However, in Javascript, Java, PHP, C and C++, Java, PHP, “naked” if-statements are allowed. These have a single body statement without using braces.

if (foo == 1) delete_everything();
if (foo == 1)
    delete_everything();

There are some other variation of naked statements, such are naked for-loops, but this is the most common.

There are two reason never to use naked if-statements. First is they cause less than optimal diffs. Second, they can introduce dangerous logic changes.

Diff Unfriendly

Modern programming is team sport, and a program is altered through a series of small changes (diffs) over time. These diffs are often scanned, and based on them, a full review is determined or not. Thus one really wants pull requests, commits and diffs to be as small and as targeted as possible, without a lot of noise. It’s very often the case that the body of an if-statement needs to be expanded, or debugging added. Let’s look at the diff of naked examples compared to:

if (foo == 1) {
    DEBUG("foo is 1)";
    delete_everything();
}

The first result is a complete rewrite of the line:

< if (foo == 1) delete_everything();
---
> if (foo == 1) {
>     DEBUG("foo is 1)";
>     delete_everything();
> }

The second form is even worse as it results in a more confusing partial rewrite.

1c1,2
< if (foo == 1)
---
> if (foo == 1) {
>     DEBUG("foo is 1)";
2a4
> }

If you used the normal, multi-line, with-brace format, the diff becomes the benign:

>     DEBUG("foo is 1");

A single change like this might not be a big deal, but often debugging is added in numerous places. With naked if-statements, adding trivial debugging statements turns a simple diff into a multi-page one.

Accidental Logic Changes

The previous examples assumed the programmer did the right thing and convert a naked if-statement into one with braces. But what if they just added the debug statement naively?

if (foo == 1)
    DEBUG("foo is 1)";
    delete_everything();

The diff is still simple:

>     DEBUG("foo is 1");

But the logic has dramatically changed, since the new version is actually:

if (foo == 1)
    DEBUG("foo is 1)";
delete_everything();

oops.

This can also happen in reverse, where quick commenting out of a line can also trigger logic changes.

if (foo == 1)
//    delete_everything();
copy_files();

In Summary

And in case, you are thinking this is a made up problem, I’ve personally been bitten by this, and others have too. And reading this stackexchange thread on the topic, you can almost see future accidents waiting to happen with the “not my problem if someone else screws up” attitude.

Interestingly, three of the newer and most popular languages golang, rust, and swift, do not allow naked if-statements. The confusing diffs they generated and unsafe logic changes are no doubt some of the reasons why. If you are using a more traditional language, forbid naked if-statements in your style guide and enforce it.

devops software

© 2018 Nick Galbreath