Code Comment Best Practice

Not a person

Student here,

Should code comments go to the right of the code..like this...

int main(void)
{
 int one = 2; /* declares a variable of type int, called one, with a value of 2 */
 int two = 3;
 int three = one + two;    /* adds variables named one and two */
    
 printf("%d",three);            /* outputs the sum of one and two */
 return 0;
}

or above the relevant line... like this??

int main(void)
{
    /* declares a variable of type int, called one, with a value of 2 */
    int one = 2;                        
    int two = 3;
    
    /* adds variables named one and two */
    int three = one + two;                 
            
    /* outputs the sum of one and two */
    printf("%d",three);            
    
    return 0;
}    

ChRoMe

Not a person wrote: »

Student here,

Should code comments go to the right of the code..like this...

int main(void)
{
 int one = 2; /* declares a variable of type int, called one, with a value of 2 */
 int two = 3;
 int three = one + two;    /* adds variables named one and two */
    
 printf("%d",three);            /* outputs the sum of one and two */
 return 0;
}

or above the relevant line... like this??

int main(void)
{
    /* declares a variable of type int, called one, with a value of 2 */
    int one = 2;                        
    int two = 3;
    
    /* adds variables named one and two */
    int three = one + two;                 
            
    /* outputs the sum of one and two */
    printf("%d",three);            
    
    return 0;
}    

My vote is for above the line, its just a matter of personal preference though.

GreeBo

typically I would comment above the line also, but for some reason within a condition I always seem to comment afterwards

I think simple, short remarks go after the relevant line, longer explanations naturally go above.

Stainless_Steel

I am self employed and therefore nobody sees my code.

I have a terrible habit of procrastinating and leaving comments for another day. Never get around to it!!! I'm a bold boy I know.

amen

Preference for above the line but I would not be impressed if someone was commenting such simple lines of code.

I would rather have a meaningful description of the function, its inputs/outputs and any gotchas.

Not a person

Thanks.

Short comments to the right and longer ones above seems a sensible strategy.

Not a person

amen wrote: »

I would not be impressed if someone was commenting such simple lines of code.

This is one of the first programs the college got us to do for an introductory module in C.

Don`t worry, I wouldn't imagine anyone would comment such lines in a real world scenario

GreeBo

Stainless_Steel wrote: »

I am self employed and therefore nobody sees my code.

I have a terrible habit of procrastinating and leaving comments for another day. Never get around to it!!! I'm a bold boy I know.

Stop this immediately.
Commenting is not something you do after the fact, you should comment as you go. Even if its only you, you are going to have to read your own code someday in the future and understand it.

seamus

Generally above the line, but in some cases it can make more sense from a readability POV to do short one-word comments beside.

The simplest examle I can think of is a switch/case condition. Putting a comment above each case() would interfere with the flow of the code, so a short comment beside each case() statement would make it easier.

Thusly:

switch(someVar) {
	
	case("J"): //Joe
		...
	break;
	
	case("O"): //John
		...
	break;
	
	case("C"): //Jack
		...
	break;
	
	case("N"): //Jane
		...
	break;
}

I find this reads easier than

switch(someVar) {
	//Joe
	case("J"): 
		...
	break;
	
	//John
	case("O"): 
		...
	break;
	
	//Jack
	case("C"): 
		...
	break;
	
	//Jane
	case("N"): 
		...
	break;
}

Anima

To answer the question, I think above it looks better. You shouldn't be needing to comment individual lines to have to place it on the side. If the code is too complicated to see what it does at a glance then it needs to be re-written.

More generally however, I think commenting should be a thing of last resort as well. The code should be the documentation and written with clear variable/method/class names. If the method/function gets too long and complicated that you need a paragraph of text to describe what it does, then it should be re-factored.

Having to modify the code and then the comments all the time (because there are so many) just leads to comments that don't get updated and that's worse than nothing at all.

Creamy Goodness

Above the line for two reasons.

1. long lines make me cry
2. I like code that reads as close to a book as possible.

In the western world we read up to down, left to right, I don't particularly want to read the code then read what it does. If the comments are good I won't need to read the code, therefore the comment should come first.

In my humblest anyways.

PrzemoF

OP, if you're looking for some coding style guidelines I'd recommend this:

http://www.kernel.org/doc/Documentation/CodingStyle

If you like it there is a script that can check the style of a patch or file (scripts/checkpatch.pl) in kernel source - it's really handy.

You can also check http://www.x.org/wiki/CodingStyle

Both are huge projects, so keeping consistent coding style is quite important.

Sparks

Wherever the company's coding standard says they should go; and if there's no standard, then wherever makes the comment&code more readable.

GreeBo

Anima wrote: »

If the code is too complicated to see what it does at a glance then it needs to be re-written.

Often you are going to have some complicated logic or case that deserves an explanation rather than forcing someone to step through the code. I want to know what it does, not necessarily how it does it.

Anima wrote: »

Having to modify the code and then the comments all the time (because there are so many) just leads to comments that don't get updated and that's worse than nothing at all.

If you are modifying the code to do something different then the comments should be changed. If the comments shouldnt be there at all then remove them rather than update them imo.

Tar.Aldarion

Another vote for above, it can give you an understanding of the code before you see it, so that you don't have to go reading back over the line after reading the comment.

Skrynesaver

First you need to understand the purpose of comments, they enable you to return to the code and understand it, or someone else to maintain your code. in order to maintain code you need to understand the intended purpose of the code and also how functions are to be used (so you can extend functionality) something like the following would be my preferred template.

/*********************************************************
 * Here we would describe the purpose of the function    *
 *  What parameters it expects and what it returns       *
 *  We might also include an example outlining  how it   *
 * should be called                                      * 
 *********************************************************/
int main(void)
{
    int one = 2;
    int two = 3;
    int three = one + two;    
 
   /* Imagine this line does something esoteric or counter-intuitive *
    * so we need to explain why for the maintainer of the code       */
    printf("%d",three);          
    return 0;
}

Esterhase

I have to agree with Anima here - I used to comment the hell out of everything but now I prefer to let my code speak for itself. I don't miss comments at all now. Even for complicated logic, using clear descriptive names and breaking up methods if necessary goes a long way towards making things easy enough to understand with minimal commenting. When the code is written nicely, a sentence of overview describing the function should suffice. In my opinion anyway.

As for the OP's question - I tend to find comments easiest to read when they are above the line, so that's where I put them.

Anima

GreeBo wrote: »

Often you are going to have some complicated logic or case that deserves an explanation rather than forcing someone to step through the code.

Complicated code is bad code though. The majority of the code in an application isn't going to be like that and if it is then that's a bad sign. I'm talking in general here and in cases where code clarity is sacrificed for performance or whatever, then yes a comment is well justified.

I want to know what it does, not necessarily how it does it.

That's what a method name is for and you only need to write that once or refactor using the IDE if you find that it isn't clear enough. A comment requires you to keep two seperate things 1:1 which leaves scope for error.

If you are modifying the code to do something different then the comments should be changed. If the comments shouldnt be there at all then remove them rather than update them imo.

That's what I'm saying though, "should be" changed often leads to "never changed" I find and then you're left with comments that say one thing and code that does another.

It's better to have just the code (that's written clearly) because it can't lie unlike the comments.

GreeBo

Anima wrote: »

Complicated code is bad code though. The majority of the code in an application isn't going to be like that and if it is then that's a bad sign. I'm talking in general here and in cases where code clarity is sacrificed for performance or whatever, then yes a comment is well justified.


That's what a method name is for and you only need to write that once or refactor using the IDE if you find that it isn't clear enough. A comment requires you to keep two seperate things 1:1 which leaves scope for error.


That's what I'm saying though, "should be" changed often leads to "never changed" I find and then you're left with comments that say one thing and code that does another.

It's better to have just the code (that's written clearly) because it can't lie unlike the comments.

I disagree that you can learn all you need to know from method names, sure most of the time you can, but sometimes, particularly with an entry point for example, the name isnt going to cut it and you will need some documentation.

If the problem is people not updating needed comments when they update the code then thats not the comments fault, its the developer doing it.

Sparks

Esterhase wrote: »

I don't miss comments at all now.

You wouldn't be the one that misses them.
It's the people who wind up debugging your code six months after you move company to a new job that miss them...

Skrynesaver

Creamy Goodness wrote: »

Above the line for two reasons.

1. long lines make me cry
2. I like code that reads as close to a book as possible.

In the western world we read up to down, left to right, I don't particularly want to read the code then read what it does. If the comments are good I won't need to read the code, therefore the comment should come first.

In my humblest anyways.

The comment is what the writer thought the code was doing, always read the code.

Esterhase

Sparks wrote: »

You wouldn't be the one that misses them.
It's the people who wind up debugging your code six months after you move company to a new job that miss them...

My current company rarely uses them, and has had this policy as long as I've been here. We all manage just fine, and if/when I leave I'll be replaced by a person who will also be expected to read and write clean code that explains itself with minimal comments. Don't believe me if you don't want to, but we have few problems understanding each others code.

Sparks

Esterhase wrote: »

My current company rarely uses them, and has had this policy as long as I've been here. We all manage just fine, and if/when I leave I'll be replaced by a person who will also be expected to read and write clean code that explains itself with minimal comments. Don't believe me if you don't want to, but we have few problems understanding each others code.

Yeah, after five years working in places like that (and nine more in other places where that would be grounds for a long chat with a senior developer in an office with a closed door, at best), I'm just going to politely disagree until I can get to within arms reach of enough alcohol to make what you just said stop hurting the inside of my head.

GreeBo

Esterhase wrote: »

My current company rarely uses them, and has had this policy as long as I've been here. We all manage just fine, and if/when I leave I'll be replaced by a person who will also be expected to read and write clean code that explains itself with minimal comments. Don't believe me if you don't want to, but we have few problems understanding each others code.

Maybe, but your productivity is likely suffering.

Skrynesaver wrote: »

The comment is what the writer thought the code was doing, always read the code.

Again, its counter productive to have everyone read and understand every bit of code they interact with. Do you read your way down through every part of java.io package before you read from a file, or do you read the appropriate javadoc/api?

Smoggy

Clean Code and no comments is the way forward.

The code comments iteself.

Now if it's a complex piece of code and a comment is required, then my vote goes with above.

ChRoMe

Sparks wrote: »

Yeah, after five years working in places like that (and nine more in other places where that would be grounds for a long chat with a senior developer in an office with a closed door, at best), I'm just going to politely disagree until I can get to within arms reach of enough alcohol to make what you just said stop hurting the inside of my head.

Mine is a Guinness

Sparks

Smoggy wrote: »

Clean Code and no comments is the way forward.

stevenmu

Almost exclusively above the line, or to be more accurate above the block. Beside the line often means having to scroll right to read which is just awful. Also, as someone else mentioned, individual lines of code should be pretty clear.

The one exception I can think of right now is that for some complicated or more obscure API calls I might put a short note after. Even then if the comment needs to be more than a few words I'll just put it above.

Esterhase

Fair enough. I know what works for me doesn't work for everyone else.

GreeBo wrote: »

Again, its counter productive to have everyone read and understand every bit of code they interact with. Do you read your way down through every part of java.io package before you read from a file, or do you read the appropriate javadoc/api?

I don't think anyone's saying it's a good idea to pore over each and every line of code to understand what it's doing. If I glance over a File class and see a method called setReadOnly, I don't need a comment to tell me that the method sets a file to be read only. If I need more detail than that I read the method itself which, if written well, has its steps laid out in a clear manner. If there is some hidden piece of functionality in there that would need a comment to explain, the method should be renamed to include that.

Those short paragraphs explaining what the function does are the kinds of comments I am all too happy to do without, and those are the ones I don't miss. I'm not up for going 100% comment free at all, but I do think aiming towards the minimal end of the spectrum is a good thing.

Skrynesaver

GreeBo wrote: »

Again, its counter productive to have everyone read and understand every bit of code they interact with. Do you read your way down through every part of java.io package before you read from a file, or do you read the appropriate javadoc/api?

I tend to trust Sun (and 15 years of trial by usage) more than the clown over the corridor who I saw stumble in hungover on a Tuesday, but if I were debugging an issue in java.io I'd read the code in the method in question rather than the comment that said it was wonderful and shat rainbows

GreeBo

Skrynesaver wrote: »

I tend to trust Sun (and 15 years of trial by usage) more than the clown who I saw stumble in hungover on a Tuesday, but if I were debugging an issue in java.io I'd read the code in the method in question rather than the comment that said it was wonderful and shat rainbows

Exactly, read the comments to figure out what it does, read the code if you need to know more.
btw Oracle have bugs too yunno.

Gurgle

Skrynesaver wrote: »

I'd read the code in the method in question rather than the comment that said it was wonderful and shat rainbows

Generally if you're in there actually reading the fine detail of someone else's code (or your own code from a year ago) its because it doesn't work as intended.

Read the comment to see what it was intended to do, then read the code to see what it actually does.

-edit- oh yeah, comments above code

Anima

Gurgle wrote: »

Read the comment to see what it was intended to do, then read the code to see what it actually does.

Who is to say that the comment is correct?

BrokenArrows

Commenting should be explaining WHY your code does something rather than what it does. People should be able to workout what something does.

An Example would be a FOR loop staring at 2 instead of 0.
Not much point in commenting:
// loop staring at 2 to 100

Needs to be:
// loop starting at 2 because array locations 0 and 1 are place holders.

Not a person

When starting this thread i could never have anticipated the amount of comments it would generate.

Interesting and differing views.

Elfman

I think after a while you don't need to think about this but as a rule of thumb.

- If the comment relates to a line //Do it to the right

//You should do it above
If it relates to a block of code or will take multiple comment lines
line this

Smoggy

BrokenArrows wrote: »

Commenting should be explaining WHY your code does something rather than what it does. People should be able to workout what something does.

An Example would be a FOR loop staring at 2 instead of 0.
Not much point in commenting:
// loop staring at 2 to 100

Needs to be:
// loop starting at 2 because array locations 0 and 1 are place holders.

Could better named variables negate the need for comments in your example ?


iPostPlaceHolderArrayStart = 2
iArrayEnd = 100

for(int i=iPostPlaceHolderArrayStart; i<iArrayEnd; i++)
{
// Test
}

GreeBo

Smoggy wrote: »

Could better named variables negate the need for comments in your example ?


iPostPlaceHolderArrayStart = 2
iArrayEnd = 100

for(int i=iPostPlaceHolderArrayStart; i<iArrayEnd; i++)
{
// Test
}

In this (albeit contrived) example I think comments would be far easier to understand, sure you could also use more descriptive names, especially if its a nested loop for example (bah use recursion! )

PrzemoF

Smoggy wrote: »

Could better named variables negate the need for comments in your example ?


iPostPlaceHolderArrayStart = 2
iArrayEnd = 100

for(int i=iPostPlaceHolderArrayStart; i<iArrayEnd; i++)
{
// Test
}

Those "placeholders" smell like a nasty workaround... Ooops, off-topic!

Smoggy

PrzemoF wrote: »

Those "placeholders" smell like a nasty workaround... Ooops, off-topic!

There is no doubt the first two positions of the array being used as a place holder is bad, But we will ignore that.

How about this, that when the code needs to get updated, lets say another developer uses array position 2 as a place holder. In this case the code AND the comment needs to be updated.

More often than not the code gets updated, but the comments left or someone moves this block of code furthur up the function, but left the comments in situ. In my world this isn't a situation. No comments, no need to update them.

In my example :

iPostPlaceHolderArrayStart = 3
iArrayEnd = 100

for(int i=iPostPlaceHolderArrayStart; i<iArrayEnd; i++)
{
// Test
}

Colonel Panic

I've seen arrays with the size of the array in the first element...

GreeBo

Smoggy wrote: »

There is no doubt the first two positions of the array being used as a place holder is bad, But we will ignore that.

How about this, that when the code needs to get updated, lets say another developer uses array position 2 as a place holder. In this case the code AND the comment needs to be updated.

More often than not the code gets updated, but the comments left or someone moves this block of code furthur up the function, but left the comments in situ. In my world this isn't a situation. No comments, no need to update them.

In my example :

iPostPlaceHolderArrayStart = 3
iArrayEnd = 100

for(int i=iPostPlaceHolderArrayStart; i<iArrayEnd; i++)
{
// Test
}

So your argument is to avoid the case where someone needs to read the code because the comment is out of date, you want someone to have to read the code everytime?!

Presumably the person who changed the code had to read the comment to figure out what to change, if they then dont update the comment to reflect the change they are a goon, and poor comments are probably the least of your worries with their code.

BrokenArrows

Lol..I love the hatred of the placeholder example. It was just some random thing I thought of on the spot that would require some explanation.

Smoggy

GreeBo wrote: »

So your argument is to avoid the case where someone needs to read the code because the comment is out of date, you want someone to have to read the code everytime?!

Presumably the person who changed the code had to read the comment to figure out what to change, if they then dont update the comment to reflect the change they are a goon, and poor comments are probably the least of your worries with their code.

In the original example you were going to have to read the comment and the code, my point is well written code is easy to read and makes comments redundant in many cases.

The example of updating the code and not the comment is extreme, but highlights how they are needed to be kept in sync and together when I suggesting that you don't need comments.

Just read the code, job done.

conor.hogan.2

Creamy Goodness wrote: »

If the comments are good I won't need to read the code, therefore the comment should come first.

Comments can and often do "lie" though.

BrokenArrows

conor.hogan.2 wrote: »

Comments can and often do "lie" though.

True, but you can also have code that makes perfect sense but not be doing what was intended.

conor.hogan.2

BrokenArrows wrote: »

True, but you can also have code that makes perfect sense but not be doing what was intended.

Of course, but the code is always doing what is done. Whether that is right or not is another thing. But what the code says, the code does.

Smoggy

BrokenArrows wrote: »

True, but you can also have code that makes perfect sense but not be doing what was intended.

That's where your automated unit tests come in, the original developer who wrote the code, also wrote the unit tests. Therefore keeping the code always in aligment with what it was intended to do.

amen

That's where your automated unit tests come in, the original developer who wrote the code, also wrote the unit tests. Therefore keeping the code always in aligment with what it was intended to do

but both the code and the unit tests might agree but the code could be still be wrong in that it is not doing what its meant to.

Feathers

GreeBo wrote: »

Again, its counter productive to have everyone read and understand every bit of code they interact with. Do you read your way down through every part of java.io package before you read from a file, or do you read the appropriate javadoc/api?

Conversely, do you read all the way through the JavaDocs of IO to find the method you want to use?

Generally, I'm coming at it from the other direction - if I know roughly what a class is doing, I know roughly the operation that I want to do on its object & look for a method signature to match. You only need comments here if the method signature is confusing or has side effects.

One good example is the Joda DateTime class - if I trust that its plus and minus methods work in a sensible way & are thread safe, I can happily call plusMonth(1) just from the method name. If I don't trust that it will handle year-ends correctly, I'd have to dig into the code anyway, even if I had a comment.

IMO, I'd go for comments to show the why rather than the what - if you can't read what it's doing, I'd split the inner workings into smaller private methods. For things like deprecation though, you can't tell be reading the annotation why the use of the method should be phased out.

Even most other 'why' comments can sit happily in VCS commit logs too, where you can have his issue number (why he made the change), his summary of changes & a reason for the approach & a diff against the last revision.

Not a person wrote: »

When starting this thread i could never have anticipated the amount of comments it would generate.

& they were generated neither to the right nor above the original post :P

GreeBo

amen wrote: »

but both the code and the unit tests might agree but the code could be still be wrong in that it is not doing what its meant to.

If the tests are passing then you have to assume the code is doing what the developer intended, that may not tie in with the requirements, but thats a whole different argument.

Feathers wrote: »

Conversely, do you read all the way through the JavaDocs of IO to find the method you want to use?

No, I read the exposed interface and assume that the rest of it works as intended, most often i dont care about the details of the inner workings unless something is not working as I expected.

Feathers wrote: »

Generally, I'm coming at it from the other direction - if I know roughly what a class is doing, I know roughly the operation that I want to do on its object & look for a method signature to match. You only need comments here if the method signature is confusing or has side effects.

One good example is the Joda DateTime class - if I trust that its plus and minus methods work in a sensible way & are thread safe, I can happily call plusMonth(1) just from the method name. If I don't trust that it will handle year-ends correctly, I'd have to dig into the code anyway, even if I had a comment.

But I dont think anyone is arguing with you about this, no one is saying comment everything, rather comment the bits that would benefit from an explanation, should someone want to read it.

Feathers wrote: »

IMO, I'd go for comments to show the why rather than the what - if you can't read what it's doing, I'd split the inner workings into smaller private methods. For things like deprecation though, you can't tell be reading the annotation why the use of the method should be phased out.

Even most other 'why' comments can sit happily in VCS commit logs too, where you can have his issue number (why he made the change), his summary of changes & a reason for the approach & a diff against the last revision.

Yes you should always explain what/why you made changes in your commits but I dont want to have to trawl through commits to find out the basic reason for something, only if I need to track it back down to a requirement change/defect etc.