Commenting source code

Commenting source code is a hot topic in several communities.

The most prominent advice is that comments should say "why", not what.

While I generally agree with such advice, I believe that it is not enough for deciding if a comment is worth or good, also because, it is not always clear what is "why", and what is "what".

Consider a piece of code like

++i;

Novice programmers might add a comment that this line increments i by one, and we would all agree that it is telling what the code is doing, not why. So it should follow that a comment like "repeat five times" should be better. It tells why we are doing ++i, and we cannot recognize this information from this line of code alone.

If we look at the context, we might see something like

while(i<5) {
	f();
	++i;
}

And in this context, the comment would be completely superfluous, as it is obvious that we are repeating something five times.

Now the question would be "Why are we repeating something 5 times?".

So looking at the scope of a single operation, the "why" mixes with the "what" of a greater context.

If we apply this reasoning recursively, we should end with just one comment before the main function.

We do not want to have the whole program context in mind, it is too error prone after a certain code size. Comments can and should alleviate this task, they are just like other tools, like functions, classes, and variables, that should help to manage complexity. Unlike those tools, they are not evaluated by static and dynamic analyzers (with some exceptions). This is, in most situations, a very big disadvantage, since it opens possibilities for a lot of errors.

Thus in most situations, I treat comments as shortcomings or code smell, but not something that should necessarily be avoided at all costs. Sometimes it is the best tool we have at disposal, for example for generating documentation, or because alternatives would be too much complicated or error prone.

I have looked in different projects for comments and checked in which situation it would have been possible to replace them with something else.

Complexity and performance considerations

This type of comments tries to explain some design decisions. An algorithm might take some parameters by pointer, mutate them in place, or do something that might not seem obvious. Updating the algorithm with, for example, a more readable implementation would not necessarily cause some runtime-errors, but it might change the behaviors, as the new function might be slower.

A better approach would be to have some test that validates those properties. For a generic algorithm that can work with almost every type, create a type that counts moves and copies, and assert an upper limit in the test case. Unfortunately those metrics might not be enough for determining the performance of an algorithm (see Allegro means both fast and happy for some interesting details), an even better approach would be to have a platform where to run regression tests, those should check, for example, that there is no performance loss.

Performance considerations are especially error-prone, determining if cache-locality should be preferred to fewer operations is difficult, so stating it in comments adds little value without something more concrete.

Compiler specific checks

Every major project have some checks like

// MSVC does not define this or that
#ifdef MSVC
// ...
#endif

The problem with those comments is that they are very imprecise. Most of those do not even document with which version the incompatibility has been observed. Newer versions of the Microsoft compiler are much more standard compliant, and many of those conditions do not apply necessarily.

A better alternative would be to define a compile-test, and check if a piece of code compiles with the given compiler.

The advantage of that approach is that if newer versions of the compiler are more standard complaint (or implement the tested extension), then it is not necessary to search for all those comments and verify if they still apply. It also scales more easily, as the test would apply to every compiler version, instead of enumerating by hand all compilers and compiler versions. It is, therefore, less error-prone, as we might be choosing a slower or wrong implementation without noticing it.

Specification comments

Some projects have an external specification, for example from the client, written in a PDF file.

As this specification might describe some complex or questionable operations, it is a good idea to put a mark in the source code. In some cases, it might even be more helpful to copy a snippet of the specification. This practice can help to maintain the code, as it helps to avoid to diverge from the specifications because future programmer now knows where to look at. It also coincides very well with the rule "comments should say why, not what", since the comments are stating, with a quote, why the code looks like that.

Like all other types of comments, those are ignored by all tools (if not: congratulations). The specification might change over time, but who is going to check if all comments do still apply? Another scenario is code being refactored, there might not be anymore a single function in our source code that matches what described in the specification.

A much better approach, if possible, is to have test cases with appropriate descriptions.

Suppose that the source code looked like:

// as described in rfc_xxx, section yyy
foo parse(/* ... */);

A first step, if possible, would be to rename the function (or put it in a namespace or class)

foo parse_rfc_xxx_section_yyy(/* ... */);

This might make the name of the function less readable but avoids the problem that comments might get out-of-sync.

As already mentioned, functionalities might be implemented in multiple functions that are used for multiple purposes, so there is no easy way to name those with a reference to the specification.

A test case is, therefore, a better approach, to ensure the correct behavior:

TEST_CASE("rfc_xxx"){
	SECTION("Parsing as described in section yyy") {
		// ....
	}
	SECTION("Parsing as described in section yyy with invalid data") {
		// ...
	}
	SECTION("Parsing as described in section yyy needs to support this and that") {
		// ...
	}
	// ....
}

If someone changes the code, and a test case fails, this will provide much more useful feedback than a comment alone. We can embed much more information, validate them, provide guaranteed working examples on how to use those functions, and all that in the long-term is easier to maintain than a comment.

This can lead to very comfortable REPL scenarios in case of failures, were trying to change some code immediately prints an appropriate error message, and makes the developer realize his error with little effort.

One possible disadvantage of tests, compared to comments, is that they are not "near" the function, as they normally live at least in a separate file.

Test frameworks like doctest provide the ability to write test cases directly next to the function.

Bug Ticket references

Every time I see a comment like Fixes Ticket nr xxx I need to roll my eyes. On one hand, this type of information is surely important, especially during code review. It tells the "why" a specific line of code has been changed.

But what value does such a comment add after one week or maybe one year? Especially if removing that specific line of code does not trigger the bug described in the ticket system anymore?

Comments get old, but those tend to get old really fast. And the cost of validating them every time is too high, otherwise, we would have something for accomplishing that job.

So how can we provide this type of information, but without the disadvantages of comments?

Commit messages are another place where to add useful information for humans. And they do not get old, as the comments in a commit message refer to a specific point in the lifetime of the software. Comments in the source code do not have that property.

Also, other types of comments that get old should probably go in the commit message, as for example performance measurements.

Comment in commit messages has another nice property that is difficult to accomplish with comments in the source code. They can refer to multiple pieces (granted, only those that were changed) of code or functions without getting out of sync.

For example, suppose there are two functions, foo and bar, and one has a dependency on the other, like a similar implementation. It would be actually a good idea to state somewhere this fact, and why there is a minor difference that did not make it possible to reuse one implementation for all use-cases.

If we write this as a comment on both functions, there is, in the comment, a reference to the other function. While time passes, the other function might get deleted, renamed, of its implementation might get completely overhauled. In all those cases, we have a dangling comment or a misleading comment.

If we describe this information in the commit message when introducing the foo and/or bar function, since commit messages do not get old, we also ensured the correctness of the information.

Silencing compiler warnings

Though pragma or other tricks (also depending on the language), it is possible to silence specific compiler warnings, or warning coming from other static analyzers.

Because of the "why, not what" rule, it does seem the right thing to do why there is a pragma.

Again, a test case is more useful: Turn the warning into error and compile the offending piece of code.

If someone, in the future, removes that directive, there are two possible outcomes. Either the code does not compile, then the pragma should probably still stay there, or it is not useful anymore (for example the compiler got updated).

"Do not edit unless you know what you are doing"

A perfect example of a completely unnecessary comment.

Under what circumstances is it fine to change something without any knowledge? How do I know if my knowledge is enough to ignore the warning, especially if there are no hints about possible issues or why editing this file is discouraged?

If the code has been generated, just write that it has been generated. If there are real or possible issues or some hidden dependencies, then describe them. Better yet, add tests, and add validation (with clear error messages) on those parts on the system that rely on such code. If something else crashes terribly, just write that, or better, write a test to ensure that nothing crashes.

Code living outside a project

The source code does not always live inside a project, repository or bug trackers. Not everyone has necessarily the same toolchain like the same compiler versions, …​ or a compiler at all.

Comments in commits are not easy or possible to change, and commits only show changed code, thus commenting existing code is nontrivial.

Sometimes code gets copy-pasted out of context, a good comment might survive that operation, while most other techniques will fail. Of course, we should avoid it, prefer other forms of code reuse, but it happens so frequently that the issue cannot be ignored.

Sites like Stack Overflow are full of code snippets, functions, or even complete programs that are copied on a daily basis many times. In those case, there is no project structure, VCS, tests or other tools. Comments are the only alternatives.

Comments are also super-useful when learning something new, for a beginner ++i is something new and worthy to annotate.

Conclusion

People should be confident to change the code, especially the one they do not understand in order to analyze it.

Comments should help, but I experienced multiple situations where in the best case they are ignored and in the average case they are wrong or out of date. In the worst case they obtained the opposite effect, instead of helping or being ignored, they put people on the wrong track. Comments might add fear because it means there might be something not detected by the tooling, or something that will break silently or in subtle ways. Thus we end up with sayings like "never touch a running system".

I prefer to look beyond the code itself; we should not focus solely on the code. We should use and learn the tooling around the language (static analyzers, build system, compiler flags,. ..), a VCS, test suites, grep and other search utilities

This is of course not very beginner friendly.

It’s very probable that sooner or later you should learn those tools anyway, especially if working with other people, and it is probably easier to master those tools than to write foolproof and useful comments.

Many comments might help short term, learning to use better/other tools is a long term investment.