Should I use block identifiers ("end;") in my code?

Question

Code Complete says it is good practice to always use block identifiers, both for clarity and as a defensive measure.

Since reading that book, I've been doing that religiously. Sometimes it seems excessive though, as in the case below.

Is Steve McConnell right to insist on always using block identifiers? Which of these would you use?

//naughty and brief
with myGrid do
  for currRow := FixedRows to RowCount - 1 do
    if RowChanged(currRow) then
      if not(RecordExists(currRow)) then
        InsertNewRecord(currRow)
      else
        UpdateExistingRecord(currRow);

//well behaved and verbose
with myGrid do begin
  for currRow := FixedRows to RowCount - 1 do begin
    if RowChanged(currRow) then begin
      if not(RecordExists(currRow)) then begin
        InsertNewRecord(currRow);
      end  //if it didn't exist, so insert it
      else begin
        UpdateExistingRecord(currRow);
      end;  //else it existed, so update it
    end;  //if any change
  end;  //for each row in the grid
end;  //with myGrid

Answer 1

I have always been following the 'well-behaved and verbose' style, except those unnecessary extra comments at the end blocks.

Somehow it makes more sense to be able to look at code and make sense out of it faster, than having to spend at least couple seconds before deciphering which block ends where.

Tip: Visual studio KB shortcut for C# jump begin and end: Ctrl + ]

If you use Visual Studio, then having curly braces for C# at the beginning and end of block helps also by the fact that you have a KB shortcut to jump to begin and end

Answer 2

I would use whichever my company has set for its coding standards.

That being said, I would prefer to use the second, more verbose, block. It is a lot easier to read. I might, however, leave off the block-ending comments in some cases.

Answer 3

If I remember correctly, CC also gave some advices about comments. Especially about not rewriting what code does in comments, but explaining why it does what it does.

Answer 4

Personally, I prefer the first one, as IMHO the "end;" don't tell me much, and once everything is close, I can tell by the identation what happens when.

I believe blocks are more useful when having large statements. You could make a mixed approach, where you insert a few "begin ... end;"s and comment what they are ending (for instance use it for the with and the first if).

IMHO you could also break this into more methods, for example, the part

  if not(RecordExists(currRow)) then begin
    InsertNewRecord(currRow);
  end  //if it didn't exist, so insert it
  else begin
    UpdateExistingRecord(currRow);
  end;  //else it existed, so update it

could be in a separate method.

Answer 5

I'd say he's right just for the sake that the code can still be interpreted correctly if the indentation is incorrect. I always like to be able to find the start and end block identifiers for loops when I skim through code, and not rely on proper indentation.

Answer 6

It's never always one way or the other. Because I trust myself, I would use the shorter, more terse style. But if you're in a team environment where not everyone is of the same skill and maintainability is important, you may want to opt for the latter.

Answer 7

Block identifier are not only easier to read they are much less error prone if you are changing something in the if else logic or simply adding a line and don't recognizing that the line is not in the same logical block then the rest of the code.

I would use the second code block. The first one looks prettier and more familiar but I think this a problem of the language and not the block identifiers

If it is possible I use checkstyle to ensure that brackets are used.

Answer 8

commenting the end is really usefull for html-like languages so do malformed C code like an infinite succession of if/else/if/else

Answer 9

frequent // comments at the end of code lines (per your Well Behaved and Verbose example) make the code harder to read imho -- when I see it I end up scanning the 'obvious' comments form something special that typically isn't there.

I prefer comments only where the obvious isn't (i.e. overall and / or unique functionality)

Answer 10

I think it depends somewhat on the situation. Sometimes you simply have a method like this:

void Foo(bool state)
{
    if (state)
        TakeActionA();
    else
        TakeActionB();
}

I don't see how making it look like this:

void Foo(bool state)
{
    if (state)
    {
        TakeActionA();
    }
    else
    {
        TakeActionB();
    }
}

Improves on readability at all.

Answer 11

My knee-jerk reaction would be the second listing (with the repetitive comments removed from the end of the lines, like everyone's been saying), but after thinking about it more deeply I'd go with the first plus a one or two line useful comment beforehand explaining what's going on (if needed). Obviously in this toy example, even the comment before the concise answer would probably not be needed, but in other examples it might.

Having less (but still readable) and easy to understand code on the screen helps keep your brain space free for future parts of the code IMO.

Answer 12

I'm a Python developer, so I see no need for block identifiers. I'm quite happy without them. Indentation is enough of an indicator for me.

Answer 13

I'm with those who prefer more concise code.

And it looks like prefering a verbose version to a concise one is more of a personal choice, than of a universal suitableness. (Well, within a company it may become a (mini-)universal rule.)

It's like excessive parentheses: some people prefer it like (F1 and F2) or ((not F2) and F3) or (A - (B * C)) < 0, and not necessarily because they do not know about the precedence rules. It's just more clear to them that way.

Answer 14

I vote for a happy medium. The rule I would use is to use the bracketing keywords any time the content is multiple lines. In action:

// clear and succinct
with myGrid do begin
  for currRow := FixedRows to RowCount - 1 do begin
    if RowChanged(currRow) then begin
      if not(RecordExists(currRow))
        InsertNewRecord(currRow);
      else
        UpdateExistingRecord(currRow);
    end;  // if RowChanged
  end;  // next currRow
end;  // with myGrid

Answer 15

Personally I recommend always using block identifiers in languages that support them (but follow your company's coding standards, as @Muad'Dib suggests).

The reason is that, in non-Pythonic languages, whitespace is (generally) not meaningful to the compiler but it is to humans.

So

with myGrid do
  for currRow := FixedRows to RowCount - 1 do
    if RowChanged(currRow) then
      Log(currRow);
      if not(RecordExists(currRow)) then
        InsertNewRecord(currRow)
      else
        UpdateExistingRecord(currRow);            

appears to do one thing but does something quite different.

I would eliminate the end-of-line comments, though. Use an IDE that highlights blocks. I think Castalia will do that for Delphi. How often do you read code printouts anymore?