Skip to main content
gamedev.net
🔒 Locked

On Comments

Started by M2tM 21 replies 3.9k views
Original Post
M2tM
M2tM
Hey guys and girls, I found this while perusing the internet and I figured it would be an interesting topic to open for discussion here. Notice: I am a nefarious blog necromancer responding to a post in 2007, but the topic is interesting anyway: Original Post And my response is in the comments with my real name (Michael Hamilton) Since then he has made another blog posting: Comment the Why not the What Which does introduce some of the potential places for comments certainly, but he asked for a followup on my thoughts and I've posted it, though it is awaiting moderation. Here it is in summary a combination of experience and reading on the subject. I am interested in what other people think about this topic. The code sample I have written is between 5% and 6% comments, but I imagine about half of that is made up of comment blocks to separate class implementations for easier scrolling in .cpp files which have more than one class in them. Certainly I do comment where confusion needs to be avoided with a comment, but I try my best to steer around explaining things in text when I can better explain it and more accurately in code. Here is my response, it is lengthy so I'm putting it in source tags because it may be authenticated on the other site later: 

You have the exact right idea in your parting thought, but comments are not
 exactly the way it should be gone about. All too often they are a clumsy 
clutch for a poorly designed interface. Typically a well designed application
 will have very few comments and the ones it does have will be edge cases which
 have no other clear method of explanation.

The understanding of an application’s flow should be well defined and easy to
 follow by the interfaces presented. Functions and methods should be short, 
and then even shorter than that. Assuming you are developing in an object 
oriented language you should strive to follow the principles mentioned here: 
http://butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod


In particular you will notice your classes are smaller and easier to read.

Assuming you are in a strictly typed language you can make use of this in your 
interfaces to clear ambiguity between argument lists. You should however strive
 to have no more than 3 arguments per function, 2 is good, 1 is better and 0 
are ideal when possible.


Each function should deal with 1 level of abstraction and call other functions
 to deal with further complexity. In this way you can easily follow the 
function calls and get a very strong understanding of what everything does. 
With sanely named functions and variable names this method of coding actually 
completely negates the need for -most- comments.


Notice, however, I never said that comments should -never ever- be used. I only 
said you should not strive for them and certainly they are a terrible quality 
metric. Comments indicate a lack of ability to express yourself in code. 
Wherever you have written a comment it is because the functionality of that 
code segment is not otherwise clear. This strongly implies a problem with the 
code, I hope you agree with that. There are some important places for comments 
though and you did cover pretty much all the valid cases here, but wherever you
 make a comment do a double take and see if a few minutes re-factoring can’t 
clear up your meaning and avoid the need for a comment.


I also completely understand that there is a strong need to “Just Make It Work"
 ™ which is where many comments and kludges pop up and I have professional 
experience with game programming and with web development (I am currently 
working for Squareflo new media http://www.squareflo.com and am the primary 
developer on our cornerstone CMS project responsible for almost all our new 
websites and deployed across multiple servers and client sites).


When releasing for a deadline there isn’t always time to make all code you 
write immaculate, however any code which is being built on top of should be 
cleaned up and properly re-factored (if it needs it) first or dependency will 
stack up and it will become a severe liability later. Programming is a lot like
 writing in that you rarely get it right in the first draft, in fact 
re-factoring is a vital step which is often missed by less experienced 
developers.


The primary difficulty is when working with a “messy" language like PHP where 
clean code can be written, but is much more difficult to enforce and variables 
can be passed containing literally -anything- comments become a necessary evil.
 I call them evil because in the ideal situation your code should read and 
express intent clearly. When written properly other developers should not have 
difficulty reading through your code, complicated bits should be realized as 
deeper abstractions, wrapped in functions with descriptive names and used 
appropriately. In this way the function name becomes a very powerful 
documenting tool.


All of this aside, however, understanding the gist of a system without having 
to read code-level is a seperate concern and thus having a clearly written 
project outline which can be viewed seperate from the code is useful. Also, 
small but useful hints at the headers of interfaces can be useful. There are 
certainly places where a well-placed comment will save some trouble in 
understanding how a set of classes is utilized in a large system. But let me be
 clear, my original post was primarily to disagree with the use of comments to 
describe code in detail such that a layperson could understand it. This type of 
commenting is actually bad, crufty, and prone to error. As I am also in a 
position where I assist in hiring new programmers I am simply giving my two 
cents on the subject.


But you have presented an interesting and well written blog otherwise so I am 
not trying to pick you apart here. I wouldn’t spend the time commenting like 
this if I weren’t interested in your opinion or in attempting to present a 
different way of thinking (even if you simply consider my points seriously and 
then dismiss them.)


The primary places I use comments is in providing a visual break between 
classes, and providing small seed comments that will save time in understanding
 why I implemented something in a particular way. In less well-written cases I 
will sometimes comment what a particular parameter’s values can be, but usually 
you want to avoid having flag parameters in the first place as that indicates a 
function may be taking on too much.


I do not absolve myself of any problems or messy code, but certainly if you 
would like a comprehensive code sample of what I believe is a clean program I 
have a c++ project I recently did for a code sample in presenting to game 
development studios for a resume I’m in the process of compiling. E-mail me if 
you are interested, I am actually looking for thoughts on it anyway.
Anyway, thanks for reading this, I am also interested in getting some other people to take a look at the code sample (a basic 2d platform game with physics and sound) I've crafted as I'll be using it as a highlight in my portfolio shortly. If you are willing to take a look in any amount of detail (skim or up close) pm me and I will send a link.
_____"You're using a screwdriver to nail some glue to a ming vase. " -ToohrVyk
Trapper Zoid
Trapper Zoid
I'm with the original blog authors on this. I prefer code to have their reasoning spelled out in comments. Certainly you should use clear variable and function names, but that usually just tells you the "what", not the "why". The "why" comments are important, because in a few months time when another developer or the future you looks at the code it helps enormously to know what you were thinking.
capn_midnight
capn_midnight
I like seeing comments in code. I don't typically get to see enough to have a preference for what the comments are on. But when I see comments, I can rest reasonably assured that the previous coder cared about those who would come after him. He cared to try to do his due diligence, and the code usually reflects that.

One thing I do hate, though: seeing large chunks of code commented out. Entire functions called "DoSomething2" commented out, nestled between a still active "DoSomething1" that isn't called anymore, and a "DoSomething3" that is. It's not literally "DoSomething", but it is the same name, and they are all numbered sequentially. That pisses me off, because it means someone who didn't know how to use his tools (source control) was coding.
owl
owl
As MacDonalds does with it's employees, in large projects, programmers should be forced to follow certain commenting and variable/function naming standards because, otherwise, sooner or later the code will look like written by a crazy person.
[size="2"]I like the Walrus best.
M2tM
M2tM
Quote:
Original post by capn_midnight
I like seeing comments in code. I don't typically get to see enough to have a preference for what the comments are on. But when I see comments, I can rest reasonably assured that the previous coder cared about those who would come after him. He cared to try to do his due diligence, and the code usually reflects that.

One thing I do hate, though: seeing large chunks of code commented out. Entire functions called "DoSomething2" commented out, nestled between a still active "DoSomething1" that isn't called anymore, and a "DoSomething3" that is. It's not literally "DoSomething", but it is the same name, and they are all numbered sequentially. That pisses me off, because it means someone who didn't know how to use his tools (source control) was coding.


Dead code comments are one of the worst offenders. They introduce horrendous code rot. Providing the original commenter doesn't go back and remove the commented code or do something with it you will rarely see another person delete it. Was it useful? Will it be useful? Nobody knows, and I'll be damned if I'm the one to delete it. Even a descriptive comment nearby with advice on what that is commented out for may become out of date and completely irrelevant as methods change and the once possibly useful code becomes a large crufty comment. Getting a decent subversion solution and just deleting that kind of thing really helps.

Also, about resting assured based on comments, that can be dangerous. What happens when dependencies in other places change and the comments become invalid in subtle ways? The code won't lie, so if you look at that you won't be deceived and provided it is clean and easy to read you won't have any more trouble reading that than a comment. It is not as easy to write good code as it is to hack out a comment describing mediocre or bad code. People who write mediocre or bad code are also more apt to miss something in the comment. So chances are that the comment is either 1: out of date, 2: inaccurate, 3: useless or 4: all of the above. And furthermore, by relying on it you make assumptions that can cause a lot of headache later on. This has indeed happened to me on several occasions.

My main point is that comments are not something to strive towards, or something to lean on and sigh with relief when something isn't clear in the code. They should be the ultimate fallback and really do indicate a failure to communicate in code what your purpose is. If you can't tell by glancing at the code chances are there is a problem and a comment will only mask that problem by providing a (necessarily) inaccurate abstraction as to intent. I say inaccurate because the accuracy is seen in the details. If you want the gist of what something is doing, properly named variables and methods should really tell you all you need to know in -most- situations.

I'm all for external API documentation and generated code graphs, however. This I think is a great thing to have as long as it is kept current. It can give you a quicker overview of how a class interacts than a few piecemeal comments scattered throughout the four corners of your application where they get ignored and lost and out of date.

Consolidated documentation, and clear code on the implementation is, for me, always preferable to lots of comments detailing how things work. The few comments that do exist should definitely be for weird cases, or un-clear dependencies which cannot easily be detailed, but should also be mentioned in the documentation.

Proper unit testing, clean code with well thought out names conforming to a standard, small functions representing a single level of abstraction and classes which don't try to do everything are the ideal.

External documentation with code examples for common use and a good over-view of the API help centralize any helper documentation and makes it a lot easier to maintain without having to root around for little bits of trivia all over the place in code.
_____"You're using a screwdriver to nail some glue to a ming vase. " -ToohrVyk
Trapper Zoid
Trapper Zoid
Quote:
Original post by M2tM
My main point is that comments are not something to strive towards, or something to lean on and sigh with relief when something isn't clear in the code. They should be the ultimate fallback and really do indicate a failure to communicate in code what your purpose is. If you can't tell by glancing at the code chances are there is a problem and a comment will only mask that problem by providing a (necessarily) inaccurate abstraction as to intent. I say inaccurate because the accuracy is seen in the details. If you want the gist of what something is doing, properly named variables and methods should really tell you all you need to know in -most- situations.

I really do not agree with this. Well written code, without the comments, will very clearly tell you what is going on at the atomic level. But it's extremely hard to encapsulate the "why" without some extra explanation in a comment. At best, in extremely clear code you can figure it out and reverse engineering the programmers thought process. But why go to that trouble when a simple clear comment will tell you in plain English?

For example, here's a snippet of code I found on my hard drive for reading in a font configuration file that I apparently wrote in 2001:

        // check if we have a digit (or a leading minus sign)if (isdigit(c) || ((c == '-') && (number_state == FALSE))){        number_state = TRUE;                if (digit < NUMBER_BUFFER_SIZE)                buffer[digit] = c;		// read in the character (if we are within the buffer size)        digit++;				}else{        if (number_state == TRUE)		// we have the end of a number!        {                number_state = FALSE;                                        // add a terminating zero to make this thing work                if (digit < NUMBER_BUFFER_SIZE)                        buffer[digit] = '\0';                digit = 0;					                        // now get the number!                number = atoi(buffer);                                                                        // now process the number depending where we are in the file reading                switch (file_state)                {                        case STATE_ASCII_CODE:                                        // we have read the ASCII code!                                ascii_code = number;                                file_state = STATE_FRAME_REFERENCE;                                break;                        case STATE_FRAME_REFERENCE:                                        // we have read the frame_reference!                                frame_ref = number;                                        // got all the information, so store it in the array                                if ((frame_ref >= 0) && (frame_ref < my_character_set -> NumberFrames()))                                        my_character_ref[ascii_code] = frame_ref;																				                                file_state = STATE_ASCII_CODE;                                                                break;                }        }			}


Even though a lot of the comments appear to be just repeating obvious things from the code, I find they make it much easier to figure out the intent behind the code. Take the first comment ("check if we have a digit (or a leading minus sign)") - that's pretty much exactly what the code says in the next line ("if (isdigit(c) || ((c == '-') && (number_state == FALSE)))"). However just reading the bit in English for "leading minus sign" instantly twigs me to why I put in the extra rule to check for c == '-'; I'm looking for negative numbers. I'd have figured that out with a bit thought, but the comment makes it so much easier.

You can think of it as like a Rosetta stone of code. You've got one description in code, and another in snippets of plain English. Together they're much easier to understand than alone.
KulSeran
KulSeran
1) I'd like to side with the "comments suck" side of things for 1 moment. Doxygen is NOT a replacement for documentation, and I've seen one too many times, source code with overly verbose comment blocks for a tool like Doxygen, and not a single comment that is worth it's weight in bytes.

2) On the "code rotted commented out code". Deleted!
If you intend to switch a block of code on/off at somepoint it better have a
BLAMEME_TODO: Fix this before ship
like tag, or be inside a #if #endif block.
And, even then it sometimes gets replaced with "// NOTE: removed change #9951"
Because if you think you might need some code again, that is exactly what version control is for.
The revisions you check into the main branch should be ready to use no questions asked, and no hunting down "why is this code a mess! what does it do!?"

3) Comment comment comment! I'm all for comments.
a) The fastest code isn't always the easiest to understand. Or, even if it is easy to understand the concept, following the code is going to take a while. Especially when shortcuts are taken, or assumptions made. If I know object X does Y, I could be tempted to just copy-paste the relevant code and use it (or make it a function i can call). But if the code for feature Y assumes things that aren't true for my object I could be walking into a nightmare of a debugging scenario. I'd be nice to know.

And on the note of fastest code. Virtual functions can be hella slow. Deep call chains can make things hella slow. Deep virtual function call chains are slow. And sure the compiler is smart and WILL inline your functions for you, but then again, it WONT inline a damn thing if you told it -O0 or the equivelent. So writing well formed OO code can mean your code is imposible to run in debug at a playable speed, and imposible to debug in a reasonable time in release mode. When debugging something I'd rather have good comments and a large block of code anyday to a deep call chain that got inlined. Because spending 10sec to comment a piece of code is well worth the 10+min it can save someone debugging.

b) The most elegant code may just have a LOT of code to dig around in. And a lot of nice, verbosely named functions might give meaning to the code, but without comments it CAN'T give a good overview of what the code on a whole does. Expecially in conjunction with generalized functions, where the context of the function means more than the function itself (sort() obviously sorts, but without digging into the predicate used, or having a comment, the line is almost meaningless)

c) A good IDE like vim has powerful code "folding" features that let you trim thousands of lines of code into just an outline. And, while a code outline should have code names that give you an idea of what you are looking at, a single line comment like "Finds the best target" can help you skip over large blocks of code inside a function. (and no, breaking up the function into more, smaller functions is not a fix all solution)

d) The most important comment ever, the "Creator: ME". Sure I can dig through source control, but it is nice to quickly know exactly who on the team to go hunting for when you need to know about a piece of code.
brent_w
brent_w
Quote:
Original post by Trapper Zoid
I'm with the original blog authors on this. I prefer code to have their reasoning spelled out in comments. Certainly you should use clear variable and function names, but that usually just tells you the "what", not the "why". The "why" comments are important, because in a few months time when another developer or the future you looks at the code it helps enormously to know what you were thinking.

My thoughts exactly. I second Trapper Zoid.
I'd like to add something, but he's covered it rather succinctly.

Edit:
I will say that the why is especially helpful to new coders.
Sure the reasoning might be obvious to someone with a few years on the job experience, but new hires have an uphill battle going into your code when the why is not included.
jtagge75
jtagge75
Its good to know why a particular piece of code is done the way it is. Maybe there are other factors that went into the way the code was done then just the code around it. Not everybody is the super programmer can always get the absolute correct way to do something. And maybe sometimes you just don't have the time and need to do a hack.

If you have no comments and just rely on fancy variable and function names you (or a coworker) is going to be wondering what you could have possibly been thinking at some point in the future. By the same token it is a waste of time to comment every little section of code when its quite clear what it does.
M2tM
M2tM
Trapper, that's what I call "messy" code. It's really kind of a headache to parse. You're a good programmer, I know this because I've seen you on the forums quite a bit, but this is not good code so don't try and put it forth as an example as to why comments are important. I'm sorry to say but of course you're going to need comments to work your way through something like that years later. Here are some code smells I'm getting:

1. Does it do one thing? No, it does many things with multiple branching statements.
2. What level of abstraction are we working at? It seems like we're doing a lot of different levels of stuff all in this one function (I presume function.)
3. Are the comments that exist meaningful? For the most part as you yourself said, no. They are "feel good" comments "we have the end of a number" "read in the character (if we are within the buffer size)" this type of thing is self-evident.

I can guarantee you can re-factor that and eliminate all the comments, I don't know if this is exactly what you were going for, but something like (and this is rough and considering I was missing a lot of information really about as good as I'm willing to do) Note I commented the places it could break where I acknowledge my lazyness. This is a clear failing on my part and the comment points that out. Note, I may have lost some meaning in the nitty gritty because I don't exactly know what the surrounding classes are like. Also depending on the file state, it may need to be a member instead of passed around.

For all of that I hope this is a decent example of what I'm trying to explain:

class FontConfiguration{public:   void loadFontConfigurationFile(std::string filename){      int digit = 0;      char currentCharacter;      FileStates currentFileState = STATE_ASCII_CODE;      //...logic to load the file and iterate for each character... pseudocode         parseStep(currentCharacter, digit, currentFileState);      //...endloop.  I am doing it this way because no method was supplied in the original code example.   }private:   enum FileStates{STATE_ASCII_CODE, STATE_FRAME_REFERENCE};   enum {NUMBER_BUFFER_SIZE = 1024, NUMBER_CHARACTER_SET_SIZE = 256};   void parseStep(char c, int &digit, FileStates ¤tFileState){      if(isCharacterANumber(c) || ((c == '-') && (workingWithANumber == false))){         workingWithANumber = true;         readCharacterIntoBuffer(digit, c);      }else if(workingWithANumber){         workingWithANumber = false;         addTerminatingCharacterToBuffer(digit);         processNumberFromBuffer(currentFileState);      }   }   void readCharacterIntoBuffer(int &digit, char c){      if (digit < NUMBER_BUFFER_SIZE){         buffer[digit] = c;      } //really should add error reporting      digit++;   }   void addTerminatingCharacterToBuffer(int &digit){      if (digit < NUMBER_BUFFER_SIZE){         buffer[digit] = '\0';      } //really should add error reporting      digit = 0;   }   bool isCharacterANumber(char c){      return (c > '0' && c < '9');   }   void processNumberFromBuffer(FileStates currentFileState){      int parsedNumber = atoi(buffer);      switch (currentFileState){         case STATE_ASCII_CODE:            currentFileState = handleAsciiCharacterAssignment(parsedNumber);            break;         case STATE_FRAME_REFERENCE:            currentFileState = handleAssignInformationToArray(parsedNumber);            break;      }      return currentFileState;   }   FileStates handleAsciiCharacterAssignment(int parsedNumber){      tmpAsciiCode = parsedNumber;      return STATE_FRAME_REFERENCE;   }   FileStates handleAssignInformationToArray(int frameReference){      if ((frameReference >= 0) && (frameReference < myCharacterSet->NumberFrames())){         characterReference[tmpAsciiCode] = frameReference;																				      }      return STATE_ASCII_CODE;   }   CharacterSetDefinition *myCharacterSet;   char characterReference[NUMBER_CHARACTER_SET_SIZE];      bool workingWithANumber;   int tmpAsciiCode;   char buffer[NUMBER_BUFFER_SIZE];};


Quote:
Original post by KulSeran
1) I'd like to side with the "comments suck" side of things for 1 moment. Doxygen is NOT a replacement for documentation, and I've seen one too many times, source code with overly verbose comment blocks for a tool like Doxygen, and not a single comment that is worth it's weight in bytes.


Essentially yes.

Quote:

2) On the "code rotted commented out code". Deleted!
If you intend to switch a block of code on/off at somepoint it better have a
BLAMEME_TODO: Fix this before ship
like tag, or be inside a #if #endif block.
And, even then it sometimes gets replaced with "// NOTE: removed change #9951"
Because if you think you might need some code again, that is exactly what version control is for.
The revisions you check into the main branch should be ready to use no questions asked, and no hunting down "why is this code a mess! what does it do!?"


Precisely.

Quote:

3) Comment comment comment! I'm all for comments.
a) The fastest code isn't always the easiest to understand. Or, even if it is easy to understand the concept, following the code is going to take a while. Especially when shortcuts are taken, or assumptions made. If I know object X does Y, I could be tempted to just copy-paste the relevant code and use it (or make it a function i can call). But if the code for feature Y assumes things that aren't true for my object I could be walking into a nightmare of a debugging scenario. I'd be nice to know.


See, but this is an excuse for bad code which is precisely why I don't advocate comments. Of course commenting bad code is necessary. But this is a terrible first point for advocating commenting over good readable code. I hope you can see and concede that much. Note that I am not against comments entirely in messy code because honestly poorly named things and huge methods that do more than one or two things need comments all over the place to explain all the side effects. This is a given. I am suggesting that the comment here is a bad crutch that is being leaned on where the proper solution (though not always the one taken) is to write better code. If a function needs a manual to understand it chances are it is taking on too many things and needs to be re-factored.

Quote:

And on the note of fastest code. Virtual functions can be hella slow. Deep call chains can make things hella slow. Deep virtual function call chains are slow. And sure the compiler is smart and WILL inline your functions for you, but then again, it WONT inline a damn thing if you told it -O0 or the equivelent. So writing well formed OO code can mean your code is imposible to run in debug at a playable speed, and imposible to debug in a reasonable time in release mode. When debugging something I'd rather have good comments and a large block of code anyday to a deep call chain that got inlined. Because spending 10sec to comment a piece of code is well worth the 10+min it can save someone debugging.


Performance is of course an issue in some cases, but realistically most situations the use of virtual functions is not the bottleneck and beyond this, the use of virtual functions is not actually directly related to writing clean code at all. But in any case I do respect the need to write difficult to understand performance intensive code for certain portions, but where speed trumps clarity you have to remember maintenance will be harder and comments will be necessary. Note I never ever say that comments should never be used, just that they are the ultimate fallback when shit gets tough.

Quote:

b) The most elegant code may just have a LOT of code to dig around in. And a lot of nice, verbosely named functions might give meaning to the code, but without comments it CAN'T give a good overview of what the code on a whole does. Expecially in conjunction with generalized functions, where the context of the function means more than the function itself (sort() obviously sorts, but without digging into the predicate used, or having a comment, the line is almost meaningless)


Absolutely there will be a lot of code to dig around in, but starting at the entry point for the api you should only see one level of abstraction. If you need to know more details you can simply look at the methods or classes in more detail and trace your way down until you're at the sufficient level of detail. Note that splitting things up like this actually removes a lot of complexity as you don't need to understand the entire problem to see a working outline of each primary function. It can also help discover redundancy and make more stuff re-usable which means fewer places for the code to break which means cleaner more readable code.

You can see "oh, it initializes here, then loads, then outputs then closes. Function done." Instead of seeing one function that does all that and handles the opening and closing and everything within that function. Which is easier to read? The one with one level of abstraction that isn't dealing with file handles and direct output in the same level you're organizing the flow of the operation clearly.

Quote:

c) A good IDE like vim has powerful code "folding" features that let you trim thousands of lines of code into just an outline. And, while a code outline should have code names that give you an idea of what you are looking at, a single line comment like "Finds the best target" can help you skip over large blocks of code inside a function. (and no, breaking up the function into more, smaller functions is not a fix all solution)


Yes, code outlines are very useful, and no breaking up a function into smaller functions is not -always- feasible. But if your function is doing more than one thing or is handling multiple levels of abstraction then it is and should be done. If your function does one thing and happens to be a bit longer there is no problem with that. Examples of such functions include very basic initializing constructors and the main function (or variation) which handles a lot of complexity just in the program flow by necessity. That said, steps should always be taken to ensure a single level of abstraction and where that is not reasonable to at least limit the layers of abstraction to a small number.

Quote:

d) The most important comment ever, the "Creator: ME". Sure I can dig through source control, but it is nice to quickly know exactly who on the team to go hunting for when you need to know about a piece of code.


Version control is more useful in hunting down individuals, but creator comments, if they don't take a mile of space are totally fine as a way of tagging files that may find their way to places out of your hands as a way of keeping credit.

Remember people, I'm not advocating the use of no comments, but am rather stating that they are over-used and should typically not be used when a way to express yourself better in code is available. Some situations do not make this feasible so I'm not going to argue against that. Clearly if you need information that cannot be represented in the language you will need to write a concise and accurate comment. But that said, they are much more often a blatant flag of failure on your part.

Think hard about this because it's a worthwhile point to consider. You don't need to be an evangelistic about this, just consider the points.
_____"You're using a screwdriver to nail some glue to a ming vase. " -ToohrVyk
Trapper Zoid
Trapper Zoid
Quote:
Original post by M2tM
Trapper, that's what I call "messy" code. It's really kind of a headache to parse. You're a good programmer, I know this because I've seen you on the forums quite a bit, but this is not good code so don't try and put it forth as an example as to why comments are important. I'm sorry to say but of course you're going to need comments to work your way through something like that years later.

I'm actually a pretty mediocre programmer, most of the time [smile]. Don't know if I'm technically better now; I've picked up some new skills in eight years, but I code a lot less. I was probably more fluent in C back then than now.

But that's sort of the point. You need a habit of writing comments as a safety net. I picked that code not because it was good, but because it was old, and I knew eight years ago I'd have a different style from what I do now. Younger programmer me however has the same habit in writing comments (although I hope I'm better at making them more "why" than "what")

Sometimes you write pretty hacky code. That was a snippet from a font configuration file loader. It's not the sort of thing you spend ages agonising over and streamlining. It's code you write once, test, integrate and forget about. Although that probably was from a hobby project so I probably skipped the "test" part [wink] (all I did was search my HD for "font.cpp" and took the first hit; a bit amazed I had that actually, given I've changed computers and operating systems thrice since then.)

Frankly, I found your code to be a harder read than my eight year old code. That's probably because we've got a different coding style; I'm more used to working on my own solo projects and have my own quirks on what variable names are used for (I don't like massively long multiple word variable names, not for common use at least). But you've used the same logic as me but without the little comments to help guide my mind as to what's going on. That might be because you did a convert of my old code; if you had an example purely of your own maybe it would better make the point.

...Actually, now that I think about it, I'd still probably use the same sort of method to read file formats. Big solid state machines mightn't look neat in code, but they're easy to build and debug and work a treat.
M2tM
M2tM
The easiest way to read the re-formed code is to start in the entry point and follow the function calls. In this way you can get a good sense for the flow of the problem without having to tackle 100% of the complexity at once. This also means changes can be more incremental which can help reduce bugs. In general a better highlighting system than the forums provide would highlight methods and variables and such differently to further enhance reading.

Many of the comments are completely useless to your average person looking through to detect meaning even in your larger single function block:

Without fail each of these comments is made completely redundant by your surrounding code. The re-factoring I did of your code was primarily to provide an easier contextual reading (if you read it as I suggested) and to even further remove the need for these comments (which even in your original code are basically completely redundant at a cursory glance.)

They may assist your understanding, but this is only because you have trained yourself to need a redundant crutch. This is not because you have used comments in a useful way here. The most useful comment was the one indicating the - was for the leading minus sign of a number. This said I am not really sure why you search for a negative number when the index cannot be negative and you don't actually store the number as a negative in the first place. This seems to actually be an error. But I do not have the full picture so I cannot say with certainty.


case STATE_ASCII_CODE:
// we have read the ASCII code!
that is exactly what the case just said. why use enums if you're going to comment exactly after? May as well use magic numbers if you'll be commenting this kind of thing.

case STATE_FRAME_REFERENCE:
// we have read the frame_reference!
that's what the case said

// add a terminating zero to make this thing work
if (digit < NUMBER_BUFFER_SIZE)
buffer[digit] = '\0';
it is commonly known that c style strings require a \0 termination so this is pretty much redundant.

if (digit < NUMBER_BUFFER_SIZE)
buffer[digit] = c; // read in the character (if we are within the buffer size)
Well, yeah. Note, however there is no error handling or any mention as to what happens if the buffer fails? We just truck along. So we actually miss out on a valid use for the comment as an indication of incomplete implementation and substitute with a forehead slapper.

// got all the information, so store it in the array
if ((frame_ref >= 0) && (frame_ref < my_character_set -> NumberFrames()))
my_character_ref[ascii_code] = frame_ref;
Yes, of course. You state "got the information, store it in an array" and then you write that in code directly afterwards. This is however an over-simplification as we perform bounds checking before storing it and the comment doesn't indicate that. However we don't want to be too redundant, just a little redundant? Write it in code clearly as you have already or comment to alleviate confusion, but don't comment what is already clear.

// check if we have a digit (or a leading minus sign)
if (isdigit(c) || ((c == '-') && (number_state == FALSE)))
isdigit tells me we're looking to see if it is a digit. The - indicates we are looking for a leading - sign if the current number_state is false. This is kind of redundant, but the - comment is a little useful if we didn't know that we were parsing for number values that could be negative. Of course, the context of this entire function implies that we are dealing with numbers read character by character so it is easily inferred.

// now get the number!
number = atoi(buffer);
Yes, of course.

// now process the number depending where we are in the file reading
switch (file_state)
This is kind of useful because of all the code surrounding the idea of processing the number. I have removed this confusion by putting this entire idea into its own set of well named functions. But it doesn't really tell us much other than a vague "process the number" so here I'd say if you are going to commit, suggest what you are processing the number for and how it is a state machine that swaps between setting the ascii code and the frame reference (which I am still not exactly sure what is in this context, though I understand we're reading in ascii values and storing references to them. It seems like each character counts as a "frame" or something).

Reading these over I can honestly say without the comments I would have been able to read it just as quickly, though near the bottom with the switch it was a tad hairy because it gets into some specific code I had to guess at. In fact, the extra visual clutter of the comments served as a bit of a distraction because of the odd tabbing and haphazard placement.

In any case this is old code and I've written worse, but it is still a good example of why comments are typically over-used, unnecessary and detrimental to the proper writing of applications typically. It promotes a "good enough" type mentality which honestly may be ok in smaller applications but which gathers quickly into an impossible to untangle knot of terrible.

Now, I apologize if it seems I'm nitpicking all over your code, but when you post an example to a topic to support a point make sure it is relevant! :)

Finally I'll leave off with this:
I too write larger functions, not typically quite as big as that, and then periodically refactor them as I write. Programming, like writing a novel requires several drafts and revisions. Typically it is enough to simply write a sentence, look over it, finish the paragraph and then edit. After that you look over the whole document and revise. I tread programming in a similar manner. I'll write a bigger function, revise, write a class, revise, finish an entire module and revise. That's usually as far as I go and it doesn't add much time to the whole production, it indeed will save you time over the long haul. You'll fight less with your code and re-use more. You will have easier to test functions and more flexible work. Less comments required because less confusion is possible when you don't have sprawling masses.

[Edited by - M2tM on March 5, 2009 4:53:55 AM]
_____"You're using a screwdriver to nail some glue to a ming vase. " -ToohrVyk
Trapper Zoid
Trapper Zoid
I looked for that code partly because I knew it would be ugly. [wink] It's part of the code to read in a custom made data file. I typically don't spend much time those parts, and it doesn't matter if it's that ugly. Given it was from 2001 I knew they'd be some basic level of naivety as well. [grin] (Oh, that isn't the whole function. It's quite a lot larger than that. My file reading functions are loooooooong and heavy on the switches).

As for your comments on my comments (heh): I'm not sure how you go about writing code, but I start by writing the comments. When I write a comment that says something like "now get the number", I'd have written that comment first and then started thinking about how I was actually going to get that number.

The comments after the cases are actually the same thing. Something like "we have read the ASCII code!" after "case STATE_ASCII_CODE:" actually is short for "we now have an ASCII code to deal with, how shall we deal with that?". It's a way for my programmer mind to start breaking down the problem into smaller chunks.

Should I stop making comments, even if they help me solve my programming problems? Obviously not. The question is: should I go back through my code and remove them? I'd argue not. In the worst cases, the comment is merely redundant. But I find that having them there makes the code easier to understand, as the same information is presented in two slightly different ways. If it speeds up my comprehension of the code, they're a good thing.

The only downside is if I forget to change one if I change the code, which is a thing you always need to look out for. But generally speaking if the comment is short and right next to the code it's pretty easy to spot a change you need to make.

The thing is, I don't know how much of this is just my own coding style, or whether programmers in general prefer to make little comments like that. Or instead, whether they prefer virtually commentless code, like you're suggesting. I know from my experience with library code that I tend to vastly prefer the code with frequent comments, as it lets me grasp the intent and flow of the code much quicker. Without comments I have to trace through the operation of a function line by line, and there's always bits that are "obvious" to the coder but aren't to me.
Dmytry
Dmytry
my current favorite... strangely relevant to the thread (I see we're discussing virtual functions, too?)
Quote:

// RY: Adding method to copy scroll wheel accel table.
// Unfortunately, we don't have any padding, so this
// is going to be non-virtual.
/*virtual*/

followed by couple more /*virtual*/ methods.
taken from here

It is surely clear, and it quite well explains why /*virtual*/ methods are used, though.
But I'm still going to call that kind of thing /*virtual*/ methods and /*OOP*/ interface, if I ever have to refer to such problem in future (which comes from use of C++ where it is not suitable). Operating system written in plain C can very well be OOP, while C++ system, unless you design it very carefully, could turn into /*OOP*/ as you get stuck with binary compatibility.
Dmytry
Dmytry
Quote:
Original post by Trapper Zoid
The thing is, I don't know how much of this is just my own coding style, or whether programmers in general prefer to make little comments like that. Or instead, whether they prefer virtually commentless code, like you're suggesting. I know from my experience with library code that I tend to vastly prefer the code with frequent comments, as it lets me grasp the intent and flow of the code much quicker. Without comments I have to trace through the operation of a function line by line, and there's always bits that are "obvious" to the coder but aren't to me.

I sometimes write big comment explaining what function should be doing (semi-detailed, less detailed than code but more detailed than function name), then write implementation.
It is just helpful to have plain English description of what you should do and clear understanding of intent, before your eyes, as you code implementation. Helps for reliability. Commenting existing code is not so useful imo.

Other times, in math related code, I sometimes have comment with derivation of formula, often larger than resulting code. It really helps later.
Zipster
Zipster
You should never rely solely on "self-documenting" code where there are more than a few programmers involved, unless you're performing extremely trivial operations. I guarantee you that there will be many cases where what one programmer finds "obvious" or clear will not be obvious or clear to other programmers. You might not even understand your own code anymore if there's a long enough period of time between when you first wrote it and when you see it again. While these other programmers (or yourself) may be able to eventually figure out the self-documenting code, if it takes them longer to figure out the code when reading it line-by-line than it does for them to figure it out by reading a comment, then you've defeated the purpose of writing self-documenting code in the first place.

My basic rule of thumb is that if I look at a piece of code and I'm not 100% sure it would be understood immediately by anyone who looks at it, I write a comment. It's not because the code can't/doesn't document itself, but rather because it's faster to read a comment than it is to interpret the code and I like to provide that courtesy to my fellow programmers (and in return expect the same). Plus, sometimes the general idea is all someone needs to do their job and there's no point in making them read the code line-by-line trying to figure it out if they don't need that comprehensive an understanding.

At least to me, the idea is to write code that lends itself to be self-documenting from the start. That's smart. But it doesn't mean you should sacrifice comments just because your code is self-documenting, because there's always going to be a programmer working on the code who won't see it that way and you'll only make his job harder.
Dmytry
Dmytry
a comment shouldn't be a translation of what the line of code is doing 1 to 1 in plain English, though, as this does not improve clarity a tiniest bit IMO. Comments should really be at higher level than code, else there is no point.

For example, IMHO parser could greatly benefit from precise grammar description as comment, plus comments telling which part of code is implementing which part of grammar. This will help you extend the parser in future, should need arise.

Whereas this...
"
// check if we have a digit (or a leading minus sign)
if (isdigit(c) || ((c == '-') && (number_state == FALSE)))
" is the least useful of comments you can have in parser IMO. (i'm not saying its useless, it is probably still better than nothing)

Note how comment is explaining only the immediately obvious part ( isdigit(c) and c == '-' ) but tells us absolutely nothing about (number_state == FALSE) which is the only single part of that line which might not be directly obvious without looking up what is number_state and what is its role (or guessing that its set to true when we're parsing number, and trying to see what happens with 123-456 and 123E-4) , and why is state variable used instead of, upon detecting from start that it is a number, parsing rest of it in separate loop in separate function ParseNumber or something (i'm as guilty of not doing it properly as everyone else though).

edit: also btw, consider practical case of having to read code in future... I suppose you dont support floating point numbers? Think how would you add them to your parser, and what comments would be helpful.

[Edited by - Dmytry on March 5, 2009 10:56:59 AM]
M2tM
M2tM
Quote:
Original post by Zipster
You should never rely solely on "self-documenting" code where there are more than a few programmers involved, unless you're performing extremely trivial operations. I guarantee you that there will be many cases where what one programmer finds "obvious" or clear will not be obvious or clear to other programmers. You might not even understand your own code anymore if there's a long enough period of time between when you first wrote it and when you see it again. While these other programmers (or yourself) may be able to eventually figure out the self-documenting code, if it takes them longer to figure out the code when reading it line-by-line than it does for them to figure it out by reading a comment, then you've defeated the purpose of writing self-documenting code in the first place.

My basic rule of thumb is that if I look at a piece of code and I'm not 100% sure it would be understood immediately by anyone who looks at it, I write a comment. It's not because the code can't/doesn't document itself, but rather because it's faster to read a comment than it is to interpret the code and I like to provide that courtesy to my fellow programmers (and in return expect the same). Plus, sometimes the general idea is all someone needs to do their job and there's no point in making them read the code line-by-line trying to figure it out if they don't need that comprehensive an understanding.

At least to me, the idea is to write code that lends itself to be self-documenting from the start. That's smart. But it doesn't mean you should sacrifice comments just because your code is self-documenting, because there's always going to be a programmer working on the code who won't see it that way and you'll only make his job harder.


The problem is that comments are prone to a few very significant disadvantages which should not be taken lightly, or ignored.

1) Comments have no guarantee that they are accurate. To rely on them for understanding is foolhardy at best. Often they leave important parts out which means that you may know that it does x, but not y.
2) Comments quickly fall out of date and lose meaning, even the "what" comments do this. Unfortunately even if they were once correct the fact that a comment does not parse means it becomes a second-class-citizen and programmers are lazy (tm) which means they rarely get updated as much as the code they describe. This is kind of more explaining number 1.
3) Comments are often used as a poor excuse to a well-defined interface with well-defined functions and an easy to read flow. It is much easier to write something that isn't self-evident and put a comment above it than it is to write something that is very clear.
4) If you need higher level understanding of a system reading an external document which is easier to maintain is actually easier than manually parsing several header files to gain that understanding. Because all the general stuff can be seen in one place it is much easier to keep relevant and also it will be a good introduction to a subsystem. The disadvantage comments have in this situation is that they are all over the place and so they don't actually act as very good documentation for the why of things when you are trying to understand.

All of this said, I will without fail comment something under these conditions:

Some of the uses I acknowledge as not ideal:
1) The implementation is not complete. This is a crutch which doesn't exist in the ideal case.
2) There is a common confusion which I do not have time to make clear in code. This is a crutch which doesn't exist in the ideal case.
3) There is an unclear dependancy. Note, ideally we would not have unclear dependancy in our system, the interface of our classes should indicate we require something. As such this is again a crutch which doesn't exist in the ideal case.

Some of the valid uses:
4) I am using a particular algorithm or design pattern to achieve something. This is actually a good use of a comment where you may not want to advertise to the system in code what your class uses.
5) Visual clues which help separate blocks of code, so the occasional comment flag for example. These can be useful when skimming code without using the outline view.
6) Writing comments to explain a section when writing code examples for beginners. Note: tutorials and code examples targeted towards beginners need a more verbose context than you can easily provide without comments because typically they do not understand the language well which means they won't be able to gather meaning from the syntax as easily.
7) Where it would be messier to try to expose functionality than it would be to write a comment describing a section of an interface or an action. This includes warnings of consequences which are necessarily obscure at a glance.

In all other cases it is not that I avoid writing comments so much as I avoid writing bad code and then also avoid writing comments that are self-evident. In doing this it is actually easier to read, and yes I still write the occasional comment, but the point is that they aren't something that you should be striving towards.

Another issue I had with the original blog poster's view that he has since followed up with is the idea that managers without programming experience will be reviewing code and you will be fired for writing code they cannot understand. He also suggests quite strongly that code needs to be read by laypersons which is equally ridiculous.

The main issue I'm really getting at here is that documentation is different from code level comments and many people treat them the same.

My view on comments is very closely covered here: Comements
_____"You're using a screwdriver to nail some glue to a ming vase. " -ToohrVyk
Zipster
Zipster
Quote:
Original post by M2tM
1) Comments have no guarantee that they are accurate. To rely on them for understanding is foolhardy at best. Often they leave important parts out which means that you may know that it does x, but not y.

Unfortunately, you can't rely on code to be accurate either. If I have a function called Do_X but someone changes it to do actually be doing Y without changing the name and re-factoring the references, then you're in the same boat as bad comments; reading code line-by-line and trying to figure it out on your own why it doesn't do what you expect. But bad code and bad comments are caused by bad programmers -- this includes not updating comments for code you change.

Quote:
2) Comments quickly fall out of date and lose meaning, even the "what" comments do this. Unfortunately even if they were once correct the fact that a comment does not parse means it becomes a second-class-citizen and programmers are lazy (tm) which means they rarely get updated as much as the code they describe. This is kind of more explaining number 1.

This is true. But code often suffers the same fate. Functions sometimes grow larger than they should be and aren't properly re-factored, so now they do more or less than they originally did, and no one changes the name or does anything to indicate that this function no longer does what it used to and has unintended side-effects, etc. Again, this is due to lazy programming and documenting. Out-of-date comments can also be problematic, but as programmers we naturally tend to trust code more than comments so I personally worry more about code not doing what it says or appears to do than comments (which I regard as fickle anyway). I also find myself fixing up comments all the time to be accurate.

Quote:
3) Comments are often used as a poor excuse to a well-defined interface with well-defined functions and an easy to read flow. It is much easier to write something that isn't self-evident and put a comment above it than it is to write something that is very clear.

I'm going to call shenanigans on this. I don't think there are any decent programmers out there who design bad code because they think to themselves, "ya know, I really don't have to think this through because I can explain it all away in a comment", as though their commenting practices even come to mind when designing code. Rather, they're just bad programmers, and once they're done writing bad code they think, "wow this is really confusing, I should probably add some comments" and not "wow this is really confusing, I should try to re-factor this a bit so it's clearer". I almost never think about comments until after code is written, and certainly never during code design.

Quote:
4) If you need higher level understanding of a system reading an external document which is easier to maintain is actually easier than manually parsing several header files to gain that understanding. Because all the general stuff can be seen in one place it is much easier to keep relevant and also it will be a good introduction to a subsystem. The disadvantage comments have in this situation is that they are all over the place and so they don't actually act as very good documentation for the why of things when you are trying to understand.

I agree. But there's definitely a difference between code-level documentation and system- or API-level documentation, and the key is knowing the difference and when to use what. If someone tries to use comments to explain their systems at a high-level, then again it's the individual who is at fault.

Quote:
In all other cases it is not that I avoid writing comments so much as I avoid writing bad code and then also avoid writing comments that are self-evident. In doing this it is actually easier to read, and yes I still write the occasional comment, but the point is that they aren't something that you should be striving towards.

Self-documenting code is a means toward writing better code, but not an end that replaces comments. Keep in mind that what you call "self-evident" is completely subjective. Too many comments isn't a good thing, but erring on the side of not writing comments when in doubt is also bad. Comments are just a tool, and they can be abused just like any other tool, but it isn't the existence of comments that causes abusive behavior.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.