PHP in Action Rotating Header Image

Show me your code comments and I’ll show why you don’t need them

Brandon Savage has written a blog post On Code Commenting And Technical Debt. He believes that code comments are a good way to minimize technical debt.

I’m surprised to find the term technical debt mentioned without being accompanied by the term refactoring. Refactoring is generally recognized (outside the PHP world) as the way to pay down technical debt. Commenting may help, but is clearly the second-best practice.

Martin Fowler puts it this way:

Technical Debt is a wonderful metaphor developed by Ward Cunningham to help us think about this problem. In this metaphor, doing things the quick and dirty way sets us up with a technical debt, which is similar to a financial debt. Like a financial debt, the technical debt incurs interest payments, which come in the form of the extra effort that we have to do in future development because of the quick and dirty design choice. We can choose to continue paying the interest, or we can pay down the principal by refactoring the quick and dirty design into the better design. Although it costs to pay down the principal, we gain by reduced interest payments in the future.

Brandon’s argument in favor of commenting is perfectly valid, but misses the crux of the matter, since he ignores the option of actually improving the code itself rather than just adding comments.

Let me also comment briefly on Marco Tabini’s reponse:

What I suspect Brandon really means is that the comments are there to illustrate the intentions of the author when those intentions are not immediately made obvious by the code itself.

Yes. And no. There is no absolute boundary, no limit in principle, to how intention-revealing code can be. It’s not necessarily easy in practice, though. As I’ve said before, it’s primarily inline comments that I’m objecting to. The comments I feel a need to write are often at the class level and address the interaction between different classes.

Anyway, arguing about it theoretically is not the way to resolve the issue. Show me some good examples of comments that serve to make code clearer and that supposedly can’t be usefully eliminated by refactoring the code into something more readable. I’ll either admit that you’re right or show you (or at least outline) how to do it differently. I do recognize that even inline comments are useful…occasionally.

(By the way, I seem to have missed Brandon’s comment (Where Comments Are Useful) to my comments considered harmful post last December.)

Share/Save/Bookmark

20 Comments

  1. Lukas says:

    I do not have the code anymore, nor permission to publish it, but I once did some serious screen scraping with several 200+ character regular expressions. Now you might argue that screen scraping is evil (since we were just being cheap about paying for the proper API) or using a regular expression for this, but it worked (more or less) and without comments to show some examples of what we were trying to parse it would have been even less possible to maintain the code when new formats needed to be handled.

    So what I am trying to say? I agree with the idea that comments are not really what you want, but sometimes they are inevitable to do peculiarities in the language being used. May it be that its not possible to nativly document data type for parameters or whatever else is not expressible or only very tediously or simply not efficient.

  2. Cameron says:

    I find these discussion amusing on an abstract level. There is one big assumption that is being made that completely negates the idea of “self-documenting code”.

    While most decent developers will try to do “self-documenting code”, what is self-documenting to one person may look like spaghetti to another (extreme example).

    An experienced developer will try to write efficient & self-documenting code, but a less experienced person may not understand what the original developer is a) meant to be doing & b) how they’re doing it.

    By that I mean that an experienced developer may know techniques to improve their code, and it’ll be obvious to them… but it may not be obvious to a less experienced developer. And assuming that all developers that will work on a particular piece of code will be able understand your “self-documenting code” is not just naive, it’s arrogant.

    Cameron.

  3. Tom says:

    I was quite surprised to see not one, but two rebuttals on this topic from rather well respected blogs. Personally, I think Source code documentation is nothing to argue about. It is an essential part of quality assurance in programming. Just do it.

    I think in English and I can grasp concepts written in plain English much faster than any so-called self-explanatory code could ever be written in. My brain is not a PHP engine. I have to translate code to English, then I can understand it. Understanding benefits from redundancy and paraphrasing. It’s not DRY. Programming languages are not designed to express meaning as efficient as natural languages are.

    In addition, if we believe programming against an interface is good practise, then we should describe this interface, so other team members, external parties or even the client (think DSL), does not have to bother with the insides. If I had to figure out Zend Framework or Symfony by the source code alone, I would be far from productive with them. That is another key aspect: abstraction. Docs tell me enough about a code to be able to use it. The code itself tells me how it does things.

    Also, documentation will not only enable humans to work with the documentation, but also machines. Think about how code completion will show you params and return values. Do you remember which param comes first in array_key_exist? in strstr? Let alone a foreign API?

    Finally, refactoring to reduce technical debt is – of course – valid. But refactoring is something you do later in the project, whereas documentation is something you do all the time. Brandon never said to do commenting instead of refactoring. And even when you refactor, there is no guarantee your code will be self-commenting. Breaking your code apart to simplify or clean certain code parts does reduce overall complexity. It just diverts them into smaller chunks, but you still have to digest them. And thats easier with documentation.

  4. I assume by “good examples” you mean none of the following:
    1) Obscure maths
    2) Highly optimized code
    3) Warning about broken-expectations… (don’t touch foo, even though it looks tempting!)

    The above items were taken from a comment on your “comments considered harmful” blogpost.

  5. Eli White says:

    Two quick responses:

    1) You talk about ‘refactoring instead of adding comments’. That to me misses another point. If you have to ‘go back and add comments’, don’t bother, you’ve missed the original point of commenting to my mind. As you are writing the code you should be filling in comments to explain the intentions behind the code. It should become second nature. Heck in some cases one can decide what a block of code needs to do, write a few comments explaining the ‘steps’, as an outline, and then fill in the code.

    But I agree, going ‘back’ to add some comments is a failure.

    2) I think that talking in ‘specific examples’ is going to be an exercise in futility. Any example is going to depend on the context, and therefore any example given can be argued, by either side, either way. The simplest line that ‘needs a comment’ to explain intentions to me is something like:

    $x -= 7;

    It’s obvious you are subtracting seven, but there is no explanation of the intent behind subtracting seven. Therefore there should be a comment explaining ‘why’ you are removing seven from the number there.

    Now, if the context is:
    function subtractSeven(&$x) {
    $x -= 7;
    }

    Then that’s obvious. But that line sitting in the middle of a bigger block of code may not be. (And even with a better named variable, it may not be)

    In any case, to me, part of the argument is that having been doing this ‘professional coding’ thing for 15 years or so now, I’ve never once complained that someone had done ‘too much’ commenting of their code, I’ve regularly complained that code wasn’t commented, because much of my work has been picking up previously written code, and needing to make strategic strikes inside of it. To do that without spending half-a-day or longer to ‘fully’ understand all aspects of the code, you need good comments.

    Eli

  6. dagfinn says:

    The number 7 is actually an excellent example of the phenomenon known as a magic number. The specific refactoring to use in this case is called Replace Magic Number with Symbolic Constant. So what you would do is to replace the line with (for example)

    $x = -DAYS_IN_WEEK;

    That makes it unnecessary to add a comment to clarify the intention behind the number 7. It’s superior to commenting which always carries the risk of comments and code getting out of sync.

  7. dagfinn says:

    @Tom: I agree that interface documentation is generally a good idea. As I’ve said repeatedly, my objection is to inline comments primarily.

    The idea that refactoring is something you do later in the project is a misunderstanding. The principle of Constant Refactoring assumes that you refactor as you’re developing the code. I refactor as soon as I see significant code duplication. And it doesn’t take much to make me consider it significant. Say, 2×5 lines or 3×3 lines.

    Refactor first, and then add comments if you must.

    @Herman Radtke: I would probably need actual examples to make a reasonable judgment about these three supposed exceptions. I guess that these are cases that sometimes require commenting, but not always.

  8. beberlei says:

    @Lukas: You can always extract a Regexp into a Class Constant with a good name and use sprintf() to fill the variable gaps. Then you can use a function/method for each regexp constant and have it take the variable parts as arguments with explicit names. You don’t need comments for this setup I suppose.

    @Eli: 7 is a magic number which is a code smell. You should add a class constant not a method for this. If its “const DAYS_IN_WEEK = 7;” for example the code would look like:

    $x -= self::DAYS_IN_WEEK;

  9. beberlei says:

    Oh the magic number argument was already made, sorry dagfinn :-)

  10. Dave says:

    Its all very well pulling everything out into a method with a descriptive name rather than commenting but you end up with 2 problems.

    First you end up with “onion code” which is just as bad as sphagetti code in that you have to follow through hundreds of method calls to work out where you are and the context of what you’re doing isnt locationally contextual to the code your in.

    The second is that you add more method calls to the stack with the associated overhead and more importantly you’re actually exhausting your stack. A few bytes here and there might not seem like much but if you have any kind of recursive processing you’ll reduce the total amount of work acheivable. In a language like C++ you introduce the probability that someone will leave a dangling pointer etc.

    At the end of the day commenting requires practically zero effort and is a professional courtesy to whoever has to maintain the code. In-line comments can be useful. For example as a maintenance programmer I’ve come across several helpful inline comments. For example something like:

    //This use to be a call to foo but its not guaranteed to be loaded since we can get here before loading bar.so

  11. dagfinn says:

    The objection about onion code is somewhat valid, but if it gets that bad the problem is probably that you haven’t got your abstractions straight. If you just have lots of methods and they’re not logically organized into classes, you will get that kind of problem. I will grant you that that may be more difficult than just extracting methods. One important piece of that puzzle is what Neal Ford calls SLAP (Single Level of Abstraction Principle) as in the presentation I was referring to in my previous blog post.

  12. dagfinn says:

    As for the idea that commenting requires practically zero effort, I disagree. It’s just like any other explanatory text. Making sure it’s accurate and communicates well is real work. And it’s even more difficult to remember to update comments correctly, which is why it’s frequently not done and comments turn out to be actively misleading and therefore worse than no comments.

  13. Dave says:

    The other problem of course with pulling methods out is that someone can then later call that method from an unexpected place, where assumed prconditions are not met for example. Visibility scope isnt necesarily a fix for this either since when extending functionality you’ll invariably have access to the new method. Visibility is really more a solution for separating interface from implentation.

    A method should encapsulate a unit of reuse, if it doesnt make sense to do that code block in isolation from its caller then it shouldnt be a separate method.

  14. dagfinn says:

    @Dave: To me, your argument seems to be that when you extract a method, you have something potentially reusable, and when it’s reusable it can be reused in a wrong way. Well, I think the benefit of having reusable code available far outweighs the risk of “abuse”.

    You say “..if it doesn’t make sense to do that code block in isolation from its caller…”. Why would anyone do it if it doesn’t make sense? If someone tries to use that method in a different context, it’s probably because it does make sense. But obviously they can’t assume that it will work perfectly right away. Still, they’re better off trying than writing duplicate code.

    The more interesting question is, what can you do to make the method and the class less likely to fail when it’s reused? That’s where terms like decoupling become relevant.

  15. One of the biggest reasons I still comment, though it didn’t fit with my argument regarding technical debt, is that my memory is imperfect. Writing a comment about a particularly unusual line of code or about the logic I used is often a perfect reminder to me about why I did something or how I did something.

    I honestly think that commenting code is a practice that operates shop-by-shop, and the standards should reflect the developers and their personalities. I doubt there’s one “correct” way to comment your code, just like there’s not one “correct” version control system or one “correct” bug tracker. It’s preference-based.

    My article reflects my personal view on commenting but is by no means authoritative on the subject as a whole. That said, commenting underperforming code *will* help you pay down that technical debt when the refactor comes, because you’ll have an easier time understanding it and perhaps the comments will lead you to it.

    And if you tell me that you write no underperforming code ever, well, then I don’t know what to tell you…

  16. dagfinn says:

    Code that’s hard to understand without comments will be improved by adding comments. That much we agree on. And, yes, the comments may be helpful when refactoring. That is, if they haven’t deteriorated to the point where they become misleading.

  17. Dave says:

    When writing comments you should treat them like you’re telling of a historial fact. You can tell when someone writes subjective comments, and that’s indeed not the right way to go IF the intentions of the author deviate from the actual purpose of the code.

    However, if you’re working with a team of developers and you have to spend time in eachother’s code from time to time you’ll be glad that they’re there. If you have to make adjustments to a 400-line function and all the comments you got are the ones in the PHPDoc, the chances are you’re going to try look for “clues” in the rest of the class as to how some variables are treated. Proper naming conventions help a lot in this case, but usually you want that extra bit of information.

    On the point of commenting for yourself, I’ll go with Brandon’s point of view. If you write several thousand lines of code each week you’re bound to forget how a certain function you wrote 4 months ago was made.

  18. [...] article from last week, “On Code Commenting and Technical Debt” raised a lot of response throughout the community. I think that discussion is great, and I’m all for a debate that [...]

  19. [...] Reiersol takes a more pragmatic approach to the issue and wants to see the code before making a decision on whether the comments are needed or [...]

  20. Bruce says:

    @Dave: To me, your argument seems to be that when you extract a method, you have something potentially reusable, and when it’s reusable it can be reused in a wrong way. Well, I think the benefit of having reusable code available far outweighs the risk of “abuse”.

    You say “..if it doesn’t make sense to do that code block in isolation from its caller…”. Why would anyone do it if it doesn’t make sense? If someone tries to use that method in a different context, it’s probably because it does make sense. But obviously they can’t assume that it will work perfectly right away. Still, they’re better off trying than writing duplicate code.

    The more interesting question is, what can you do to make the method and the class less likely to fail when it’s reused? That’s where terms like decoupling become relevant.

Leave a Reply