Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
    
In PHP, I use the alternative syntax for control structures if(condition): ... else: ... endif; –  systemovich Mar 1 '11 at 14:07
    
@Geoffrey van Wyk - really? I have never seen anybody use these outside of template files. They are extremely unstandard, but to each their own, I guess. –  Craige Mar 1 '11 at 15:30
3  
@Craige: Any language construct natively supported by PHP is not "Extremely Unstandard" -- the PHP interpreter defines what "standard" is. –  Billy ONeal Mar 1 '11 at 16:07
    
Ada has specific markers for the end of most constructs: if ... then ... end if; while ... loop ... end loop; procedure Foo is ... end Foo;. I find that it helps legibility (and it's checked by the compiler, which comments aren't). –  Keith Thompson Apr 4 '12 at 20:45
add comment

11 Answers 11

up vote 31 down vote accepted

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.

share|improve this answer
5  
Completely valid point for PHP, if you're using PHP mixed with Html. 1 point for Gryffindor. –  Sergio Mar 1 '11 at 12:31
2  
Even with PHP as a templating language mixed with HTML you can still indent. You also shouldn't use braces when using PHP as a templating language, but rather the while(): endwhile; and foreach(): endforeach; constructs etc. –  Htbaa Mar 1 '11 at 12:57
8  
I don't really see why you shouldn't be refactoring the PHP. Probably into another language. –  Tom Hawtin - tackline Mar 1 '11 at 13:00
    
@Htbaa: I can't believe I've been using PHP all this time and didn't know about those. Thanks! Regarding indenting, I prefer to keep the conditional HTML indented the same as the rest of the page, rather than in line with the PHP that's creating it. –  Matt Ellen Mar 1 '11 at 13:00
3  
@Tom: I laughed, but feel guilty. :P –  Sergio Mar 1 '11 at 13:03
show 2 more comments

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.

share|improve this answer
add comment

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.

share|improve this answer
    
I have a coworker (java developer) who does this. Except instead of //for and //if he uses //rof and //fi. It drives me crazy and he does it everywhere. –  Cyrena Mar 1 '11 at 15:58
    
Yeah I had one that insisted on it. AAAAAGHHHHH. –  Rig Apr 4 '12 at 20:49
1  
This really does not have anything to do with Java or Java programmers, and it's not common or a de-facto standard thing to do when programming in Java. –  Jesper Apr 5 '12 at 10:06
    
+1,000 for "is a horrible practice". –  scunliffe Dec 20 '12 at 15:25
add comment

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".

share|improve this answer
3  
This is bad. Even beginner level textbooks don't do this atrocity. "Code is read 10 times more than it's written"... all the more reason not to waste the reader's time with this code cruft. –  Thomas Eding Apr 4 '12 at 20:20
    
@trinithis What if you have 20-case switch statement? Sometimes you need to support 20 different options, and it's better to have them gathered in one place than "refactored" into some multi-level decision-making scheme. –  quant_dev Apr 5 '12 at 10:07
    
i believe this is a practice by coders who underestimate peers :) that others are not smarter enough to read the code unless. gnerally i am against this practice, but okay if someone does it coz there is a jungle of }s that makes it hardly readable. i dont do that anyways! –  Kiran Ravindranathan Apr 5 '12 at 11:15
    
@trinithis It's not about whether it's bad or not, it's about effective ways to help people get better so they grow through it. Being effective > being right. It's also a perfectly sensible thing to do if, say, you're refactoring legacy code that's even more of a mess. Sometimes bad things can make better interim steps and lead to something even better than that. –  Lunivore Apr 5 '12 at 11:35
    
My opinion is that if the code is complex enough to warrent the // end if (or // end BLANK) comment, the code is complex enough that I would have no idea what control structure any line of code belongs to. Therefore each line of code should have a comment telling the reader where the code resides: if(x == y) { // in void bar(int x) <newline> doSomething1(); // in if(x == y) <newline> doSomething2(); //in if(x == y) <newline> } // end if(x == y) <newline> ++x; // in void bar(int x) –  Thomas Eding Apr 5 '12 at 17:08
show 1 more comment

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.

share|improve this answer
    
+1 for the namespace comment and the reasons. That's the only time I do this and for the same reason –  JohnB Apr 5 '12 at 8:49
    
+ long switch statements. –  quant_dev Apr 5 '12 at 10:08
add comment

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.

share|improve this answer
    
Um, I guess I don't get some of the formatting on this site; each brace up there should be on its own line, as should the method body. –  PSU Mar 1 '11 at 14:26
    
In C++ though I will often open anonymous namespace at the top for hidden implementation stuff. Then at some point I close this brace and it's useful to know what the close-brace means here. –  CashCow Mar 1 '11 at 17:11
add comment

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.

share|improve this answer
    
I agree, it looks like a line that's been commented out, rather than an old school //endif –  StuperUser Mar 1 '11 at 11:55
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
    
erm, explain the downvote please. I didn't just invent this answer, it stems from a real life experience: my collegue who's sitting a couple of feet next to me has been programming for over 10 ears like that and had no clue about why this is wrong until I explained him. –  stijn Mar 1 '11 at 11:42
    
Possibly it was used in some textbook so a bunch of people came out of college using it? It could have a place in an introductory text. Also reasonably valid in a preprocessor, especially in #ifdef/endif include guards –  Martin Beckett Mar 1 '11 at 16:48
add comment

Indent the damn code instead. (You are doing this already aren't you???!) No fuss, no worry, no out-of-date comments, no extra maintenance.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.