What's the purpose of using braces (i.e. {}) for a single-line if or loop?

Solution 1:

Let's attempt to also modify i when we increment j:

int j = 0;
for (int i = 0 ; i < 100 ; ++i)
    if (i % 2 == 0)
        j++;
        i++;

Oh no! Coming from Python, this looks ok, but in fact it isn't, as it's equivalent to:

int j = 0;
for (int i = 0 ; i < 100 ; ++i)
    if (i % 2 == 0)
        j++;
i++;

Of course, this is a silly mistake, but one that even an experienced programmer could make.

Another very good reason is pointed out in ta.speot.is's answer.

A third one I can think of is nested if's:

if (cond1)
   if (cond2) 
      doSomething();

Now, assume you now want to doSomethingElse() when cond1 is not met (new feature). So:

if (cond1)
   if (cond2) 
      doSomething();
else
   doSomethingElse();

which is obviously wrong, since the else associates with the inner if.

Edit: Since this is getting some attention, I'll clarify my view. The question I was answering is:

What's the benefit of using the 1st version?

Which I have described. There are some benefits. But, IMO, "always" rules don't always apply. So I don't wholly support

Always use a { } block - even for a single line // not OK, why ???

I'm not saying always use a {} block. If it's a simple enough condition & behavior, don't. If you suspect someone might come in later & change your code to add functionality, do.

Solution 2:

It's very easy to accidentally change control-flow with comments if you do not use { and }. For example:

if (condition)
  do_something();
else
  do_something_else();

must_always_do_this();

If you comment out do_something_else() with a single line comment, you'll end up with this:

if (condition)
  do_something();
else
  //do_something_else();

must_always_do_this();

It compiles, but must_always_do_this() isn't always called.

We had this issue in our code base, where someone had gone in to disable some functionality very quickly before release. Fortunately we caught it in code review.

Solution 3:

I have my doubts as to the competence of the lecturer. Considering his points:

OK
Would anyone really write (or want to read) (b*b) - ((4*a)*c)? Some precedences are obvious (or should be), and the extra parentheses just add to confusion. (On the other hand, you _should_ use the parentheses in less obvious cases, even if you know that they're not needed.)
Sort of. There are two wide spread conventions for formatting conditionals and loops:
```
if ( cond ) {
    code;
}
```
and:
```
if ( cond )
{
    code;
}
```
In the first, I'd agree with him. The opening { is not that visible, so it's best to assume it's always there. In the second, however, I (and most of the people I've worked with) have no problem with omitting the braces for a single statement. (Provided, of course, that the indentation is systematic and that you use this style consistently. (And a lot of very good programmers, writing very readable code, omit the braces even when formatting the first way.)
NO. Things like if ( NULL == ptr ) are ugly enough to hinder readability. Write the comparisons intuitively. (Which in many cases results in the constant on the right.) His 4 is bad advice; anything which makes the code unnatural makes it less readable.
NO. Anything but int is reserved for special cases. To experienced C and C++ programmers, the use of unsigned signals bit operators. C++ doesn't have a real cardinal type (or any other effective subrange type); unsigned doesn't work for numeric values, because of the promotion rules. Numerical values on which no arithmetic operations would make sense, like serial numbers, could presumably be unsigned. I'd argue against it, however, because it sends the wrong message: bitwise operations don't make sense either. The basic rule is that integral types are int, _unless_ there is a significant reason for using another type.
NO. Doing this systematically is misleading, and doesn't actually protect against anything. In strict OO code, delete this; is often the most frequent case (and you can't set this to NULL), and otherwise, most delete are in destructors, so you can't access the pointer later anyway. And setting it to NULL doesn't do anything about any other pointers floating around. Setting the pointer systematically to NULL gives a false sense of security, and doesn't really buy you anything.

Look at the code in any of the typical references. Stroustrup violates every rule you've given except for the first, for example.

I'd suggest that you find another lecturer. One who actually knows what he's talking about.

Solution 4:

All the other answers defend your lecturer’s rule 3.

Let me say that I agree with you: the rule is redundant and I wouldn’t advise it. It’s true that it theoretically prevents errors if you always add curly brackets. On the other hand, I’ve never encountered this problem in real life: contrary to what other answers imply, I’ve not once forgotten to add the curly brackets once they became necessary. If you use proper indentation, it becomes immediately obvious that you need to add curly brackets once more than one statement is indented.

The answer by “Component 10” actually highlights the only conceivable case where this could really lead to an error. But on the other hand, replacing code via regular expression always warrants enormous care anyway.

Now let’s look at the other side of the medal: is there a disadvantage to always using curly brackets? The other answers simply ignore this point. But there is a disadvantage: it takes up a lot of vertical screen space, and this in turn can make your code unreadable because it means you have to scroll more than necessary.

Consider a function with a lot of guard clauses at the beginning (and yes, the following is bad C++ code but in other languages this would be quite a common situation):

void some_method(obj* a, obj* b)
{
    if (a == nullptr)
    {
        throw null_ptr_error("a");
    }
    if (b == nullptr)
    {
        throw null_ptr_error("b");
    }
    if (a == b)
    {
        throw logic_error("Cannot do method on identical objects");
    }
    if (not a->precondition_met())
    {
        throw logic_error("Precondition for a not met");
    }

    a->do_something_with(b);
}

This is horrible code, and I argue strongly that the following is vastly more readable:

void some_method(obj* a, obj* b)
{
    if (a == nullptr)
        throw null_ptr_error("a");
    if (b == nullptr)
        throw null_ptr_error("b");
    if (a == b)
        throw logic_error("Cannot do method on identical objects");
    if (not a->precondition_met())
        throw logic_error("Precondition for a not met");

    a->do_something_with(b);
}

Similarly, short nested loops benefit from omitting the curly brackets:

matrix operator +(matrix const& a, matrix const& b) {
    matrix c(a.w(), a.h());

    for (auto i = 0; i < a.w(); ++i)
        for (auto j = 0; j < a.h(); ++j)
            c(i, j) = a(i, j) + b(i, j);

    return c;
}

Compare with:

matrix operator +(matrix const& a, matrix const& b) {
    matrix c(a.w(), a.h());

    for (auto i = 0; i < a.w(); ++i)
    {
        for (auto j = 0; j < a.h(); ++j)
        {
            c(i, j) = a(i, j) + b(i, j);
        }
    }

    return c;
}

The first code is concise; the second code is bloated.

And yes, this can be mitigated to some extent by putting the opening brace on the previous line. So: if you insist on curly braces, at least put the opening brace on the previous line.

In short: don’t write unnecessary code which takes up screen space.

In the time since originally writing the answer I’ve mostly accepted the prevailing code style and use braces unless I can put the entire single statement on the previous line. I still maintain that not using redundant braces is usually more readable, and I have still never encountered a bug caused by this.

What's the purpose of using braces (i.e. {}) for a single-line if or loop?

Solution 1:

Solution 2:

Solution 3:

Solution 4:

Related

Recent Posts