Software with good documentation is far more valuable than software without. Good software consists of good code and good documentation; code is not a substitute for documentation nor vice versa.
| software = code + documentation (1) |
Countless times people have come to my office saying, “I need to do blah-de-blah; is there any chance you have a program to do that?” Sometimes I am able to help out. I may not have the exactly what was asked for, but I’ve got something similar that can be modified. And I can find it.
This is called reusability. When you are able to re-use software, productivity goes up by a huge factor.
Reusability is related to modifiability and extensibility. These ideas are particularly relevant to “open” software. Note the contrast:
| Some software is open, but only in some narrow legalistic sense. It may be legal to modify the software, but it is impractical, because the software is so badly written and so badly documented. | Some code is open in spirit, i.e. open in the broadest, grandest sense. That means that it is not only legal but practical for a wide range of people to maintain it, extend it, and/or modify it to serve new purposes. |
Software that needs to be reliable will be subject to a code review. Good documentation makes the review go more smoothly, and makes it more likely that the review will accomplish its intended purpose.
Bottom line: Software with good documentation is more reliable, more reviewable, more maintainable, more reusable, and more extensible than software without.
Documentation is the overarching idea. Various forms of documentation exist, including
On numerous occasions I’ve seen code that looked simple, but wasn’t. The code was only a few lines long, but a dozen pages of algebra were required in order to demonstrate that those were the correct few lines. In such a situation, internal documentation is a hopeless task, because within comments you cannot typeset diagrams or complex equations. The solution is to write some external documentation and bundle it with the code.
Let’s discuss the function definition in figure 1. This is a real-world example. It is not contrived; this is some code I found “in the wild”.
getBoolValue : func {
val = me.getValue();
if(me.getType() == "STRING" and val == "false") { 0 }
else { val != 0 }
}
This is pretty bad code. As it stands, the code is unreviewable and well-nigh undebuggable, because there is no way of telling what it is intended to do.
The code was found with no comments whatsoever. Comments by themselves will never transform bad code into good code. However, comments can be seen as the first step along a road that leads to better code, because some comments as to intent would give us a chance of judging whether the code was doing what it was intended to do.
Let’s try to reverse-engineer this code. We can see that it always returns either the integer 1 or the integer 0. These can be considered the canonical boolean values, i.e. the canonical representations of the concept of truth and the concept of falsity. This is arguably useful, because the language’s built-in true/false checker accepts a goodly number of non-canonical representations of truth and falsity. For example, the strings "0" and "0000" both represent falsity.
So to a first approximation, this routine takes a node whose value is any representation of truth or falsity, and returns the canonical representation. To this level of approximation, if(node.getValue()) will behave the same as if(node.getBoolValue()). There is an advantage to using getBoolValue(), because it always returns an integer, which can safely be used in arithmetical expressions, whereas getValue() might not. Similarly, getBoolValue() might look better in a printout.
But wait, we haven’t finished analyzing the code. Apparently the author decided that it would be nice to allow the string "false" as yet another representation of falsity. This means it is no longer true that if(node.getValue()) will behave the same as if(node.getBoolValue()).
| So far, so good, I guess. Using a five-letter string to represent falsity seems a little bit weird, but if that is what the author wants to do, he’s allowed to do it. Maybe that functionality is needed as part of a user interface somewhere. | I doubt that many users are going to type "false" when they could just type 0. Also, accepting "false" but not accepting "False" or "FALSE" doesn’t seem like the greatest user-interface design. |
Alas, we still have not fully analyzed the code. I suspect that the guy who wrote it never fully analyzed it, either. There remain cases that have not been considered. In particular, an uninitialzed node (i.e. a node with the value nil) will be treated as true by getBoolValue(), even though it is treated as false by the language. So this is yet another way in which if(node.getValue()) will behave differently from if(node.getBoolValue()).
Maybe this was intended. It is easy to find cases where “silence implies consent”, i.e. cases where an uninitialized variable should be treated as true. (For example, when adding a switch in series, where previously there was a solid connection instead of a switch, the default value of the switch should be on, not off.)
It strikes me as an unpleasant design to combine these three ideas into one function: Canonicalizing the representation of truth and falsity, adding a new representation of falsity, and changing the behavior of unintialized variables. If I were doing it, I would put these ideas in separate functions. But if the author really wants to smoosh them together, he’s allowed to do so.
The point remains: Some comments would make it a whole lot easier to figure out what the author intended. This in turn would make it a whole lot easier for ordinary folks to use this routine. They would be able to use it without taking the time to reverse-engineer it.
Once upon a time I was actually using this routine. I considered it a convenient way to treat unintialized nodes as representing truth.
However, the author changed the function. The new version is shown in figure 2.
getBoolValue : func {
val = me.getValue();
if(me.getType() == "STRING" and val == "false") { 0 }
elsif (val == nil) { 0 }
else { val != 0 }
}
The new version didn’t have any comments either.
How did this mess come about? Perhaps the author, to this day, has never thought clearly about what his intention was. In this case, the process of commenting the intention would have helped him clarify his thinking. Or perhaps the author changed his mind about the intention. Or perhaps the intention was rock-solid all along, but he didn’t understand his own code well enough to tell whether it complied with his intentions or not.
I had to change my code; I had to rip out all references to this function, because it no longer did what I wanted. (The fact that the author changed this without asking anybody – and without even telling anybody – just added to the unpleasantness.)
You might be tempted to say that the change was a step toward simplification, in the sense that the code now implemented only two ideas instead of three. It canonicalized the representation, and added the string "false" as a new representation of falsity.
Ah, if only that were true.
This code is so bad that there are still cases, heretofore unanalyzed cases, where if(node.getValue()) does one thing and if(node.getBoolValue()) does another. So I still have no idea what is the intent of this code, no way to know whether it is functioning correctly, and no way to know whether/how it can be used safely. (I don’t really care anymore. This is of theoretical interest only, because I’ve stopped using this code.)
Here are some suggestions, mainly about comments. These are general rules, and exceptions are to be expected.
(Comments are the main form of internal documentation. Other categories of documentation are mentioned in section 1.2.)
ii++; // increment ii
jj++; // fenceposts outnumber lintels
The weakness in figure 3 is that even the dimmest programmer can figure out that ii++ increments ii, and therefore the comment doesn’t tell us anything we didn’t already know.
The importance of documenting intent was discussed in section 2. An exceptional case, where commenting the mechanics actually helps, is discussed in section 5.2.
// environment-mgr.cxx -- manager for natural environment information.
The comment in figure 5 was found in the file environment_mgr.cxx. The apparent goal of this comment was to explain the overall purpose of the file. That’s a commendable goal. However the comment doesn’t tell us much beyond what we could have inferred from the filename.
If you need further evidence that the comment in figure 5 is uninformative, consider the fact that the same comment was found verbatim in another file, namely environment_ctrl.cxx:
// environment_ctrl.cxx -- manager for natural environment information.
The two files aren’t doing exact same thing. In fact, there is little commonality between them. Therefore they shouldn’t have the exact same comment.
Note that as emphasized in item 14, the presence of these comments does not cause a problem. Instead, the problem here is the lack of any more-informative comments. For starters, there is a particularly conspicuous lack of any comments explaining the relationship between the two files.
As an obvious corollary, if you are sure that nobody is ever going to re-use the code or even look at it, you shouldn’t bother commenting it at all. It would be a waste of time.
The converse corollary is that if you think the code is going to be re-used even once, the comments are a break-even proposition. If the number of re-uses exceeds one, then the comments pay off very handsomely.
True story: One of the first nontrivial programs I ever wrote was a communication program, sort of a primitive version of Kermit or Telnet. It was written in assembly language. I started writing it after dinner one day, and finished it before midnight. I never dreamed that anyone but me would ever use it ... but five years later it was still in use by significant numbers of people, including my friends and employees. The wanted to add some features to the program, but they couldn’t. They discovered that the source file contained exactly zero comments, and teased me about this. The program was the perfect unhappy medium: It was good enough that nobody wanted to stop using it, yet it was limited enough that they could rightfully complain about it, and the code was so inscrutable that extending it would have been harder than writing a replacement from scratch.
| If you don’t comment your code, there is little chance that it will be re-used, so in accordance with item 4 you can say to yourself, “See, it wasn’t reused, so it would have been a waste of time to comment it”. | If you do comment the code, there’s a good chance that it will be re-used, possibly many times, so in accordance with item 4 you can say to yourself, “See, it was re-used, so it’s a good thing I commented it”. |
In general, given my choice of self-fulfilling prophecies, I choose the prophecy that has the happiest outcome. In particular, I comment my code.
The independence mentioned in the previous paragraph explains why the comments should give a second viewpoint on what the code is doing, rather than mindlessly parroting what the code says, as mentioned in item 1.
Well-written works of natural language contain a great deal of redundancy. We should expect a goodly amount of redundancy in well-written works of computer language.
The need for documenting the interface can be seen in the example in section 2.
Usually the interface should be documented in the header file, not just in the implementation file. To say the same thing in the language of C or C++, the interface should be documented in the .h file, not just in the .c file. If you want to cut-and-paste the same comments into the .c file, that’s fine, but the .h file should be considered the “master” copy. If you’re in a hurry, you can just put a remark in the .c file referring readers to the .h file.
The interface documentation should be in the header file, because that is the publicly exported definition of the interface. If you do things right, incomparably more people will be reading the .h file than reading the .c file. Of course at some point you need to switch to external documentation, as discussed in section 1.2.
These tactical style issues are not trivial, but they are, relatively speaking, less important than the strategic issues. The important thing is that comments convey meaning. If your style is so bad that it interferes with intelligibility, you need to change your style. Otherwise, I’m not going to worry about it; not right now, anyway.
If you are modifying a program, you should probably adhere to the existing style, if at all possible, because a mixture of inconsistent styles can make things hard to read.
To express the same idea another way: Good comments are not an excuse for bad code, and good code is not an excuse for bad comments (or no comments).
This situation comes up fairly regularly. If you find yourself writing a long comment to explain the limitations of the code, consider the possibility that it might be easier to write some less-limited code than it is to document the limited code ... not to mention the fact that fixing the code results in better functionality.
| Of course, if a comment is maliciously deceptive, then it can be considered harmful and worse than useless. But this is rare. | With rare exceptions, a comment is never truly harmful. Even if the comment is not 100% correct, it provides a clue as to the programmer’s state of mind at the time. |
To repeat, it is common to find comments that have so little value that it was not worth the trouble of writing them ... but once a comment has been written, the value is rarely less than zero. Returning to the classic bad example of ii++; /* increment ii */, the comment is worthless but it isn’t actually harming anyone. If you see such a thing, either ignore it, delete it, or (preferably) replace it with something better.
If the code contains worthless comments, the worthless comments are not the problem. They may be symptomatic of a lack of more-valuable comments, but they are not, by themselves, a problem.
Don’t pick a fight over worthless comments. Doing so would just make you look childish. Remember, coolness of a person can be judged by the size of the problems that annoy them.
Be sure that the comments contain a reference to the external documentation, so it can be found when needed.
Commit log messages are not a substitute for comments. Information that needs to be in the file should be in the file, not hidden in the commit log. (This is obvious with modern SCMs such as git, which commit multiple files at the same time, but it remains true and important even if you are committing just one file.)
It’s a question of incremental versus cumulative, i.e. a question of differential versus integral. A glance at the comments in the files should tell you what you need to know about the current version of the project. The commit log messages pertain to the difference between the current version and the previous version.
If you see a comment that has outlived its usefulness, delete it.
“The code should document itself. Too many comments are an admission that the code isn’t understandable by itself.”
This idea is wrong in so many ways that I hardly know where to start.
For one thing, “admissions” are not directly related to truth.
| In Galileo’s day, there were a lot of people who did not admit that there were moons around Jupiter. | In truth, there were moons around Jupiter, whether anyone admitted it or not. |
Applying this idea to the subject at hand, we see that:
| If the code is bad, it’s bad whether you admit it or not; removing comments is not going to make it better. | If the code is good, adding comments is not going to make it worse. |
Furthermore, having a problem and not admitting it is far worse, from a teamwork point of view, than admitting and documenting the problem. Remember what Lyndon Johnson said: “While you’re saving your face, you’re losing your ass.”
Note that the argument cited at the top of this item is a rather direct denial of the view expressed in item 11. Also it shows an amazing degree of non-understanding of the importance of redundancy, as discussed in item 7.
Also: It is always important to distinguish what should be from what is. Some of the people who loudly assert that their code “should be” self-documenting are not reliably capable of writing code that actually is self-documenting.
There are various different motives that (separately or in combination) lead to the same behavior. These include:
Needless to say, no matter what the motive(s) may be, this sort of behavior counts as really bad teamwork. Rather than creating job security, it will get the perpetrator kicked off any well-managed software project. Almost every software manager on earth is aware of these ploys. The rule is, if somebody writes code that other people cannot understand, it reflects badly on the author, not on the others.
In the software business, there exists the idea of conditional compilation. That means it is possible to write some code that is visible in the file but will not be included in “this” version of the executable program.
Some languages have fancy features expressly for the purpose of conditional compilation such as the #if ⋯ #endif features in the C and C++ languages.
For languages that don’t have fancy conditional compilation, and even for those that do, the same effect can be achieved by putting a snippet of code into a comment; this is called “commenting-out” the code.In such a case, the commented-out snippet takes the form of a comment, but its primary purpose is not to comment the code; its primary purpose is conditional compilation. (Sometimes, but not always, the commented-out snippet also serves, secondarily, as a genuine comment, allowing the reader to see an alternative, disfavored implementation.)
One of the most common uses for conditional code pertains to debugging aids, also known as scaffolding, also known as test-harness code. In the early phases of development, the scaffolding is very useful, but when the code is fully tested the scaffolding is no longer needed. However, it is better to conditionalize the scaffolding rather than to delete it, because it will be needed again as soon as somebody tries to modify or extend the code.
Let’s be clear: Leaving the scaffolding in place, subject to conditional compilation, makes the code more open, more maintainable, more reusable, more extensible, et cetera.
| Usually, my preference is to use the “official” conditional compilation features of the language ... especially when conditionalizing a long snippet, or a snippet that depends on other snippets that should be subject to the same condition (for instance if a debugging variable is declared in one place, set in another place, and printed out in another place). | On the other hand, if the snippet is self-contained and only two or three lines long, I see no harm in just commenting it out. |
As mentioned in item 9, the names of symbols (such as variables and functions) sometimes help to explain the meaning of the symbol. An example of this is discussed in section 5.2.
On the other hand, names alone are rarely sufficient explanation (except in the almost-simplest cases).
We can draw useful analogies to natural language: A titmouse is not a mouse, chocolate turtles are not made from turtles, and milk of Magnesia is not made from milk. As Voltaire famously remarked, the Holy Roman Empire was neither holy, nor Roman, nor an empire. The point here is that you should not expect the name of a thing to tell you what you need to know about the nature of the thing. This is true in computer languages as well as natural languages.
In natural language documents, we use dictionaries and glossaries to explain the meaning of each word. By the same token, the comments should constitute a form of glossary, explaning the meaning of each symbol.
Sometimes an entire sentence – or even multiple paragraphs – will be needed to explain the meaning of a function including its uses and restrictions. Comments are the appropriate place for such an explanation. It would be absurd to encode all of that into a sentence-long or paragraph-long function name.
In particular, it seems silly to require the name of a variable to encode the type of the variable. If types are important to you, use a type-safe language. If you are using a type-safe language, it is silly to ask the programmer to keep track of something that the compiler can keep track of more easily and more thoroughly.
Similarly, it is usually not advantageous for the name of a variable to encode the units of measurement. In most cases a good technique is to pick a consistent set of units (such as SI) and use it for all internal calculations, converting (if necessary) from/to other units only when doing I/O. Quantities with non-standard units may require special comments, maybe even a special name. Or, better yet, use a software package that allows the computer to keep track of the units. This is better because it can enforce the rules, whereas names and comments do not enforce anything; they may help the programmer live within the rules, but they do not actually prevent mistakes.
| Symbols that are very local in scope can have short names. For example, in a small local context it would be reasonable to use “T0” to denote a starting temperature. In typical cases there is no advantage in using a longer name such as “starting_temperature” or “starting_temperature_in_degrees_kelvin”. | Symbols that are widely exported need to have longer and more systematic names, to avoid conflicts. |
Namespaces are an important tool, allowing symbols to be called by short names when appropriate and longer names when appropriate. Example: Atmosphere::ISA::T0 is smarter than Atmosphere_ISA_T0, because the former can be shortened when appropriate, when working within the Atmosphere::ISA namespace
Just to reiterate: The rule is, you should make the code as self-documenting as possible, and then comment it. Since the code is never fully self-documenting, the comments fill in some of the gaps, and even if the code were perfectly self-documenting, redundancy is useful.
One way to make the code more self-documenting is to avoid what we call “magic numbers”; that is, unnamed numerical constants. See reference 2 for a discussion of magic numbers and why they should be avoided.
Let’s look at a real-world example. I emphasize that this is not at all a contrived example; this is some code I found “in the wild” in a low-level library routine, written by an experienced programmer who was trying to do the right thing (but didn’t quite succeed).
// vasi is always on
lightSwitch->setValue(0, true);
if (sun_angle > 85 || updateVisitor->getVisibility() < 5000) {
// runway and taxi
lightSwitch->setValue(1, true);
lightSwitch->setValue(2, true);
} else {
// runway and taxi
lightSwitch->setValue(1, false);
lightSwitch->setValue(2, false);
}
// ground lights
if ( sun_angle > 95 )
lightSwitch->setValue(5, true);
else
lightSwitch->setValue(5, false);
if ( sun_angle > 92 )
lightSwitch->setValue(4, true);
else
lightSwitch->setValue(4, false);
if ( sun_angle > 89 )
lightSwitch->setValue(3, true);
else
lightSwitch->setValue(3, false);
In figure 7, the numbers 0,1,2,3,4, and 5 are perfect examples of magic numbers. They are the sort of magic numbers that should be avoided, for all the reasons cited in reference 2.
When somebody tries to read this section of code, all sorts of hard-to-answer questions arise. Why are runway and taxiway lights apparently not included in the category of ground lights? How did the number 0 (as opposed to 5 or 137) come to be associated with VASI lights? Are runways 1 and taxiways 2, or vice versa? What about approach lights? Are they missing entirely, or are they included in one of the six categories mentioned here? Why are we checking the visibility? What is the policy regarding visibility? Why are we implementing high-level policy in a low-level library routine anyway? Is this a check against flight visibility, or against ground visibility, or something else entirely?
A subset of these questions can be answered with the help of the comments. Based on the comments, it appears that the numbers 1 and 2 are associated with runways and taxiways. This comment is crucial, because it allows us to search the file. Trying grep -i runway.*light doesn’t help. Trying grep -i taxiway.*light doesn’t help, either. But grep -i taxi.*light hits paydirt; it finds the place where successive children of the lightSwitch object are associated with various types of lights.
This is a good-news bad-news story:
| The comments are few and shallow. | The comments are helpful; without them the code in this example would be considered really, really terrible. |
| The comments merely describe the mechanics of the code. Such comments are usually unhelpful, as discussed in item 1 | In this case the code is so weak that commenting on the mechanics actually adds information. |
| We still lack any comments on the higher-level strategy and intent. Commenting on the mechanics cannot take the place of commenting on strategy and intent. |
This code, as it stands, would irritate the reviewer during a code review. Code is easier to review if each passage, by itself, can be seen to be manifestly correct. In this case, the reviewer would have to do a lot of work to make sure that the magic numbers were being used correctly.
By the same token, this code would tricky to modify. A programmer who disturbed the order in which the children were added to the lightSwitch object would cause bugs in places far from the site of the disturbance.
Many of these questions and problems would go away if the unnamed numerical constants were replaced with named constants, such as TAXI_CHILD(0), RWY_CHILD(1), TAXI_CHILD(2), et cetera. Using an enum{} would make this even simpler and better. Then in every (!) place where the code indexes into the lightSwitch object, these names should be used. This includes (!) the place where the children are initially added to the object. This would instantly eradicate a wide class of bugs, and would make other bugs much easier to debug. Note that grepping for every occurrence of RWY_CHILD is likely to be much more rewarding than grepping for every occurrence of 1.