Original Post
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 Consider refactoring deep nesting instead. 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]
while(){
for(){
if(){
do{
if(){
}//if
}while();
}//if
}//for
}//while
void Inner(...){
if(){
do{
if(){
}
}
}
}
while(){
for(){
Inner(...);
}
}
