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 

Robinette

Code Complete might also recommend a more precise title for this question. – Capacitor

Fixed up the title for the OP. – Velleman

My God, what language is that? – Stimson

@John Saunders - It is Delphi (Object Pascal). – Robinette

@Several responders - the comments after each "end" are a habit I've developed after working with 500+ line methods, whose indentation has some lines starting halfway across the screen. It is true that those comments are not really useful, here. – Robinette

@Capacitor - thanks for the better title, I agree that the original was a poor choice. – Robinette

What is the advantage of using with in this example? – Nad

@mjustin: the advantage of the "with" is to create another level of indentation and exaggerate my example a little. Whether it adds to the "cleanness" of the code is a matter of taste. – Robinette

with does not have any practical use. Never. – Acriflavine

I disagree. with is just syntatical sugar to make code less repetitive and verbose. Used carefully, it can make code cleaner and easier to maintain. – Robinette

Answer 1

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.

Jannette

Wow, using the egyptian coding style (block opening and closing "statement" does not have the same indentation) can be truly confusing. Yes, I'm "talking" about the " then begin " line which really could use a line break between the two keywords. – Acriflavine

Answer 2

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

Izettaizhevsk

Thanks for the 'ctrl + ]' tip! I always figured there was a way to do this but was too lazy to look it up! – Epergne

Answer 3

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.

Anamorphosis

Answer 4

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.

Sylviesylvite

I think the reason I would still prefer the second example is consistency. You're right, for very brief and short statements it doesn't add a lot of value, but it does for more complicated statements it does and I'd try to keep my coding style as consistent as possible. – Lagoon

Answer 5

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.

Erratic

Yes, but in Python indentation is signficant to the compiler. – Robinette

My point is: If block identifiers are optional in any language, indentation is a sufficient indicator, regardless of whether whitespace is significant. – Erratic

Answer 6

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.

Digenesis

Answer 7

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.

Dosia

+1 for this. The "what" should be discerned by reading the line of code. The comments are for the "why". – Isobaric

Answer 8

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.

Ketty

Answer 9

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.

Pewee

Answer 10

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.

Eskilstuna

Answer 11

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.

Loleta

Answer 12

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

Dexamethasone

Answer 13

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

Lewak

Answer 14

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)

Blaisdell

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?

Nausea

Castalia do more than highlight blocks - is highlight blocks and flow of code modifiers. CnPack highlights blocks too - but AFAIK, not flow of code. – Lashawn