Hacker News Reader

Comment by jimrandomh | Hacker News Reader

> "Specifically, a warning is issued for if, else, while, and for clauses with a guarded statement that does not use braces, followed by an unguarded statement with the same indentation."

> "The warning is not issued for code involving multiline preprocessor logic ... The warning is not issued after a @code{#line} directive, since this typically indicates autogenerated code"

Seems like a good thing to have, and pretty conservative about not causing false positives. In particular, constructs like double-iteration should be fine:

    for(y=0; y<ymax; y++)
    for(x=0; x<xmax; x++)
    {
        //...
    }

Which I checked because I like this construct and people very rarely think about it.

View on HN

That pattern still seems like obfuscation to me. Why not indent the inner for loop, to make the control flow more obvious?

For instance, consider how "break" or "continue" inside the braces would work.

I don't see how 'continue' would be confusing, it would stop the current calculation and start the next one, same as with normal loops.

The behaviour of 'break' might be confusing but then that's always the case in nested loops, no matter how you notate them.

I would argue that `break` intuitively makes you go back an indent, so if you don't indent after the first `for`, one can assume `break` just exits both loops (goes back from lvl 1 to lvl 0 indent)

This doesn't quite work, since break skips over if statements (and most breaks have at least one if statement around them). So you have to look glance at the keyword at the elbow of each indent anyways, to know where it's going.

The reason for doing it that way, is to emphasize when two loops together are conceptually iterating over a single thing, and have some symmetry, like iterating over the cells in a 2D grid. It makes it possible to line up the elements of the two loops, emphasizing the similar structure.

I agree that it would be very bad form to use "break" inside a block like that. But in the cases where this construct applies, it would still be bad form to break if the outer block had curly braces and indentation, because skipping the rest of a row isn't a natural operation in the sort of way that skipping the rest of a collection is.

However one can say they are not quite the same thing, as you mentioned, how "break"s work, so it is obfuscating things a bit. But even performance wise: in order in which you iterate over the arrays can make a significant difference because of cache effects. So it is important to make obvious which one is the nested loop and which one is the outer one.

Even with that explanation I am not convinced of the soundness of that notation. For example, if you think you have a particular row that is causing you some problem and want to add a print statement for the row number, your code becomes, if you are too quick and forget to add braces:

    for(y=0; y<ymax; y++)
    printf("y=%i\n",y);
    for(x=0; x<xmax; x++)
    {
        //...
    }

And it will not work anymore as intended.

Granted it's a small mistake and it will not take more than 5 minutes to fix, but it still add a small load to your already busy mind.

Totally agreed, but in this case if the author had declared the variable in the for-loop:

  for(int y=0; y < ymax; y++) 
     ...

and didn't shadow y from an enclosing scope, that would have given the compiler a chance to catch that error.

This doesn't seem like a realistic mistake to me. The printf is just at the wrong indent level; this is the same mistake as:

    for(i=0; i<imax; i++)
    printf("i=%i\n",i)
    {
        //...
    }

Which also compiles and which is wrong in the same way. I don't recall ever having made or having seen this.

Well, not really, because in the example you just gave, you would put the printf right between the braces as it should be. In the case originally discussed, you have no braces so you would have to add them, and that can be forgotten.

If the option is between "something we personally like but has edge cases or requires other understanding that could confuse some readers" versus "something everybody will understand," the first case should never happen.

It's almost like everybody forgot about https://matt.sh/howto-c#_formatting already. :(

> The reason for doing it that way, is to emphasize when two loops together are conceptually iterating over a single thing, and have some symmetry, like iterating over the cells in a 2D grid. It makes it possible to line up the elements of the two loops, emphasizing the similar structure.

I'm not convinced. Even iterating over a 2D grid, one loop is iterating over the columns, and the other the rows, and indentation (and braces) can help see which is which.

While I understand that it's mostly a question of aesthetics, I don't like this also because it might break the expected semantics of continue/break in such a loop. When I iterate over things more complex than a single variable where the conditions in a for() would become ugly, I tend to put the domain-specific "increment" into a small static function:

    static int
    next_pixel(*x, *y, width, height) {
        (*x)++;
        ...
        if (last pixel...)
            return 0;
        return 1;
    }

    /* later on... */

    do {
        /* do something with coordinates x, y */
    } while (next_pixel(&x, &y, WIDTH, HEIGHT));

This is, of course, not only restricted to 2D arrays, but could cover walking of a tree or a filesystem...

I apologize for the stupidity of my question but shouldn't that be

    for(y=0; y<ymax; y++)
    {
    for(x=0; x<xmax; x++)
    {
        //...
    }
    }

Edit: Oh, I get it, {} are only "mandatory" if we have more than one instruction, the second for loop gets counted as one instruction and hence the omitting of {}. Thanks!

No, the two mean the same thing in C, e.g.:

    if (x)
        y;

Is the same as:

    if (x) {
        y;
    }

Just in case anyone who has never used C (or Java, or C# etc etc) is confused, it's not the indentation that is causing this meaning here:

    if (x) y;

also has the same meaning.

Why would it be? Certainly not for readability. I think it was posted as intended.

It works without the additional braces. Consider this:

    for (i=0;i<imax;i++)
        printf("%d\n", i);

You can omit the braces if the body of the loop is a single block, just like an if statement. You can also do this:

    if (a == b)
        if (b == c)
        {
            do_thing();
        }

Same deal with the for loop. The nested loop is a single block.

In C, control flow like if, for, while, do, else, etc., take the rest of the line of code (i.e., till the next semi-colon) and runs it under the conditions of the clause.

braces—{}—allow you to take multiple lines of code and have them be guarded at the same time rather than having to repeat the condition. GP is taking advantage of the fact that the next for clause will be considered a single statement, and therefore the second set of braces are not technically needed.

However, omitting braces for single-line snippets has caused plenty of havoc in the past (cf. Apple's gotofail). Some would say it is better to always include braces in control flow.

If I were doing it that way, I wouldn't give the outer blocks' curly braces their own lines. For example in C#, where pretty much every file has a single "namespace" block containing a single "class" block, I write it as:

    namespace SomeNamespace {
    class SomeClass
    {
        //...
    }}

Which avoids having shifted the whole file an extra indent level to the right. But I haven't seen anyone else doing this.

I'm not writing C# but yeah: AS3 and Java both have the Package{class{ two-step before you actually write any code. I don't even indent inside class:

  package{
  class{
  main{
    now we indent
  }
  }}

You don't indent after a package e.g.

https://github.com/apache/spark/blob/master/core/src/main/ja...

And it's fair enough to indent with a main since it is so infrequent. Ideally you only have one main entry point.

Ew! That's not any better at all...

Man's gotta have an opinion. At least C doesn't have optional semi-colons.

If I see this, I'll immediately think it's a bug, the difference between x and y is not even obvious at first sight, it looks like the line was duplicated accidentally.

What about this ?

    for(y=0; y<ymax; y++) {
    for(x=0; x<xmax; x++) {
        //...
    }
    }

I'd put the two loop-headers on the same line in that case.

Reply on news.ycombinator.com