250

A lot of people claim that "comments should explain 'why', but not 'how'". Others say that "code should be self-documenting" and comments should be scarce. Robert C. Martin claims that (rephrased to my own words) often "comments are apologies for badly written code".

My question is the following:

What's wrong with explaining a complex algorithm or a long and convoluted piece of code with a descriptive comment?

This way, instead of other developers (including yourself) having to read the entire algorithm line by line to figure out what it does, they can just read the friendly descriptive comment you wrote in plain English.

English is 'designed' to be easily understood by humans. Java, Ruby or Perl, however, have been designed to balance human-readability and computer-readability, thus compromising the human-readability of the text. A human can understand a piece of English much faster that he/she can understand a piece of code with the same meaning (as long as the operation isn't trivial).

So after writing a complex piece of code written in a partly human-readable programming language, why not add a descriptive and concise comment explaining the operation of the code in friendly and understandable English?

Some will say "code shouldn't be hard to understand", "make functions small", "use descriptive names", "don't write spaghetti code".

But we all know that's not enough. These are mere guidelines - important and useful ones - but they do not change the fact that some algorithms are complex. And therefore are hard to understand when reading them line by line.

Is it really that bad to explain a complex algorithm with a few lines of comments about it's general operation? What's wrong with explaining complicated code with a comment?

gnat
  • 21,442
  • 29
  • 112
  • 288
Aviv Cohn
  • 21,190
  • 31
  • 118
  • 178
  • 15
    If it's that convoluted, try refactoring it to smaller pieces. – Vaughan Hilts Sep 01 '14 at 00:37
  • 153
    In theory, there is no difference between theory and practice. In practice, there is. – Scott Leadley Sep 01 '14 at 02:13
  • 5
    @mattnz: more directly, at the time you write the comment you are steeped in the problem this code solves. Next time you visit, you will have less capability *with this problem*. – Steve Jessop Sep 01 '14 at 09:13
  • 27
    "What" the function or method do should be obvious from its name. How it does it is obvious from its code. Why is it done this way, what implicit assumptions were used, which papers one need to read in order to understand the algorithm, etc. - should be in comments. – SK-logic Sep 01 '14 at 09:36
  • 11
    I feel many of the responses below are purposefully misinterpreting your question. There's nothing wrong with commenting your code. If you feel you need to write an explanatory comment, then you need to. – Tony Ennis Sep 01 '14 at 13:48
  • 3
    @SK-logic I'd say what the function or method does, what are the arguments and how it handles special cases should be described in the comment that precedes the method. The method name needs only be explicit enough to be able to pick the correct one in autocompletion. – Florian F Sep 01 '14 at 19:57
  • 2
    @TonyEnnis Not really, but I think they could be put in simpler terms. For example, from the question: "comments should explain 'why', but not 'how'" - this is an argument _for_ what Prog is questioning, not against as the question seems to indicate. Explaining a complex algorithm _is_ explaining _why_ you do a bitshift here or a comparison there, rather than something like `foo << 2; // bitshift 2`. – Izkata Sep 01 '14 at 22:42
  • 2
    I honestly like small comment pieces inside the method instead of above it. The name of the method should be descriptive enough to communicate the overall idea and expectations of the method. But if they are interested in the implementation detail, then yes those comments can be really useful to take user step by step through the code. – Tarik Sep 02 '14 at 01:45
  • 3
    @FlorianF, only if you're using something like JavaDoc, or Lisp's doc strings. Otherwise such comments are useless and hard to navigate from an IDE. – SK-logic Sep 02 '14 at 06:39
  • 1
    Depending on your processes. This type of comments might be better put inside the Business Requirement Document or the Technical Specification Document. – the_lotus Sep 02 '14 at 12:53
  • 4
    Not exactly a perfect answer so I'll live it in the comments *bud-dum-tiss*, but what you many understand as clear and concise English may be of very little use to someone else. While things like ` //Test to see if file exists and is valid if (File && FileLocation && FileMimeType && FileMimeTypeValid) ` make sense and may reduce reading time for a debugger/maintainer, it's very likely that in complex blocks of code your understanding of what it's doing will not carry over to someone else, because the language you comment in is more ambiguous than the language you write it in. – Sidney Sep 02 '14 at 13:32
  • 5
    "A human can understand a piece of English much faster that he/she can understand a piece of code with the same meaning (as long as the operation isn't trivial)" I disagree. I find code much easier to understand than English, and the less trivial the concept, the easier code is to understand than English translation. – stephenbayer Sep 02 '14 at 16:28
  • 1
    asked and thoroughly answered in [“Comments are a code smell”](http://programmers.stackexchange.com/questions/1/comments-are-a-code-smell) and in answers to multiple questions linked to it – gnat Sep 03 '14 at 16:24
  • 2
    TLDR: "What's wrong with comments that explain complex code?" Nothing. It's the complex code that's the problem, not the comments. – Ajedi32 Sep 04 '14 at 15:51
  • 1
    I think it is worth noting that you can only truly understand what a function does by looking at and understanding the code. That is where the truth is. A comment can lie. A responsible programmer will read and understand the code anyway, making the comment obsolete. – Nick Sep 04 '14 at 16:55
  • 2
    My main reason for inline comments is not to say anything about the code, most often its to detail requirements or customer decisions, e.g. "//customer wants this list reverse sorted". – daveD Sep 04 '14 at 19:47
  • 3
    I don't like reductive schools like Robert Martin's for this reason. Yes, this is all well and good, except that if you have to satisfy a complex requirement sometimes complex code is required to do it. – Casey Sep 05 '14 at 16:14
  • Comments are not for extended discussion; this conversation has been [moved to chat](http://chat.stackexchange.com/rooms/16962/discussion-on-question-by-prog-whats-wrong-with-comments-that-explain-complex-c). –  Sep 05 '14 at 21:06
  • Good code is often easier to read than comments. (Not nessecary easier than good comments though.) For example a well chosen method name can be way quicker to read and easier to understand because it might contain more domain knowledge, than a comment might. Well written English often needs more words to make nice sentences, which takes at least longer to read, and sometimes it's also harder to read (more words can get in the way of the simple meaning). – Lode Sep 05 '14 at 21:34
  • 4
    Rather than answering your question here I've posted some thoughts to my blog. http://ericlippert.com/2014/09/08/comment-commentary/. In short: **a complex algorithm should have a specification**, and the comments should refer to that specification. – Eric Lippert Sep 08 '14 at 16:45
  • 1
    @EricLippert You should answer the question as well, I read your blog and with the included code sample I think your answer is much better than the currently accepted answer. By answering and perhaps summarizing your post, then linking to it, the chance of people reading it will be much greater. I think comment abuse and unreadable function names are two of the most common issues in bigger codebases today, so it deserves a stage! – Wouter Simons Sep 09 '14 at 00:35
  • Code changes and comments aren't always updated and then they get out of date. It's really that simple. – Michael Durrant Feb 07 '15 at 13:18

16 Answers16

419

In layman's terms:

  • There's nothing wrong with comments per se. What's wrong is writing code that needs those kind of comments, or assuming that it's OK to write convoluted code as long as you explain it friendly in plain English.
  • Comments don't update themselves automatically when you change the code. That's why often times comments are not in sync with code.
  • Comments don't make code easier to test.
  • Apologizing is not bad. What you did that requires apologizing for (writing code that isn't easily understandable) is bad.
  • A programmer that is capable of writing simple code to solve a complex problem is better than one that writes complex code and then writes a long comment explaining what his code does.

Bottom line:

Explaining yourself is good, not needing to do so is better.

Robert Harvey
  • 198,589
  • 55
  • 464
  • 673
Tulains Córdova
  • 39,201
  • 12
  • 97
  • 154
  • 96
    It's frequently impossible to justify spending employer's money rewriting code to be more self-explanatory, when a good comment can do the job in much less time. A dutiful programmer must use her/his judgment each time. – aecolley Sep 01 '14 at 01:27
  • 35
    @aecolley Writing self-explanatory code to begin with is better yet. – Tulains Córdova Sep 01 '14 at 01:30
  • 3
    @aecolley : In that case take a hint from 16th century charts. The words "HC SVNT DRACONES" is all the commenting needed. – mattnz Sep 01 '14 at 02:11
  • 133
    Sometimes self-explanatory code isn't efficient enough to solve a problem with today's HW&SW. And business logic is notoriously ... twisty. The subset of problems that have elegant software solutions is significantly smaller than the set of problems that are economically useful to solve. – Scott Leadley Sep 01 '14 at 02:16
  • 11
    From my own experience, it is easier to make business code self-explanatory than to make algorithm code self-explanatory. Business code can draw from common-sense intuition, whereas algorithms can only draw from papers, theorems, axioms, equivalences, algebraic abstractions, etc., none of them belong to common-sense. – rwong Sep 01 '14 at 04:13
  • 1
    Sometimes algorithm code begins as a big ball of mud, and it is only after a lengthy period of analysis "picking-apart" that reusable parts can be isolated from it, based on identifying recurring patterns which occur across different algorithm implementations. Such "algorithmic patterns" are similar to Design Patterns, but they **are not** design patterns - GoF doesn't teach algorithms. Algorithms are taught in universities. – rwong Sep 01 '14 at 04:16
  • 3
    You nailed it. Especially the "not in sync" part: Who guarantees to the reader that the comples code actually does what the simple comment says? You will have to read two entities: the code and the comment to be sure. Each time. This will probably consume more time than refactoring the code, at least over time. – Marcel Sep 01 '14 at 05:59
  • Writing *that* when you mean *than* is a very common error. I think this is the first time I've seen the opposite. – TRiG Sep 01 '14 at 09:08
  • 65
    @rwong: conversely I often find myself writing more comments in business logic, because it's important to show exactly how the code lines up with the stated requirements: "this is the line that prevents us all going to jail for wire fraud under Section Whatever of the penal code". If it's just an algorithm, well, a programmer can figure the purpose out from scratch if absolutely necessary. For business logic you need a lawyer and the client in the same room at the same time. Possibly my "common sense" is in a different domain from the average app programmer's ;-) – Steve Jessop Sep 01 '14 at 09:17
  • 29
    @user61852 Except that what's self-explanatory to the you that just wrote that code and spent the last $period immersed in it might not be self-explanatory to the you that has to maintain or edit it five years from now, let alone all the possible people that aren't you that may have to look at it. "Self-explanatory" is a nebulous holy grail of definitions. – Shadur Sep 01 '14 at 09:38
  • 1
    So, we can do away completely with comments? – Florian F Sep 01 '14 at 11:44
  • 12
    @ScottLeadley If I had one dollar for each claim that a specific hack is needed due to performance reasons, I would still be a rich man, even if I had to pay 10 dollar for each case in which the claim is actually true. – stefan.s Sep 01 '14 at 13:39
  • 3
    @SteveJessop: that is the good comments. Describing which requirements compels a particular part of code is describing the why of the code. – Lie Ryan Sep 01 '14 at 16:02
  • 10
    Also, more importantly, "self-explanatory" code is only a good idea as long as it actually *works*. If there's a bug in the code, the comment that describes what it's supposed to be doing might prove invaluable to a later debugger who wonders where the hell the program is screwing up... – Shadur Sep 01 '14 at 20:56
  • 6
    @Shadur: If the comment does not match the code, the maintainer has the problem of deciding which is more correct - the comment or the code. In my experience, flipping a coin is as accurate as any other method (i.e. its completely random). – mattnz Sep 01 '14 at 21:48
  • 2
    aecolley. You should not need to justify or discuss either 1) testing or 2) commenting with a non-technical manager. You should say how long it takes to *deliver* the feature. The technical parts of it including tests should not be broken out for non-technical managers. If you choose to do this you are offering it up as optional. – Michael Durrant Sep 02 '14 at 20:20
  • 3
    "Comments don't make code easier to test." They also don't make your code capable of cooking waffles. Why is the fact they don't make code easier to test a particularly good or bad thing? – Pharap Sep 04 '14 at 18:55
  • 2
    @Pharap as opposed to striving for simplicity that does make code easier to test and make long comments unnecessary. The thing is comprehensive comments vs simpler code that doesn't need them. – Tulains Córdova Sep 04 '14 at 19:28
  • @user61852 That's an issue with complex code, not an issue with comments. What you mean is "complex code is harder to test", in this case the comments and the complex code are different matters, the complex code affects the testing, the comments do not. – Pharap Sep 04 '14 at 19:35
  • 3
    You're all assuming the coder has a mastery of English. Let the code speak for itself, after all, it is just formalized language: logic. If you can't do logic in a clear and simple fashion, how could you possibly write anything meaningful with natural language? – FlavorScape Sep 06 '14 at 02:16
  • 3
    @FlavorScape And logic is a weak and small, though well defined subset of natural language. Anything that can be said in logic can be said with natural language as well. Yet with natural language even a fool barely knowing english is able to express more than any programmer can express in code. – David Mulder Sep 06 '14 at 22:36
  • @David Mulder: Sure, if that fool wants to describe the scent of a flower, or the experience of hearing Mozart for the first time. But we're not talking about that sort of natural language, and if you're going to claim that natural language is more capable of expressing _program logic_ than, well, the program logic, then [citation needed]. – FeRD Sep 08 '14 at 13:32
  • 2
    @FeRD: Of course language is. It might sometimes be more ambigious, but it's definitely more capable. It might be longer (likely even), but every piece of program logic can be expressed in natural language *at least* as well as in code. Why? Because code has become a subset of natural language, and all it's new constructs have been introduced in natural language as well for the sake of writing documentation. The only level on which this could be disputed is very low level programming. – David Mulder Sep 08 '14 at 14:24
  • 1
    @DavidMulder and yet, when we hire programmers, we don't interview for writing skills. Code is more explicit and precise, the grammar is locked in standards and that is restrictive. The restriction has purpose, it is intended to cancel out interpretation. Clean code, with descriptive names, small functions, clear objectives and goals, etc. can be very complex and easy to read and understand at the same time. – Wouter Simons Sep 09 '14 at 00:45
  • 1
    This answer goes to the how vs. why question. The software I work in is complex by nature. There is a lot of business logic. There's enough that not everyone is always going to know what is going on and why. That's where comments come in. I don't feel the need to explain how I am doing a type of business logic check, but I do like to explain WHY the check is being done. – scott.korin Sep 09 '14 at 15:52
  • @DavidMulder You are wrong. Logic is not a subset of natural language. If that were the case, then logic would have some flavor of German, maybe some Indonesian or Farsi mixed-in to its syntax structure. Logic is set apart because all grammatical operators are defined, whereas all natural language operators (modifiers, verb placement etc.) are derived depending upon the human interpretation of it. If you've ever written a lexical analyzer, you'd see that formal language is waaaaaay removed from natural language. – FlavorScape Sep 09 '14 at 22:09
  • 1
    @aecolley Write it well in the first place. Confusing code is buggy code, because being confusing is a bug. – Miles Rout Sep 17 '14 at 14:08
  • "Explaining yourself is good, not needing to do so is better." Is just rhetoric. Its easier said than done and sometimes its not possible without separating the problem in new function that adds more complexity than simplicity. – magallanes Oct 22 '17 at 21:39
112

There's a bunch of different reasons for code to be complicated or confusing. The most common reasons are best addressed by refactoring the code to make it less confusing, not by adding comments of any kind.

However, there are cases where a well-chosen comment is the best choice.

  • If it is the algorithm itself that is complicated and confusing, not just its implementation—the kind that get written up in math journals and are ever after referred to as Mbogo's Algorithm—then you put a comment at the very beginning of the implementation, reading something like "This is Mbogo's Algorithm for refrobnicating widgets, originally described here: [URL of paper]. This implementation contains refinements by Alice and Carol [URL of another paper]." Don't try to go into any more detail than that; if someone needs more detail they probably need to read the entire paper.

  • If you have taken something that can be written as one or two lines in some specialized notation and expanded it out into a big glob of imperative code, putting those one or two lines of specialized notation in a comment above the function is a good way to tell the reader what it's supposed to do. This is an exception to the "but what if the comment gets out of sync with the code" argument, because the specialized notation is probably much easier to find bugs in than the code. (It's the other way around if you wrote a specification in English instead.) A good example is here: https://dxr.mozilla.org/mozilla-central/source/layout/style/nsCSSScanner.cpp#1057 ...

    /**
     * Scan a unicode-range token.  These match the regular expression
     *
     *     u\+[0-9a-f?]{1,6}(-[0-9a-f]{1,6})?
     *
     * However, some such tokens are "invalid".  There are three valid forms:
     *
     *     u+[0-9a-f]{x}              1 <= x <= 6
     *     u+[0-9a-f]{x}\?{y}         1 <= x+y <= 6
     *     u+[0-9a-f]{x}-[0-9a-f]{y}  1 <= x <= 6, 1 <= y <= 6
    
  • If the code is straightforward overall, but contains one or two things that look excessively convoluted, unnecessary, or just plain wrong, but have to be that way because of reasons, then you put a comment immediately above the suspicious-looking bit, in which you state the reasons. Here's a simple example, where the only thing that needs explaining is why a constant has a certain value.

    /* s1*s2 <= SIZE_MAX if s1 < K and s2 < K, where K = sqrt(SIZE_MAX+1) */
    const size_t MUL_NO_OVERFLOW = ((size_t)1) << (sizeof(size_t) * 4);
    if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) &&
        nmemb > 0 && SIZE_MAX / nmemb < size)
      abort();
    
zwol
  • 2,576
  • 1
  • 16
  • 16
  • 27
    That's an outrage, `4` should be `CHAR_BIT / 2` ;-) – Steve Jessop Sep 01 '14 at 09:06
  • @SteveJessop: Would anything preclude an implementation where `CHAR_BITS` was 16 and sizeof(size_t) was 2, but the maximum value of size_t was e.g. 2^20 [size_t containing 12 padding bits]? – supercat Sep 02 '14 at 13:29
  • 3
    @supercat I don't see anything that obviously precludes that in C99, which means that example *is* technically incorrect. It happens to be taken from (a slightly modified version of) OpenBSD's `reallocarray`, and OpenBSD generally does not believe in catering to possibilities that don't happen in *their* ABI. – zwol Sep 02 '14 at 13:37
  • 3
    @Zack: If the code is designed around POSIX assumptions, using CHAR_BITS might give the impression that the code could work with values other than 8. – supercat Sep 02 '14 at 13:45
  • @supercat Indeed. https://stackoverflow.com/questions/25625096/compute-sqrtsize-max1-using-only-integer-constant-expressions-catering-for-w – zwol Sep 02 '14 at 13:52
  • Most numeric algorithms are expressed without regard for finite-precision and finite-magnitude floating point numbers and the like, and implementations will give anything from subtly different to completely different results from the analytical solution. That's one reason not to trust anybody who says they've implemented Mbogo's algorithm - they've very likely approximated it. – l0b0 Sep 02 '14 at 19:34
  • @supercat: That's allowed, yes. Regardless of whether it uses `CHAR_BIT` or `4`, this code also assumes that `size_t` has no padding bits. The comment hints at that by saying that `K` is `sqrt(SIZE_MAX+1)`, whereas the code only achieves what the comment says under the unstated assumption. You could `assert(SIZE_MAX / MUL_NO_OVERFLOW >= MUL_NO_OVERFLOW-1)` or something like that. I was winking because personally I don't think this makes the code "incorrect" as such, it makes it not-completely-portable. I can't remember whether Posix guarantees no padding bits. – Steve Jessop Sep 03 '14 at 08:18
  • @SteveJessop: I wish the standards committee would provide a means of defining integer types with machine-independent behavior. Rules that define the behavior of code based upon the size of `int` often make it impossible to write portable code without using ugly typecasts to render the rules irrelevant. If truly-machine-independent types existed, 32-bit code which was migrated to use them could work correctly even on machines where `int` was 64 bits. Without such type, `int` will, for compatibility, likely have to remain 32 bits even on hardware where 64 would be more efficient. – supercat Sep 03 '14 at 15:27
  • @supercat The `_least` and `_fast` types in `stdint.h` were *supposed* to do that job, but I have yet to see a situation where they were the right thing; turns out exact-width types are what everyone _really_ wants. (And look at Ada representation clauses sometime if you want to see a language where the design committee really thought about how much control system programmers want over this sort of thing.) – zwol Sep 03 '14 at 15:53
  • 2
    @Zack: For exact-width unsigned types to be useful, their semantics would need to be defined independent of the size of `int`. As it is, given `uint32_t x,y,z;`, the meaning of `(x-y) > z` depends upon the size of `int`. Further, a language designed for writing robust code should allow programmers to distinguish between a type where computations are expected to exceed the range of the type and should silently wrap, versus one where computations exceeding the range of the type should trap, versus one where computations aren't expected to exceed the range of the type, but... – supercat Sep 03 '14 at 16:04
  • ...it would be better for the compiler to ignore that possibility than to add code to force wrapping or trapping behavior [given `ufast32_t a=1,b=2; ufast32_t x=a-b; ufast64_t y=x; if (b>a) doSomething();`, I would suggest that the compiler be allowed to trap or store any value in `y` (even those beyond the range of a 64-bit int, if it happens to be a 128-bit register) but if the arithmetic doesn't trap the compiler shouldn't be allowed to skip the call to `doSomething();`]. – supercat Sep 03 '14 at 16:15
  • @supercat ... yeah, you're headed down the road that leads to representation clauses. – zwol Sep 03 '14 at 16:52
  • @Zack: A difficulty with representation clauses (and I've looked at the documentation for ADA, and don't quite see what it does to handle this issue) is that given e.g. [Pascal syntax] `var w,x,y,z: 0..60000; v: 0..16000000;`, it's not clear how `v=(w*x*y)/z;` should be evaluated. One could make a sound argument that the result of the multiplication should be able to handle any value up to 960,000,000,000, but such behavior would not always be practical. One could, however, say that all computations on *numeric* integers which would fit within the largest int-number type without overflow... – supercat Sep 03 '14 at 17:13
  • ...should yield the same result as if they had been performed entirely using that type, while computations on wrapping algebraic rings should be performed with semantics appropriate to such rings. Given an expression like `w=x*y*z;` a compiler could recognize that since the result was being stored to a variable that could only handle values up to 60000, there would be no need for a 48-bit multiply. – supercat Sep 03 '14 at 17:19
  • @supercat I don't mean to claim that representation clauses solve *all* the problems in this area, only that the Ada committee clearly thought about this class of issues more than the C committee did. I also know that for `(a*b)/c` constructs one very often wants the intermediate quantity to be wider than the result, but that's the extent of my own thinking about "how *should* this work" questions. – zwol Sep 03 '14 at 17:34
  • @supercat: my thinking on that is that the default promotion to `int` was an incredibly harmful optimization for exactly the reason you say. Quite aside from the value of types with machine-independent arithmetic, I don't think that arithmetic in `unsigned short` should ever have depended on the size of `int` in the first place. Ship's sailed on that, though, but if it hadn't been that way then all numeric types would have behavior defined only by their own representation, not also by the representation of `int`, and so `(u)intN_t` would indeed be machine-independent if they exist at all. – Steve Jessop Sep 04 '14 at 09:16
  • @SteveJessop: I don't think the ship needs to be viewed as having sailed; if the Committee were to introduce a new syntax for declaring implementation-independent integer types, it could define new rules for arithmetic on those types without breaking any code which uses older types. For example, it could specify that a value of type `int<32,uwrap>` be considered a member of an algebraic ring rather than a "number"; operations may be performed between rings and numbers, yield members of the same ring, or between matching ring types (likewise). Thus, adding an `int` to an `int<32,uwrap>`... – supercat Sep 04 '14 at 16:06
  • ...would yield an `int<32,uwrap>` regardless of whether `int` is 16 bits, 32 bits, or 64 bits. If the standard wanted to be a little nicer, it could even specify slightly different rules for bitwise operators (e.g. using `&` on two rings should yield the larger; `|` on two rings should yield the smaller; using "~" on a ring should behave as though it yields a ring whose size is infinite, but which may be coerced to any smaller one). Since C doesn't allow operator overloading, provided the largest unsigned type was a "ring" rather than a "number", it should be practical to say that... – supercat Sep 04 '14 at 16:12
  • ...all operations on "number" [numerical integer, as distinct from `int`] types will yield the same result as if they had been performed on the system's largest number type, since in most cases a compiler could infer that computations could be performed using smaller types [the biggest difficulties would be non-constant shifts or divides; even if the language compelled the use of extra typecasts in an expression like `((v<> z`, explicitly indicating the expected precision of the intermediate result would probably be a good idea anyway [one cast of the sum should suffice] – supercat Sep 04 '14 at 16:18
62

So what's wrong with explaining complicated code with a comment?

It's not a question of right or wrong, but of the 'best practice', as defined in Wikipedia article:

A best practice is a method or technique that has consistently shown results superior to those achieved with other means, and that is used as a benchmark.

So the best practice is to try to improve the code first, and to use English if that is not possible.

It's not a law, but it's much more common to find commented code that requires refactoring than refactored code that requires comments, the best practice reflects this.

gnat
  • 21,442
  • 29
  • 112
  • 288
FMJaguar
  • 3,039
  • 18
  • 14
  • 44
    +1 for "it's much more common to find commented code that requires refactoring than refactored code that requires comments" – Brandon Sep 01 '14 at 01:42
  • 8
    Okay, but how often is that comment: `//This code seriously needs a refactor` – Erik Reppen Sep 05 '14 at 11:21
  • 2
    Of course, any so-called best practice not backed up by a rigorous scientific study is merely an opinion. – Blrfl Feb 07 '15 at 12:57
59

A day will come when your beautiful, perfectly crafted, well structured and readable code won't work. Or it won't work well enough. Or a special case will arise where it doesn't work and needs adjusting.

At that point, you will need to do something that changes things so it works correctly. Particularly in the case where there are performance problems, but also often in scenarios where one of the libraries, APIs, web services, gems or operating systems you are working with doesn't behave as expected, you can end up making suggestions that are not necessarily inelegant, but are counter-intuitive or non-obvious.

If you don't have some comments to explain why you have chosen that approach there is a very good chance that someone in future ( and that someone may even be you ) will look at the code, see how it could be "fixed" to something more readable and elegant and inadvertently undo your fix, because it doesn't look like a fix.

If everyone always wrote perfect code then it would be obvious that code that looks imperfect is working around some tricky intervention from the real world, but that isn't how things work. Most programmers often write confusing or somewhat tangled code so when we encounter this it is a natural inclination to tidy it up. I swear my past self is an actual idiot whenever I read old code I have written.

So I don't think of comments as an apology for bad code, but maybe as an explanation for why you didn't do the obvious thing. Having // The standard approach doesn't work against the 64 bit version of the Frobosticate Library will allow future developers, including your future self, to pay attention to that part of the code and test against that library. Sure, you might put the comments in your source control commits too, but people will only look at those after something has gone wrong. They will read code comments as they change the code.

People who tell us that we should always be writing theoretically perfect code are not always people with a lot of experience of programming in real-world environments. Sometimes you need to write code that performs to a certain level, sometimes you need to interoperate with imperfect systems. That doesn't mean that you can't do this in elegant and well written ways, but non-obvious solutions need explanation.

When I am writing code for hobby projects that I know nobody else will ever read I still comment parts that I find confusing - for example, any 3D geometry involves maths which I'm not entirely at home with - because I know when I come back in six months I will have totally forgotten how to do this stuff. That's not an apology for bad code, that's an acknowledgement of a personal limitation. All I would do by leaving it uncommented is create more work for myself in future. I don't want my future self to have to relearn something unnecessarily if I can avoid it now. What possible value would that have?

glenatron
  • 8,729
  • 3
  • 29
  • 43
  • This question isn't about comment that explain "why" you do something. It's about comments that explain "how". – Christian Sep 01 '14 at 12:51
  • 5
    @Christian is it? The first line references that statement, certainly, but beyond that it is a little broader as I understand it. – glenatron Sep 01 '14 at 13:11
  • 9
    "I swear my past self is an actual idiot whenever I read old code I have written." Four years into my development career and I find this is an occurrence that happens whenever I look at anything older than 6 months or so. – Ken Sep 04 '14 at 18:18
  • 6
    In many cases, the most informative and useful historical information relates to things which are considered but decided against. There are many cases where someone chooses approach X for something and some other approach Y would seem better; in some of those cases, Y will "almost" work better than X, but turn out to have some insurmountable problems. If Y was avoided because of those problems, such knowledge can help prevent others from wasting their time on unsuccessful attempts to implement approach Y. – supercat Sep 04 '14 at 19:39
  • 4
    On a day to day basis I use work in progress comments a lot too- they aren't there for the long run, but dropping in a TODO note or a short section to remind me what I was going to do next can be a useful reminder in the morning. – glenatron Sep 05 '14 at 09:08
  • I feel like your final paragraph shouldn't be restricted to personal projects as it can make naturally complex code much more readable. As an example, even simple regex statements can confuse developers that don't normally work with them. Sure, any developer worth his salt should know basic regex or at least be able to decipher it but simply describing the match can avoid a lot of documentation reading. – Lilienthal Jul 15 '15 at 13:30
  • 1
    @Lilienthal, I don't think that last para is *restricted* to personal projects—he said "...I **still** comment parts that I find confusing." – Wildcard Nov 30 '15 at 18:38
31

The need for comments is inversely proportional to the abstraction level of the code.

For example, Assembly Language is, for most practical purposes, unintelligible without comments. Here's an excerpt from a small program that calculates and prints terms of the Fibonacci series:

main:   
; initializes the two numbers and the counter.  Note that this assumes
; that the counter and num1 and num2 areas are contiguous!
;
    mov ax,'00'                     ; initialize to all ASCII zeroes
    mov di,counter                  ; including the counter
    mov cx,digits+cntDigits/2       ; two bytes at a time
    cld                             ; initialize from low to high memory
    rep stosw                       ; write the data
    inc ax                          ; make sure ASCII zero is in al
    mov [num1 + digits - 1],al      ; last digit is one
    mov [num2 + digits - 1],al      ; 
    mov [counter + cntDigits - 1],al

    jmp .bottom         ; done with initialization, so begin

.top
    ; add num1 to num2
    mov di,num1+digits-1
    mov si,num2+digits-1
    mov cx,digits       ; 
    call    AddNumbers  ; num2 += num1
    mov bp,num2         ;
    call    PrintLine   ;
    dec dword [term]    ; decrement loop counter
    jz  .done           ;

    ; add num2 to num1
    mov di,num2+digits-1
    mov si,num1+digits-1
    mov cx,digits       ;
    call    AddNumbers  ; num1 += num2
.bottom
    mov bp,num1         ;
    call    PrintLine   ;
    dec dword [term]    ; decrement loop counter
    jnz .top            ;
.done
    call    CRLF        ; finish off with CRLF
    mov ax,4c00h        ; terminate
    int 21h             ;

Even with comments, it can be quite complicated to grok.

Modern Example: Regexes are often very low abstraction constructs (lower case letters, number 0, 1, 2, new lines, etc). They probably need comments in the form of samples (Bob Martin, IIRC, does acknowledge this). Here is a regex that (I think) should match HTTP(S) and FTP URLs:

^(((ht|f)tp(s?))\://)?(www.|[a-zA-Z].)[a-zA-Z0-9\-\.]+\.(com|edu|gov|m
+il|net|org|biz|info|name|museum|us|ca|uk)(\:[0-9]+)*(/($|[a-zA-Z0-9\.
+\,\;\?\'\\\+&amp;%\$#\=~_\-]+))*$

As the languages progress up the abstraction hierarchy, the programmer is able to use evocative abstractions (variable name, function names, class names, module names, interfaces, callbacks, etc) to provide built-in documentation. To neglect to take advantage of this, and use comments to paper over it is lazy, a disservice to and disrespectful of the maintainer.

I am thinking of Numerical Recipes in C translated mostly verbatim to Numerical Recipes in C++, which I infer began as Numerical Recipes (in FORTAN), with all the variables a, aa, b, c, cc, etc maintained through each version. The algorithms may have been correct, but they did not take advantage of the abstractions the languages provided. And they p*** me off. Sample from a Dr. Dobbs article - Fast Fourier Transform:

void four1(double* data, unsigned long nn)
{
    unsigned long n, mmax, m, j, istep, i;
    double wtemp, wr, wpr, wpi, wi, theta;
    double tempr, tempi;

    // reverse-binary reindexing
    n = nn<<1;
    j=1;
    for (i=1; i<n; i+=2) {
        if (j>i) {
            swap(data[j-1], data[i-1]);
            swap(data[j], data[i]);
        }
        m = nn;
        while (m>=2 && j>m) {
            j -= m;
            m >>= 1;
        }
        j += m;
    };

    // here begins the Danielson-Lanczos section
    mmax=2;
    while (n>mmax) {
        istep = mmax<<1;
        theta = -(2*M_PI/mmax);
        wtemp = sin(0.5*theta);
        wpr = -2.0*wtemp*wtemp;
        wpi = sin(theta);
        wr = 1.0;
        wi = 0.0;
        for (m=1; m < mmax; m += 2) {
            for (i=m; i <= n; i += istep) {
                j=i+mmax;
                tempr = wr*data[j-1] - wi*data[j];
                tempi = wr * data[j] + wi*data[j-1];

                data[j-1] = data[i-1] - tempr;
                data[j] = data[i] - tempi;
                data[i-1] += tempr;
                data[i] += tempi;
            }
            wtemp=wr;
            wr += wr*wpr - wi*wpi;
            wi += wi*wpr + wtemp*wpi;
        }
        mmax=istep;
    }
}

As a special case about abstraction, every language has idioms / canonical code snippets for certain common tasks (deleting a dynamic linked list in C), and regardless of how they look, they shouldn't be documented. Programmers should learn these idioms, as they are unofficially part of the language.

So the take away: Non-idiomatic code built from low-level building blocks that can't be avoided needs comments. And this is necessary WAAAAY less than it happens.

Toby Speight
  • 550
  • 3
  • 14
Kristian H
  • 1,261
  • 8
  • 12
  • 2
    Nobody should really be writing a line like this in assembly language: `dec dword [term] ; decrement loop counter`. On the other hand, what your assembly language example is missing is a comment before each "code paragraph" explaining what the next block of code does. In that case, the comment would typically be equivalent to a single line in pseudocode, such as `;clear the screen`, followed by the 7 lines it actually takes to clear the screen. – Scott Whitlock Sep 02 '14 at 16:57
  • 2
    Yes, there are what I would consider some unnecessary comments in the assembly sample, but to be fair, it is pretty representative of 'Good' Assembly style. Even with a one or two line paragraph prologue, the code would be really hard to follow. I understood the ASM sample better than the FFT example. I programmed an FFT in C++ in grad school, and it didn't look anything like this, but then we were using the STL, iterators, functors an quite a few method calls. Not as fast as the monolithic function, but a lot easier to read. I will try to add it to contrast to the NRinC++ sample. – Kristian H Sep 02 '14 at 17:09
  • Did you mean `^(((ht|f)tps?)\:\/\/)?(www\.)*[a-zA-Z0-9\-\.]+\.(com|edu|gov|mil|net|org|biz|info|name|museum|us|ca|uk)(\:[0-9]+)*(\/($|[a-zA-Z0-9\.\,\;\?\'\\\+&%\$#\=~_\-]+))*$` ? Be aware of numeric addresses. – izabera Sep 04 '14 at 13:51
  • More or less my point: some things built from very low level abstractions aren't easy to read or verify. Comments (and, not to get too far off track, TESTS) can be useful, and not a detriment. At the same time, not using higher level abstractions that are available (:alpha: :num: where available) makes it harder to understand, even with good comments, than using the higher level abstractions. – Kristian H Sep 04 '14 at 15:18
  • 3
    +1 : `"The need for comments is inversely proportional to the abstraction level of the code."` Pretty much sums up everything right there. – Gerrat Sep 04 '14 at 21:16
  • Assembly fight! – Erik Reppen Sep 05 '14 at 11:23
22

I don't believe there's anything wrong with comments in code. The idea that comments are somehow bad in my opinion is due to some programmers taking things too far. There's a lot of bandwagoning in this industry, particularly towards extreme views. Somewhere along the way commented code became equivalent to bad code and I'm not sure why.

Comments do have problems - you need to keep them updated as you update the code they refer to, which happens far too infrequently. A wiki or something is a more appropriate resource for thorough documentation about your code. Your code should be readable without requiring comments. Version control or revision notes should be where you describe code changes you made.

None of the above invalidates the use of comments, however. We don't live in an ideal world so when any of the above fail for whatever reason, I'd rather have some comments to fall back.

Roy
  • 513
  • 2
  • 8
19

I think you're reading a little bit too much in to what he's saying. There are two distinct parts to your complaint:

What's wrong with explaining (1) a complex algorithm or (2) a long and convoluted piece of code with a descriptive comment?

(1) is inevitable. I don't think that Martin would disagree with you. If you're writing something like the fast inverse square root, you're going to need some comments, even if it's just "evil floating point bit level hacking." Barring something simple like a DFS or binary search, it's unlikely that the person reading your code will have experience with that algorithm, and so I think there should be at least a mention in the comments about what it is.

Most code isn't (1), however. Rarely will you write a piece of software that's nothing but hand-rolled mutex implementations, obscure linear algebra operations with poor library support, and novel algorithms known only to your company's research group. Most code consists of library/framework/API calls, IO, boilerplate, and unit tests.

This is the kind of code that Martin is talking about. And he addresses your question with the quote from Kernighan and Plaugher at the top of the chapter:

Don’t comment bad code—rewrite it.

If you have long, convoluted sections in your code, you have failed to keep your code clean. The best solution to this problem isn't to write a paragraph-long comment at the top of the file to help future developers muddle through it; the best solution is to rewrite it.

And this is exactly what Martin says:

The proper use of comments is to compensate for our failure to express ourself in code...Comments are always failures. We must have them because we cannot always figure out how to express ourselves without them, but their use is not a cause for celebration.

This is your (2). Martin agrees that long, convoluted code does need comments -- but he puts the blame for that code on the shoulders of the programmer who wrote it, not some nebulous idea that "we all know that's not enough." He argues that:

Clear and expressive code with few comments is far superior to cluttered and complex code with lots of comments. Rather than spend your time writing the comments that explain the mess you’ve made, spend it cleaning that mess.

Patrick Collins
  • 2,165
  • 18
  • 24
  • 3
    If a developer I was working with simply wrote "evil floating point bit level hacking" to explain the fast square-root algorithm - they'd get a talking to by me. So long as they included a reference to somewhere more useful I'd be happy though. – Michael Anderson Sep 01 '14 at 03:56
  • 9
    I disagree in one way - a comment explaining how something bad works is a lot quicker. Given some code that is likely not to be touched again (most code I guess) then a comment is a better business solution than a big refactoring, that often introduces bugs (as a fix that kills relied-upon bug is still a bug). A perfect world of perfectly understandable code is not available to us. – gbjbaanb Sep 01 '14 at 07:42
  • @gbjbaanb, Sadly, this is true. In a perfect world, or one led by programmers & not businesspeople, this code would still be fixed, before the product is shipped. In this world, the best you can do is get them to fix it in a patch some years down the road, likely right before end-of life. – trysis Sep 01 '14 at 14:07
  • 2
    @trysis haha, yes but in a world where the programmers are responsible and not businesspeople, they'll never ship as they're forever gold-plating a constantly refactored codebase in a vain quest for perfection. – gbjbaanb Sep 01 '14 at 14:26
  • Hey, I never said it would be a good world, or a perfect one. – trysis Sep 01 '14 at 15:00
  • @gbjbaanb In fairness, Martin's book is primarily about doing it right the first time, not fixing it later on when you've already messed up. – Patrick Collins Sep 02 '14 at 02:33
  • 4
    @PatrickCollins nearly everything I read on the web is about doing it right first time. Almost nobody wants to write articles on fixing up messes! Physicists say "given a perfect sphere..." Comp.Scientists say "given a greenfield development..." – gbjbaanb Sep 02 '14 at 07:26
  • 2
    The best solution is to rewrite it given infinite time; but given someone else's code base, typical corporate deadlines, and reality; sometimes the best thing to do is comment, add a TODO: Refactor and get that refactor into the next release; and that fix that needed to be done yesterday done now. The thing about all of this idealistic talk about just refactoring is it doesn't account for how things really work in the work place; sometimes there are higher priorities and soon enough deadlines that will preempt fixing legacy poor-quality code. That's just how it is. – hsanders Sep 04 '14 at 18:17
8

What's wrong with explaining a complex algorithm or a long and convoluted piece of code with a descriptive comment?

Nothing as such. Documenting your work is good practice.

That said, you have a false dichotomy here: writing clean code vs. writing documented code - the two are not in opposition.

What you should focus on is simplifying and abstracting complex code into simpler code, instead of thinking "complex code is fine as long as it is commented".

Ideally, your code should be simple and documented.

This way, instead of other developers (including yourself) having to read the entire algorithm line by line to figure out what it does, they can just read the friendly descriptive comment you wrote in plain English.

True. This is why all your public API algorithms should be explained in the documentation.

So after writing a complex piece of code written in a partly human-readable programming language, why not add a descriptive and concise comment explaining the operation of the code in friendly and understandable English?

Ideally, after writing a complex piece of code you should (not an exhaustive list):

  • consider it a draft (i.e. plan to re-write it)
  • formalize the algorithm entry points/interfaces/roles/etc (analize and optimize interface, formalize abstractions, document preconditions, postconditions and side effects and document error cases).
  • write tests
  • cleanup and refactor

None of these steps are trivial to do (i.e. each can take a few hours) and the rewards for doing them are not immediate. As such, these steps are (almost) always compromized on (by developers cutting corners, managers cutting corners, deadlines, market constraints/other real world conditions, lack of experience etc).

[...] some algorithms are complex. And therefore are hard to understand when reading them line by line.

You should never have to rely on reading the implementation to figure out what an API does. When you do that, you are implementing client code based on the implementation (instead of the interface) and that means your module coupling is already shot to hell, you are potentially introducing undocumented dependendencies with every new line of code that you write, and are already adding technical debt.

Is it really that bad to explain a complex algorithm with a few lines of comments about it's general operation?

No - that is good. Adding a few lines of comments is not enough though.

What's wrong with explaining complicated code with a comment?

The fact that you shouldn't have complicated code, if that can be avoided.

To avoid complicated code, formalize your interfaces, spend ~ 8 times more on API design than you spend on the implementation (Stepanov suggested spending at least 10x on the interface, compared with the implementation), and go into developing a project with the knowledge that you are creating a project, not just writing some algorithm.

A project involves API documentation, functional documentation, code/quality measurements, project management and so on. None of these processes are one-off, fast steps to make (they all take time, require forethought and planning, and they all require that you come back to them periodically and revise/complete them with details).

utnapistim
  • 5,285
  • 16
  • 25
  • 3
    "You should never have to rely on reading the implementation to figure out what an API does." Sometimes this is inflicted on you by an upstream that you're committed to using. I had a particularly unsatisfying project littered with comments of the form "the following ugly Heath Robinson code exists because simpleAPI() does not work properly on this hardware despite what the vendor claims". – pjc50 Sep 01 '14 at 14:56
6

instead of other developers (including yourself) having to read the entire algorithm line by line to figure out what it does, they can just read the friendly descriptive comment you wrote in plain English.

I would consider this a slight abuse of "comments". If the programmer wants to read something instead of the entire algorithm, then that's what function documentation is for. OK, so the function documentation might actually appear in comments in the source (perhaps for extraction by doc tools), but although syntactically it's a comment as far as your compiler is concerned, you should consider them separate things with separate purposes. I don't think "comments should be scarce" is necessarily intended to mean "documentation should be scarce" or even "copyright notices should be scarce"!

Comments in the function are for someone to read as well as the code. So if you have a few lines in your code that are difficult to understand, and you can't make them easy to understand, then a comment is useful for the reader to use as a placeholder for those lines. This could be very useful while the reader is just trying to get the general gist, but there are a couple of problems:

  • Comments aren't necessarily true, whereas the code does what it does. So the reader is taking your word for it, and this is not ideal.
  • The reader doesn't understand the code itself yet, so until they come back to it later they still aren't qualified to modify or re-use it. In which case what are they doing reading it?

There are exceptions, but most readers will need to understand the code itself. Comments should be written to assist that, not to replace it, which is why you're generally advised that comments should say "why you're doing it". A reader who knows the motivation for the next few lines of code has a better chance of seeing what they do and how.

Steve Jessop
  • 5,051
  • 20
  • 23
  • 6
    One useful place for comments: in scientific code, you can often have computations that are quite complex, involving lots of variables. For the sanity of the programmer, it makes sense to keep variable names really short, so you can look at the maths, rather than the names. But that makes it really hard to understand for the reader. So a short description of what is going on (or better, a reference to the equation in a journal article or similar), can be really helpful. – naught101 Sep 01 '14 at 08:50
  • 1
    @naught101: yes, especially since the paper you're referring to also probably used single-letter variable names. It's usually easier to see that the code does indeed follow the paper if you use the same names, but that's in conflict with the goal of the code being self-explanatory (it's explained *by the paper* instead). In this case, a comment where each name is defined, saying what it actually means, substitutes for meaningful names. – Steve Jessop Sep 01 '14 at 08:56
  • 1
    When I am searching for something specific in code (where is this specific case handled?), I don't want to read and understand paragraphs of code just to discover that it is not the place after all. I need comments that summarize in a single line what the next paragraph is doing. This way, I will quickly locate the parts of code related to my problem and skip over uninteresting details. – Florian F Sep 01 '14 at 11:57
  • 1
    @FlorianF: the traditional response is that variable and function names should indicate roughly what the code is about, and hence let you skim over things that certainly aren't about what you're looking for. I agree with you that this doesn't always succeed, but I don't agree so strongly that I think *all* code needs to be commented to aid searching or skim-reading. But you're right, that's a case where someone is reading your code (sort of) and legitimately doesn't need to understand it. – Steve Jessop Sep 01 '14 at 12:01
  • In regards to the section you quoted, one of the risks is that the code and comments get out of synch such that the comment no longer accurately describes the code. I see this all the time. I have actually seen people wholesale copy a class I wrote, rename it, make it do something completely different, and leave my comments in place. –  Sep 01 '14 at 17:10
  • 2
    @Snowman People could do that with variable names. I have seen code where the variable listOfApples contained a list of Bananas. Someone copied the code processing the list of Apples and adapted it for Bananas without bothering changing the variable names. – Florian F Sep 01 '14 at 19:42
6

I forget where I read it but there is a sharp and clear line between what should appear in your code and what should appear as a comment.

I believe you should comment your intent, not your algorithm. I.e. comment what you meant to do, not on what you do.

For example:

// The getter.
public <V> V get(final K key, Class<V> type) {
  // Has it run yet?
  Future<Object> f = multitons.get(key);
  if (f == null) {
    // No! Make the task that runs it.
    FutureTask<Object> ft = new FutureTask<Object>(
            new Callable() {

              public Object call() throws Exception {
                // Only do the create when called to do so.
                return key.create();
              }

            });
    // Only put if not there.
    f = multitons.putIfAbsent(key, ft);
    if (f == null) {
      // We replaced null so we successfully put. We were first!
      f = ft;
      // Initiate the task.
      ft.run();
    }
  }
  try {
    /**
     * If code gets here and hangs due to f.status = 0 (FutureTask.NEW)
     * then you are trying to get from your Multiton in your creator.
     *
     * Cannot check for that without unnecessarily complex code.
     *
     * Perhaps could use get with timeout.
     */
    // Cast here to force the right type.
    return (V) f.get();
  } catch (Exception ex) {
    // Hide exceptions without discarding them.
    throw Throwables.asRuntimeException(ex);
  }
}

Here there is no attempt to state what each step performs, all it states is what it is supposed to do.

PS: I found the source I was referring to - Coding Horror: Code Tells You How, Comments Tell You Why

OldCurmudgeon
  • 778
  • 5
  • 11
  • 8
    The first comment: Has it run yet? Has what run yet? Same for the other comments. For someone not knowing what the code does, this is useless. – gnasher729 Sep 01 '14 at 14:48
  • 1
    @gnasher729 - Taken out of context almost any comment will be useless - this code is a demonstration of adding comments that indicate **intent** rather than attempting to **describe**. I am sorry that it does nothing for you. – OldCurmudgeon Sep 01 '14 at 14:55
  • 2
    A maintainer of that code won't have a context. It's not particularly difficult to figure out what the code does, but the comments are not helping. If you write comments, take your time and concentrate when you write them. – gnasher729 Sep 01 '14 at 22:51
  • BTW - The *Has it run yet* comment is referring to the `Future` and indicates that a `get()` followed by a check against `null` detects whether the `Future` has already been run - correctly documenting the **intent** rather than the **process**. – OldCurmudgeon Sep 05 '14 at 09:54
  • If you need to explain the comments, then surely something is wrong with the comments? Code that needs explaining can do what it's supposed to do perfectly fine. Comments that need explaining are broken. – gnasher729 Sep 08 '14 at 10:18
  • 1
    @OldCurmudgeon: Your reply is close enough to what I was thinking, that I'll just add this comment as an example of your point. While a comment isn't needed to explain clean code, a comment IS good to explain why coding was done ONE WAY OVER ANOTHER. In my limited experience, comments are often useful to explain idiosyncracies of the data set the code is working upon, or the business rules the code is meant to enforce. Commenting code that is added to fix a bug is a good example, if that bug happened because an assumption about the data was wrong. – Syntax Junkie Feb 14 '17 at 18:37
5

Often we have to do complicated things. It's certainly right to document them for future understanding. Sometimes the right place for this documentation is in the code, where the documentation can be kept up to date with the code. But it's definitely worth considering separate documentation. This can also be easier to present to other people, include diagrams, colour pictures, and so on. Then the comment is just:

// This code implements the algorithm described in requirements document 239.

or even just

void doPRD239Algorithm() { ...

Certainly people are happy with functions named MatchStringKnuthMorrisPratt or encryptAES or partitionBSP. More obscure names are worth explaining in a comment. You could also add bibliographic data and a link to a paper that you've implemented an algorithm from.

If an algorithm is complex and novel and not obvious, it's definitely worth a document, even if only for internal company circulation. Check the document into source control if you're worried about it getting lost.

There is another category of code which isn't so much algorithmic as bureaucratic. You need to set up parameters for another system, or interoperate with someone else's bugs:

/* Configure the beam controller and turn on the laser.
The sequence is timing-critical and this code must run with interrupts disabled.
Note that the constant 0xef45ab87 differs from the vendor documentation; the vendor
is wrong in this case.
Some of these operations write the same value multiple times. Do not attempt
to optimise this code by removing seemingly redundant operations.
*/
pjc50
  • 10,595
  • 1
  • 26
  • 29
  • 3
    I'd argue against naming functions/methods after their internal algorithm, most of the time the method used should be an internal concern, by all means document the top of your function with the method used, but don't call it `doPRD239Algorithm` that tells me nothing about the function without having to look up the algorith, the reason `MatchStringKnuthMorrisPratt` and `encryptAES` work is that they starts with a description of what they do, then follows up with a description of the methodology. – scragar Sep 02 '14 at 14:35
4

But we all know that's not enough.

Really? Since when?

Well designed code with good names is more than enough in the vast majority of cases. The arguments against using comments are well known and documented (as you refer to).

But these are guidelines (like anything else). In the rare case (in my experience, a about once every 2 years) where things would be worse when refactored into smaller legible functions (due to performance or cohesion needs) then go ahead - put in some lengthy comment explaining what the thing is actually doing (and why you're violating best practices).

Telastyn
  • 108,850
  • 29
  • 239
  • 365
  • 7
    I know it is not enough. – Florian F Sep 01 '14 at 11:47
  • 2
    Since when? Apparently, you already know the answer to that. "Well designed code with good names is more than enough in the *vast majority* of cases." So, it's probably not enough in a minority of cases, which is exact what the asker is asking. – Ellesedil Sep 02 '14 at 20:01
  • 3
    I am ever trying to decipher other peoples' code whom I wish had added some comments more than once every two years. – Ogre Psalm33 Sep 04 '14 at 20:15
  • @OgrePsalm33 - Do they have small methods and use good names? Bad code is bad, regardless of comments. – Telastyn Sep 04 '14 at 20:26
  • 2
    @Telastyn Unfortunately, when working on a large code base, "small" methods and "good" names are subjective to each developer (so is a good comment, for that matter). A developer writing Flarbigan graphical processing algorithm code for 7 years, can write something perfectly clear to him and similar developers, but would be cryptic to the new guy who spent the last 4 years developing Perbian grid infrastructure code. Then, 2 weeks later, the Flarbigan expert quits. – Ogre Psalm33 Sep 04 '14 at 21:04
  • @OgrePsalm33 - any codebase has subjective measures, hence coding standards and code reviews. – Telastyn Sep 04 '14 at 21:28
  • The funny thing with "good names" is that they are just comments in disguise. The compiler doesn't care what you call your procedure. It could be named for something that has later been taken out of its body. Or its name could a blatant lie because it was copied from somewhere else that was doing something similar but on a different kind of data. Yes, names can be updated, but so can comments. – Florian F Oct 10 '16 at 19:44
  • @FlorianF - except names appear at callsites in your code - and if they're functionally equivalent, why have _both_? – Telastyn Oct 10 '16 at 19:56
2

The principal purpose of code is commanding a computer to do something, so a good comment is never a substitute for good code because comments can't be executed.

That being said, comments in the source are one form of documentation for other programmers (including yourself). If the comments are about more abstract issues than what the code is doing at every step, you're doing better than average. That level of abstraction varies with the tool you're using. Comments accompanying assembly language routines generally have a lower level of "abstraction" than, for example, this APL A←0⋄A⊣{2⊤⍵:1+3×⍵⋄⍵÷2}⍣{⍺=A+←1}⎕. I think that would probably merit a comment about the problem it's intended to solve, hmmm?

Scott Leadley
  • 226
  • 1
  • 4
2

If the code is trivial, it doesn't need an explanatory comment. If the code is non-trivial, the explanatory comment will most likely also be non-trivial.

Now, the trouble with non-trivial natural language is that many of us are not very good at reading it or writing it. I'm sure your written communication skills are excellent, but nevertheless someone with a lesser grasp of written language might misunderstand your words.

If you try very hard to write natural language that cannot be misinterpreted you end up with something like a legal document (and as we all know those are more verbose and difficult to understand than code).

Code should be the most concise description of your logic, and there shouldn't be much debate about the meaning of your code because your compiler and platform have the final say.

Personally I wouldn't say that you should never write a comment. Only that you should consider why your code needs a comment, and how you might fix that. This seems to be a common theme in answers here.

Martin
  • 521
  • 2
  • 10
  • Exactly what I was thinking when I disagreed with the statement "A human can understand a piece of English much faster that he/she can understand a piece of code with the same meaning (as long as the operation isn't trivial)" Code is always less ambiguous and more concise. – stephenbayer Sep 02 '14 at 16:49
0

One point not yet mentioned is that sometimes commenting precisely what a piece of code does can be helpful in cases where a language uses a particular syntax for multiple purposes. For example, assuming all variables are of type float, consider:

f1 = (float)(f2+f3); // Force result to be rounded to single precision
f4 = f1-f2;

The effect of explicitly casting a float to float is to force the result to be rounded to single precision; the comment could thus be viewed as simply saying what the code does. On the other hand, compare that code to:

thing.someFloatProperty = (float)(f2*0.1); // Divide by ten

Here, the purpose of the cast is to prevent the compiler from squawking at the most efficient way of accurately computing (f2/10) [it's more accurate than multiply by 0.1f, and on most machines it's faster than dividing by 10.0f].

Without the comment, someone who was reviewing the former code might think the cast was added in a mistaken belief that it would be needed to prevent the compiler from squawking and that it wasn't needed. In fact, the cast serves the purpose of doing exactly what the language spec says it does: force the result of the computation to be rounded to single-precision even on machines where the rounding would be more expensive than keeping the result in higher precision. Given that a cast to float can have a number of different meanings and purposes, having a comment specify which meaning is intended in a particular scenario can help make clear that the actual meaning lines up with intent.

supercat
  • 8,335
  • 22
  • 28
  • I'm not sure that J. Random Programmer, looking at the second example, will realize that the constant is written 0.1 for a good reason, rather than because the original programmer forgot to type an 'f'. – David K Sep 07 '14 at 15:45
  • Especially during debugging, you never assume that anything has been done for a good reason. – gnasher729 Sep 08 '14 at 10:16
  • @DavidK: The purpose of my second example code was to contrast it with the first piece of code. In the second piece of code, the programmer's intention is probably to have `someFloatProperty` hold the most accurate representation of `f2/10` that it can; *the primary purpose of the second cast is thus simply to make the code compile*. In the first example, however, the cast clearly isn't needed for its normal purpose (changing one compile-time type to another) since the operands is already `float`. The comment serves to make clear that the cast *is* needed for a secondary purpose (rounding). – supercat Sep 08 '14 at 15:29
  • I agree with the notion that you don't need to make any comment about the `(float)` cast in the second example. The question is about the literal constant `0.1`. You explained (in the next paragraph of text) why we would write `0.1`: "it's more accurate than multiply by 0.1f." I'm suggesting that _those_ are the words that should be in the comment. – David K Sep 08 '14 at 16:54
  • @DavidK: I would certainly include the comment if I knew that 0.1f would be unacceptably imprecise, and would use 0.1f if I knew that the loss of precision would be acceptable and that *0.1f would in fact be materially faster than 0.1*. If I don't know either of those things to be true, preferred my coding habit would be to use `double` for constants or intermediate calculations whose value may not be representable as `float` [though in languages that require annoying explicit double-to-float casts, laziness may push be to use use of `float` constants not for speed, but to minimize annoyance]. – supercat Sep 08 '14 at 17:07
  • @DavidK: I wish there was a way of specifying a type whose semantics would be, essentially, "I'd wouldn't mind having the best precision I can get, but I'm willing to accept values which are rounded to `float` in cases where that saves memory or execution time". If on a particular machine, the fastest way to compute an expression like `a=b+(c*(1.0f/10.0f))` would be to evaluate everything as `double` and round the result to `float`, I see no reason the machine should use 0.10000000149 rather than (1.0/10.0) as its scaling constant. – supercat Sep 08 '14 at 17:14
  • @supercat: I thought you gave good reasons for writing the code as you did; if you're not sure about them, then the comment `// more accurate than f2*0.1f` becomes `// possibly more accurate than f2*0.1f`. I'm a bit baffled as to why you're so reluctant to express your own excellent reasoning in comments in code. Is the use of `double` literals within calculations of `float` values so routine among your coworkers that the reasons go without saying? If so, your experience is different than mine (lucky you) and we have no argument after all. – David K Sep 08 '14 at 17:46
-1

Comments that explain what the code does are a form of duplication. If you change the code and then forget to update the comments this can cause confusion. I am not saying don't use them, just use them judiciously. I subscribe to the Uncle Bob maxim: "Only comment what the code can't say".

murungu
  • 389
  • 1
  • 4
  • 6