"// ..." comments at end of code block after } - good or bad? [closed]

Question

I've often seen such comments be used:

function foo() {
   ...
} // foo

while (...) {
   ...
} // while

if (...) {
   ...
} // if

and sometimes even as far as

if (condition) {
   ...
} // if (condition)

I've never understood this practice and thus never applied it. If your code is so long that you need to know what this ending } is then perhaps you should consider splitting it up into separate functions. Also, most developers tools are able to jump to the matching bracket. And finally the last is, for me, a clear violation to the DRY principle; if you change the condition you would have to remember to change the comment as well (or else it could get messy for the maintainer, or even for you).

So why do people use this? Should we use it, or is it bad practice?

Answer 1

I would say if you code is so long that you can't easily follow your braces, your code needs refactoring, for most languages.

However, in templating languages (like PHP) it could be valid, because you might have a large block of HTML that separates the beginning and end of the condition or loop structure.

Answer 2

It's a code-smell and usually a hangover from old-fashioned code style. Before decent IDEs refactoring was more difficult and not as common as it is now, hence methods were longer and these comments were there to help navigate them better.

Answer 3

This is a horrible practice made obsolete by many factors.

• Most modern IDE's highlight the corresponding brace when the caret is on either symbol.
• If you're coding cleanly, you'll rarely find a place where your method is more than 10 lines.

I notice a lot of Java programmers having this mindset, and it makes the Java code look really dirty and takes the focus away from the code and towards the comments.

Highly suggest against using this.

Answer 4

Code is read 10 times more than it's written.

If it makes it easier to read, do it.

I'd also suggest to anyone doing this that they should look at some other ways to make things easier to read. The refactoring techniques, brackets on different lines, etc. that other people have mentioned are all good. Splitting things out into different functions, methods or classes so that the code is self-commenting is also good. There are also ways of eliminating most "ifs" and putting "for" loops into obvious places, thus eliminating the need for any of this.

But, sometimes people are learning. If this is something they're doing that's genuinely making the code more readable, encourage it, and then encourage some other practices too. People who are learning deserve and will benefit from encouragement, regardless of how they start. Saying "This is bad" isn't as useful as saying "This other thing is better".

Answer 5

In C++ there are two holdovers where this is still useful and the advice of "split up your code" doesn't necessary hold:

1. For namespaces. A namespace can encompass an entire file, and that last bracket can sometimes throw people off, so adding a comment to indicate the bracket is the closing of a namespace is useful. For the particular coding style at my company this is important because we do not indent namespaces as it was decided that such indentation would just waste space in a file.

2. For #ifdef / #endif pairs. Sometimes there's a lot of code in there for conditional compilation, it can get nasty with nesting, and the editor we use often heavy-handedly "helpfully" eliminates indentation, so the comments are useful during a quick overview.

Answer 6

I've got a large (C++) code base full of this sort of thing:

int Class::AccessorMethod(void)
{
    return privateValue;
}//end AccessorMethod

For something this small, I'd say this goes beyond "code smell" into "code stink". Especially in an IDE where I can match the closing brace with a keystroke to find the opening brace. Given a longer method, I'll still take the brace matching over the terminal comment. Such comments distract me, and I tend to think of them as noise.

Answer 7

For me the code has to be confusing to add a comment like that which you specified.

If it just says // IF Statement. Then you got to wonder why it's there in the first place.

Answer 8

The alternative to seeing what your brace is closing is having the opening one on the same column as the close one. I find that much clearer and more readable.

The comment is useful when it would normally be hard to trace because the open happened a long time ago. This should normally happen only for a namespace (particularly the anonymous one in C++, used for implementation detail in the compilation unit). In most other cases it should be obvious what you are closing.

Answer 9

This is largely a holdover from the old days of working in 80x24 character terminal windows, especially if you were using a windowed editor like EVE. Even now, I do most of my work in a terminal session using vim, and I may split the session into three or four subwindows, so I can only really view a few lines at any one time.

That said, I never really warmed to the convention, even though it would have saved my bacon on more than one occasion. I just see it as noise. If your loops or conditionals are getting that big, yeah, you might want to look into refactoring.

Answer 10

you basically give all valid reasons why not to use this. Every decent programmer should apply these. So why do people use it? Because they are doing it wrong and do not know better.