Bad uses of comments:
States the obvious, therefore redundant.
//initialize variables
x = 10;
Proper layout standards and headers.
/************************************************************************************
Globals
*************************************************************************************/
Consider writing a standards document once, no need to copy and paste standards into every file. Should be obvious in language where external files are included, globals are declared, etc. Don't state the obvious.
Alerts user to all assumptions made. Problem is with code, comments don't "fix" the problem. Who reads 'em anyway? Use ASSERT, CASSERT (compile time assert), or even make impossible with refactoring.
//does not work with negative numbers
f(int x){}
Comment should be eliminated and action taken to remove assumption.
f(unsigned x){}
or
f(int x){
ASSERT(x > 0 && "function assumes x > 0")
}
or return an error code
enum Errors{ERROR_OK, MUST_BE_POSITIVE};
Errors f(int x){}
Clarifies obscure code. Problem is with code, comments don't "fix" the problem. Who reads 'em anyway?
//hash for fast lookup
char a = b[0] ^ b[1] & c[1] | d[5];
Fix with refactoring and better naming
GetHash(...){...}
char hash = GetHash(...);
Example 2:
//qsort
void Sort(...){}
Better naming. Just a matter of moving the comments into the identifiers involved.
void QSort(...){}
Signals end delimiter. Shows lack of proper indentation.
#ifdef X
#ifdef Y
...
#else // Y
...
#endif // Y
#endif // X
Fix indentation and get an IDE that colors dead code so you can see if you don't have one.
#ifdef X
# ifdef Y
...
# else
...
# endif
#endif
or
while(){
for(){
if(){
do{
if(){
}//if
}while();
}//if
}//for
}//while
Consider refactoring deep nesting instead.
void Inner(...){
if(){
do{
if(){
}
}
}
}
while(){
for(){
Inner(...);
}
}
Function return documentation.
//returns -1 if less than, 0 if equal, 1 if greater than
int compare(...){}
Use enumerations.
enum CompareReturns{CR_LESS, CR_EQUAL, CR_GREATER};
CompareReturns compare(...){}
or
//test for bad file
if(file == -1)
Use named constants
const handle BAD_FILE;
if(file == BAD_FILE)
Reminders. Not the best way to remind.
//TODO: Add something here
Use pragma
#pragma message("TODO: Add something here")
//if you modify this array, you will need to also modify array2
Use CASSERT.
CASSERT(sizeof(array) / sizeof(array[0]) == sizeof(array2) / sizeof(array2[0]) && "Must synchronize arrays")
//should not get here
Use ASSERT.
ASSERT(0 && "Should not get here")
//do not modify this variable
Use access modifiers, ASSERTS and remove dangerous global variables.
Integrated documentation.
//this variable stores the current directory
char dir[MAX_PATH];
Rename variable so documentation will not have to be read.
char g_currentDirectory[MAX_PATH];
//function returns result of b + c in a
void f(int &a, int b, int c);
Rename identifiers so documentation will not have to be read.
void Add(int &result_out, int operand1, int operand2);
or safer
void Add(int *result_out, int operand1, int operand2);
Program that makes documentation automatically reads comments. External documentation should be redundant if you code correctly.
Modified log. Shows lack of source control. Get it!
Remove unused code you might want to revert back to or worse, code that didn't work and got replaced. Shows lack of source control (if you don't have it, get it) or laziness (hit delete).
//memcpy(dest, src, count * 4);
memcpy(dest, src, count * sizeof(int*));
Comment obscure optimized code. This is assembly in which case does not qualify as "Bad C++ comment", or you underestimate your compilers optimizing capabilities.
The flip side. Valid uses of comments.
Your job requires it. Don't get yourself fired.
Document obscure external API calls.
//set valid processors this thread can run on. pass in bitmask of processors
SetThreadAffinity(...);
Distribute the API's documentation and look this stuff up. If it is as easy as hitting F1, don't comment. If bad documentation, comment is helpful. A valid question may be: "Why are you using an API with poor documentation?"
Comment out single line of test/debug code. Prefer #if 0 though over /**/ since inline comments can't nest. Also consider using #ifdef _DEBUG instead of comment. Will be less error prone if you don't need in release testing/debugging.
Comments to facilitate searching.
//***globals***
Temporary comments as placeholders for code while constructing.
My conclusion is that comments show deficiencies in code or lack of proper tools. When you have no control over the code, comments are ok.
I'm interested in any exceptions to the following rules you can come up with. I would also like to take a poll. If you read this post, please vote so you don't skew the results. Just put at the start of your reply "I prefer comments" or "I prefer no comments". The I prefer no comments assumes the rules above are followed making the comments unnecessary.
Edit: added another example
[Edited by - KodeWarrior on March 21, 2007 6:36:45 PM]
