Is code commenting bad?

Page 3 - Seeking answers? Join the AnandTech community: where nearly half-a-million members share solutions and discuss the latest tech.

EagleKeeper

Discussion Club Moderator<br>Elite Member
Staff member
Oct 30, 2000
42,591
5
0
Do you really mean the "thought process of the implementation"? The design should be properly conveyed in the design artifacts.

You say potAto, I say potato.

Design artifacts are not always existing and/or available.

Reason for the module and any special information/decision making that went into the structure of the code.
 

Cogman

Lifer
Sep 19, 2000
10,278
126
106
You're code should be readable and your comments should contain the whys and hows (Why am I looping here, How am I trying to process this data, etc) But not the whats (This is a for loop that visits each node, etc).

Most functions that are 100+ lines should have comment headers describing what the function does, what it takes as inputs, what it outputs, what happens when it errors out, and if using this function has any side effects on the object or the input variables.

Things I don't want in comments
* Really basic stuff like "assign 1 to i"
* non-working Code
* revision data (I really don't care that this is version 1.0.0.1 of the code or what fixes have been done to this code. That should all be documented in the revision control)
 
Last edited:

purbeast0

No Lifer
Sep 13, 2001
52,931
5,803
126
personally, i disagree about block comments above functions, describing the input and output. the names of the function and variables should be descriptive enough that you don't need this big ass comment block above.

if you have the following function:

Address getPersonsAddress(int personId, String firstName, String lastName)

it would be 100&#37; useless and a waste of time adding in a block comment describing that the function gets a persons address, describing what it takes a perons ID, their first name, and their last name as parameters, and returns their address. waste of time and real estate.
 

Train

Lifer
Jun 22, 2000
13,863
68
91
www.bing.com
personally, i disagree about block comments above functions, describing the input and output. the names of the function and variables should be descriptive enough that you don't need this big ass comment block above.

if you have the following function:

Address getPersonsAddress(int personId, String firstName, String lastName)

it would be 100% useless and a waste of time adding in a block comment describing that the function gets a persons address, describing what it takes a perons ID, their first name, and their last name as parameters, and returns their address. waste of time and real estate.

The comment should describe what happens when the PersonID is NOT found... return empty address, throw an error, etc. Or does it do a "smart" search on the names, i.e. "Richard" == "Dick"

I dont see how either of those could fit in a parameter name unless it is obscenely long.

They are especially handy when you have the xml comments that let you view then via intellisense when using the method elsewhere.
 

Pia

Golden Member
Feb 28, 2008
1,563
0
0
Address getPersonsAddress(int personId, String firstName, String lastName)

it would be 100&#37; useless and a waste of time adding in a block comment
In addition to what Train said, this is a confusing interface; why should I enter both an ID and a name? What happens if I enter a valid ID and a valid name of two different people? If the interface is like this for a reason and cannot be refactored to something self-explanatory, it needs comments.
 
Last edited:

AyashiKaibutsu

Diamond Member
Jan 24, 2004
9,306
3
81
I comment classes/functions for the sake of autodocumenters. If I have to do something weird or there's a compact but complicated algorithm I'll throw comments in. Otherwise, if you need a lot of comments it probably means your function/variable naming is bad.
 

Madwand1

Diamond Member
Jan 23, 2006
3,309
0
76
When I programmed in assembler, I sometimes used C code for commenting (actually scripted it first in C and then translated that to assembler). And similarly sometimes when I'm programming in a higher-level language, I might script it out first in an even high-level language, like English, and leave the script around as comments. Am I a 'weak developer' if/when I do this? I don't think so. It's just a difference in what level you want to work at mentally at what time, and often it's better to work mostly at the conceptual level and treat the implementation as a translation of that. And while code is the substance, sometimes all I need to learn when looking at some foreign code is not the why or the how, but just the what the heck it means to do.

That said, lately I've been writing code without any comments at all, as a side-effect of the atmosphere of the workplace, and I find that it sucks, as can I have a hard time even getting back into my own code when the structure is non-trivial and the thought behind it has gone.
 

purbeast0

No Lifer
Sep 13, 2001
52,931
5,803
126
In addition to what Train said, this is a confusing interface; why should I enter both an ID and a name? What happens if I enter a valid ID and a valid name of two different people? If the interface is like this for a reason and cannot be refactored to something self-explanatory, it needs comments.

i was just giving that as an example for explanation purposes, not saying it's a real world function because like you said, you shouldn't need all that info. a real world function would ONLY take a personID. but the same explanation goes for that.

you could very easily leave a 1 line comment when a person ID is not found, right around the return statement instead of making a block comment at the top.

very rarely should a block comment be used before a function. if you are doing a full block comment for getters and setters, you are doing it wrong.
 

Cogman

Lifer
Sep 19, 2000
10,278
126
106
very rarely should a block comment be used before a function. if you are doing a full block comment for getters and setters, you are doing it wrong.

And I disagree with this statement. Getters and setters and very simple functions are about the only thing that I would support having no block comments on.

Block comments are easy to ignore, don't clutter the code, and can be very useful for complex function usage (especially if some autodocumentation tool is being used).
 
Last edited:

purbeast0

No Lifer
Sep 13, 2001
52,931
5,803
126
And I disagree with this statement. Getters and setters and very simple functions are about the only thing that I would support having no block comments on.

Block comments are easy to ignore, don't clutter the code, and can be very useful for complex function usage (especially if some autodocumentation tool is being used).

if it's a very complex function, yes i agree they are okay, especially if you are adding em for documentation and for javadocs to pull em up.

but at the same time, if you have a ton of complex functions in one class, you may be doing it wrong and should look at possibly breaking the function down into more simpler functions.
 

purbeast0

No Lifer
Sep 13, 2001
52,931
5,803
126
on another note, i'm actually working with some 3rd party software at work that i'm integrating into our application. they have some API calls such as getField(String st, int i1, int i2, int i3) and they have absolutely no javadoc documentation or comments in their API and it has been a total fucking pain in the ass trying to work with it lol.

but me personally, i think the problem could be solved by simply having better variable names instead of adding a big comment. but still if you are creating an API for other people to use, then the javadoc comments should DEFINITELY be before functions that aren't getters and setters.
 

Aikouka

Lifer
Nov 27, 2001
30,383
912
126
on another note, i'm actually working with some 3rd party software at work that i'm integrating into our application. they have some API calls such as getField(String st, int i1, int i2, int i3) and they have absolutely no javadoc documentation or comments in their API and it has been a total fucking pain in the ass trying to work with it lol.

but me personally, i think the problem could be solved by simply having better variable names instead of adding a big comment. but still if you are creating an API for other people to use, then the javadoc comments should DEFINITELY be before functions that aren't getters and setters.

I have that problem at work with people that write some of the APIs that I have to use. After all of the pestering that I've done over it in the past, I think they've finally gotten to the point where they write Javadocs without me having to remind them. Although, it was kind of funny when a newer employee told me that she had an easier time understanding the API by reading the comments in my code where I implemented it.

Although, I did find a few spots on Friday where I forgot to update the Javadocs after making a variable type change. Shame on me!
 

CuriousMike

Diamond Member
Feb 22, 2001
3,044
543
136
A lot of programmers must think their sh*t doesn't stink.

"Why, my code is so super-awesome it's obvious to anyone but a nimrod what I'm doing."
 

iCyborg

Golden Member
Aug 8, 2008
1,327
52
91
A lot of programmers must think their sh*t doesn't stink.

"Why, my code is so super-awesome it's obvious to anyone but a nimrod what I'm doing."
Maybe it's about job security. If no one knows what the heck is some module doing but you, they'll think twice before sacking you, right?
 

Ichinisan

Lifer
Oct 9, 2002
28,298
1,234
136
Maybe this is an example of when simple code still needs comments?

Leap year rules:
  1. Every year divisible by 4 is a leap year.
  2. But every year divisible by 100 is NOT a leap year
  3. Unless the year is also divisible by 400, then it is still a leap year.

It took me a while to get the logic right because I keep reading the comparisons incorrectly. From what I can tell, this is perfect:

HTML:
<script type="text/javascript">
function isLeap(year){
	if(year&#37;4){return false;}
	if(year%100){return true;}
	if(year%400){return false;}
	return true;
}
</script>

...but it's still confusing to read, for me. Maybe it's because I don't code professionally, but I just keep reading the first condition as "if year is evenly divisible by 4 ..." It should actually read: "if year is NOT evenly divisible by 4 ..."

Sure, the code can be more readable by re-writing each condition like "if(year%4!=0){...}", but that would impact performance with additional and unnecessary comparisons to 0. If I used this code in the future as part of some loop for checking millions of records, there could be a significant performance impact.
 
Last edited:

TecHNooB

Diamond Member
Sep 10, 2005
7,460
1
76
Maybe it's about job security. If no one knows what the heck is some module doing but you, they'll think twice before sacking you, right?

I've actually brought that up at work. There are, however, some really oldschool coders who actually go back through their code and delete all their comments out of habit. It used to make things faster and save space..
 

dighn

Lifer
Aug 12, 2001
22,820
4
81
Sure, the code can be more readable by re-writing each condition like "if(year%4!=0){...}", but that would impact performance with additional and unnecessary comparisons to 0. If I used this code in the future as part of some loop for checking millions of records, there could be a significant performance impact.

I always explicitly compare results of "%" with 0 or 1 for clarity, but then with compiled code there's no performance penalty.
 

Aikouka

Lifer
Nov 27, 2001
30,383
912
126
Sure, the code can be more readable by re-writing each condition like "if(year%4!=0){...}", but that would impact performance with additional and unnecessary comparisons to 0. If I used this code in the future as part of some loop for checking millions of records, there could be a significant performance impact.

While some languages allow you to perform a Boolean check on an integer, I'd argue that it fails a readability test. Although, do note that I said some languages. For example, Java does not allow integer operations to be checked like that.
 

AyashiKaibutsu

Diamond Member
Jan 24, 2004
9,306
3
81
While some languages allow you to perform a Boolean check on an integer, I'd argue that it fails a readability test. Although, do note that I said some languages. For example, Java does not allow integer operations to be checked like that.

No strongly typed language should allow it. I'd also question whether it's actually doing an extra operation on a machine level.
 

Markbnj

Elite Member <br>Moderator Emeritus
Moderator
Sep 16, 2005
15,682
13
81
www.markbetz.net
Maybe this is an example of when simple code still needs comments?

Leap year rules:
  1. Every year divisible by 4 is a leap year.
  2. But every year divisible by 100 is NOT a leap year
  3. Unless the year is also divisible by 400, then it is still a leap year.

It took me a while to get the logic right because I keep reading the comparisons incorrectly. From what I can tell, this is perfect:

HTML:
<script type="text/javascript">
function isLeap(year){
	if(year%4){return false;}
	if(year%100){return true;}
	if(year%400){return false;}
	return true;
}
</script>

...but it's still confusing to read, for me. Maybe it's because I don't code professionally, but I just keep reading the first condition as "if year is divisible by 4 ..." It should actually read: "if year is NOT divisible by 4 ..."

Sure, the code can be more readable by re-writing each condition like "if(year%4!=0){...}", but that would impact performance with additional and unnecessary comparisons to 0. If I used this code in the future as part of some loop for checking millions of records, there could be a significant performance impact.

That code is completely comprehensible to me just based on the function name.
 

PricklyPete

Lifer
Sep 17, 2002
14,714
164
106
I only comment for two reasons:

1) The code is complex/tricky and needs some explaining
2) I need to explain why I chose to do it a certain way (to avoid a programmer who is maintaining the code second guessing why I did something).

If the code is straightforward, I let it speak for itself.
 

iCyborg

Golden Member
Aug 8, 2008
1,327
52
91
Maybe this is an example of when simple code still needs comments?

Leap year rules:
  1. Every year divisible by 4 is a leap year.
  2. But every year divisible by 100 is NOT a leap year
  3. Unless the year is also divisible by 400, then it is still a leap year.

It took me a while to get the logic right because I keep reading the comparisons incorrectly. From what I can tell, this is perfect:

HTML:
<script type="text/javascript">
function isLeap(year){
	if(year%4){return false;}
	if(year%100){return true;}
	if(year%400){return false;}
	return true;
}
</script>

...but it's still confusing to read, for me. Maybe it's because I don't code professionally, but I just keep reading the first condition as "if year is divisible by 4 ..." It should actually read: "if year is NOT divisible by 4 ..."

Sure, the code can be more readable by re-writing each condition like "if(year%4!=0){...}", but that would impact performance with additional and unnecessary comparisons to 0. If I used this code in the future as part of some loop for checking millions of records, there could be a significant performance impact.
That code is completely comprehensible to me just based on the function name.
I gotta agree with Ichinisan on this one - I think a short comment would help here, either that or explicitly comparing to 0. At first look I made the same mistake of reading "year%4" as 'year divisible by 4', only when I continued reading his post I realized the code is correct afterall. If one is superficially looking at this, I think it's too easy a mistake to make.
 

Train

Lifer
Jun 22, 2000
13,863
68
91
www.bing.com
I gotta agree with Ichinisan on this one - I think a short comment would help here, either that or explicitly comparing to 0. At first look I made the same mistake of reading "year%4" as 'year divisible by 4', only when I continued reading his post I realized the code is correct afterall. If one is superficially looking at this, I think it's too easy a mistake to make.

I think what Markbjn means by "totally comprehensible based on the function name" is that as a CONSUMER of the function, you don't need to know what it going on inside the method. Treat it as a "black box". In a function such as this, which has a single strongly typed parameter and a boolean response, with zero outside dependencies, the internals should never have to be worried about again, once the function is written and tested the first time.
 

Markbnj

Elite Member <br>Moderator Emeritus
Moderator
Sep 16, 2005
15,682
13
81
www.markbetz.net
I think what Markbjn means by "totally comprehensible based on the function name" is that as a CONSUMER of the function, you don't need to know what it going on inside the method. Treat it as a "black box". In a function such as this, which has a single strongly typed parameter and a boolean response, with zero outside dependencies, the internals should never have to be worried about again, once the function is written and tested the first time.

That's a good point too, but not exactly what I meant. My background as an old C++ programmer is probably what explains my take on this. When I look at the function name I know immediately that it will return true if the year is a leap year. You're right that I don't need to know more than that if it is working correctly. But moving on to look at the body, I am quite used to mentally evaluating boolean conditions, and to regarding 0 as boolean false. The form of the statements in the test is so familiar to me that it reads immediately as 'if the year modulo the literal integer results in a non-zero value then return false.'

Also, I don't like the phrase "not divisible." What Ichinisan should have said is "not evenly divisible." The mod operator should not be cryptic to a good programmer, and I wouldn't hire anyone who couldn't look at this code for five seconds and tell me exactly what it is doing (not to put Ichinisan in that category, by any means, I'm simply being picky about what the mod operator does).

Lastly, in terms of respect, someone who doesn't comment code like this will still get more of it from me than someone who writes code like this without hitting the spacebar from time to time. .
 
sale-70-410-exam    | Exam-200-125-pdf    | we-sale-70-410-exam    | hot-sale-70-410-exam    | Latest-exam-700-603-Dumps    | Dumps-98-363-exams-date    | Certs-200-125-date    | Dumps-300-075-exams-date    | hot-sale-book-C8010-726-book    | Hot-Sale-200-310-Exam    | Exam-Description-200-310-dumps?    | hot-sale-book-200-125-book    | Latest-Updated-300-209-Exam    | Dumps-210-260-exams-date    | Download-200-125-Exam-PDF    | Exam-Description-300-101-dumps    | Certs-300-101-date    | Hot-Sale-300-075-Exam    | Latest-exam-200-125-Dumps    | Exam-Description-200-125-dumps    | Latest-Updated-300-075-Exam    | hot-sale-book-210-260-book    | Dumps-200-901-exams-date    | Certs-200-901-date    | Latest-exam-1Z0-062-Dumps    | Hot-Sale-1Z0-062-Exam    | Certs-CSSLP-date    | 100%-Pass-70-383-Exams    | Latest-JN0-360-real-exam-questions    | 100%-Pass-4A0-100-Real-Exam-Questions    | Dumps-300-135-exams-date    | Passed-200-105-Tech-Exams    | Latest-Updated-200-310-Exam    | Download-300-070-Exam-PDF    | Hot-Sale-JN0-360-Exam    | 100%-Pass-JN0-360-Exams    | 100%-Pass-JN0-360-Real-Exam-Questions    | Dumps-JN0-360-exams-date    | Exam-Description-1Z0-876-dumps    | Latest-exam-1Z0-876-Dumps    | Dumps-HPE0-Y53-exams-date    | 2017-Latest-HPE0-Y53-Exam    | 100%-Pass-HPE0-Y53-Real-Exam-Questions    | Pass-4A0-100-Exam    | Latest-4A0-100-Questions    | Dumps-98-365-exams-date    | 2017-Latest-98-365-Exam    | 100%-Pass-VCS-254-Exams    | 2017-Latest-VCS-273-Exam    | Dumps-200-355-exams-date    | 2017-Latest-300-320-Exam    | Pass-300-101-Exam    | 100%-Pass-300-115-Exams    |
http://www.portvapes.co.uk/    | http://www.portvapes.co.uk/    |