67

I'm trying to clean up some of my code using some best practices, and read that comments are almost always a bad idea for future maintainability. I've managed to get rid of most of them by refactoring to methods with good names.

However, there are several comments sprinkled around that explain why a single line of code needs to exist. I'm having trouble figuring out a way to get rid of these. They often follow this kind of structure:

// Needs to start disabled to avoid artifacts the first frame. Will enable after frame complete.
lineRenderer.enabled = false;

Then later on in the code I enable it.

I've thought about extracting it into a one-line method called StartLineRendererDisabledToAvoidArtifactsFirstFrame() but this doesn't seem all that clean to me.

Are there any best practices for dealing with such one-liners? Many of them explain the existence of code that on the surface looks superfluous but then actually has an important function. I want to guard against future deletion.

Sidenote: I've already run into some scenarios where refactoring/renaming has made these comments be out-of-date or in the wrong spot etc. so I definitely see why removing them would be useful.

Related but different question here:

EDIT BASED ON ANSWERS AND COMMENTS BELOW

Here's my takeaway from all the great discussion below.

  1. Comments are fine if there's not an easy way to refactor/rename for more clarity. But they should be used sparingly.
  2. Removing comments is generally a good thing. But some comments need to exist to help future readers understand the code without having to dig too deep.
  3. But they should explain the WHY, not the HOW.
  4. If the code is there for a particular reason that is very important or fixes a bug, it should probably also have a corresponding unit test anyway.
  5. Commit messages can help track why and where and how the commented code came to be.
Jonathan Leffler
  • 1,846
  • 14
  • 21
Adam B
  • 1,506
  • 2
  • 12
  • 20
  • 152
    There is nothing wrong with comments explaining the code if there no better way to convey that information. Best practices don't cover everything. – Martin K Feb 05 '20 at 17:30
  • 1
    @MartinK Are you essentially saying that "comment why, not how" is not a best practice? (which is fair opinion, I just always consider it to be one... something like https://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/) – Alexei Levenkov Feb 06 '20 at 02:06
  • 1
    This is one of the many uses of [RAII](https://en.wikipedia.org/wiki/Resource_acquisition_is_initialization). If your language of choice doesn't support RAII, that is very sad. You should upgrade to a language that does. Then, you can offload this setup and teardown work to a helper class, reducing the amount of code you have to write and comment. As an even bigger bonus, you don't have to worry about exception safety. – Cody Gray - on strike Feb 06 '20 at 02:18
  • 40
    Change the word "will" to "must" and call it a great comment. Don't let 'best practices' override your conscience and common sense. – joshp Feb 06 '20 at 02:44
  • _...but this doesn't seem all that clean to me_ - trust your self, if it doesn't seem clean - don't do this. Remember there are no absolute rules in software development - use right tool for the job in the current context – Fabio Feb 06 '20 at 02:45
  • 26
    *read that comments are almost always a bad idea for future maintainability* Who is the "they" who said that? It's too bad "they" aren't gun safety educators. Darwin would take care of this for us :) – Harper - Reinstate Monica Feb 06 '20 at 03:02
  • 16
    Your "name" is just a comment with the spaces removed. – jamesqf Feb 06 '20 at 03:27
  • 2
    Comments do not really help with "what is this doing" (too complex = break down, not use comments) but help with "why does this happen" (e.g. describe edge cases). – Tvde1 Feb 06 '20 at 11:33
  • 27
    Your example is one of the most valuable comments, imho. Just keep it. – cmaster - reinstate monica Feb 06 '20 at 13:21
  • 1
    It's even worse because now this object has multiple responsibilities (it starts the line renderer disabled to avoid artifacts on the first frame, plus whatever it usually does). Don't you need to create a `LineRendererDisabledStateStarterToAvoidArtifactsFirstFrame` class? – user253751 Feb 06 '20 at 15:04
  • 6
    Don't forget a `LineRendererDisabledStateStarterToAvoidArtifactsFirstFrameFactoryRegistry` so we don't have to hardcode how the `LineRendererDisabledStateStarterToAvoidArtifactsFirstFrame` is instantiated. – user253751 Feb 06 '20 at 15:05
  • your example sounds like it should be a "todo:" comment, imo. – Erin B Feb 06 '20 at 15:43
  • 1
    There are two very divide camps on comments for always verbosely commenting to only writing self-documenting code and every view in between. I just follow whatever my employer and the power structure prefer because I can go either way on the issue. – Mark Rogers Feb 06 '20 at 16:27
  • 5
    If you avoid comments in your code, I will avoid your code – donjuedo Feb 06 '20 at 21:13
  • Does this answer your question? ["Comment everything the right way" and "Instead of writing comments, write more readable code." - Both valid strategies?](https://softwareengineering.stackexchange.com/questions/335512/comment-everything-the-right-way-and-instead-of-writing-comments-write-more) – Graviton Feb 07 '20 at 01:15
  • With good, well written code, comments are usually redundant and unnecessary. But that doesn't mean that comment is bad. Redundant comment is bad for future maintainability, because they're lines that have to be kept updated, so remove them. Once you've removed all the redundant comments, what you're left with are comments that are helpful in understanding the code. These generally tend to fall into documentation, overview, high level architecture notes, and warnings/pitfalls. – Lie Ryan Feb 07 '20 at 03:30
  • 9
    If you consider comments to be bad because of the danger of them not being updated when the code changes, consider how much worse it is if comments are embedded in function names, the behavior of the function is then later changed, but its name not updated... – vsz Feb 07 '20 at 07:17
  • The summary added to the end of the question is excellent. – Spudley Feb 07 '20 at 13:35
  • 1
    Don't go with "_comments are almost always a bad idea_". That's an extreme, just like "_always write comments_". What works well is "_Comments are a failure to express yourself in code_". You just _can't_ always express yourself clearly in code. Comments are fine, but they're not something to be celebrated. That being said, if your method was just called `AvoidArtifactsFirstFrame()`, that seems like pretty clear communication to me. – R. Schmitz Feb 07 '20 at 13:37
  • 5
    Comments do not harm maintainability. What hurts maintainability is an idiot who changes the code and doesn’t change obsolete comments. – WGroleau Feb 07 '20 at 15:47
  • 2
    Reading this question I checked the date, expecting it's April 1. of last year :-) You should **not** tear code apart by extracting a one liner to some other function somewhere else. And you should **never** expect someone to find and look at a commit in order to find out details of why you did X at code place Y. – puck Feb 08 '20 at 17:34
  • No matter how well a piece of code is written, it cannot tell you the most important piece of information of all: What the code is _supposed_ to be doing. – EvilSnack Feb 08 '20 at 19:19
  • 1
    Who says “comments should be used sparingly”? Add the comments that I need to know. All of them. – gnasher729 Feb 13 '20 at 09:33

13 Answers13

190

and read that comments are almost always a bad idea for future maintainability

And now you are reading that the above is total and absolute BULL____.

Use comments. Put in as many as you think are necessary. A lot of people think that comments are used to just describe what the code is doing. Basically narrating the steps. That itself isn't usually very helpful. But comments can also describe the backstory about why the code is doing what its doing. And that can be critical.

Case in point - I was recently addressing a bug in a network streaming application. There was already a bug fix in place that took care of part of the problem. Needless to say I was very glad the previous developer who had worked on the problem had left copious comments in the code describing why it was doing things in its particular order, and why that should not be changed. "This should be done after X because...".

In your example, in my opinion, that comment is perfectly fine and should be left alone. It describes why the renderer is disabled so that future devs looking at the code won't think it is unnecessary and remove it.

Can that information be stored in the JIRA ticket or commit comments? Sure. It can also be stored right in the code that I'm looking at so I won't miss it.

Paul D. Waite
  • 1,164
  • 14
  • 18
GrandmasterB
  • 37,990
  • 7
  • 78
  • 131
  • 9
    Best of both worlds is a short comment describing the issue and the issue number with the full story. – Qwertie Feb 06 '20 at 00:19
  • 17
    @Qwertie That however is also worst of all things: it goes outdated very fast and the link between "where in the story" and "what comment refers to this" is quickly breaking. – paul23 Feb 06 '20 at 02:57
  • 10
    @paul23 That has not been my experience. I find that the ticket numbers have helped me the most on very old and obscure bits of code that no one can remember exactly what they were for and a comment with a ticket number takes me to the full discussion for how the feature should work and what the purpose was. – Qwertie Feb 06 '20 at 03:23
  • Well I work mostly on highly volatile and new code, where big chunks are often rewritten or rearranged... I guess that's the problem, there's some point where code matures and comments become invaluable; before that point they're highly misleading. The problem is: it's a gradual slope and hard to define when code is at that point. – paul23 Feb 06 '20 at 03:54
  • 2
    @paul23 Why is that a problem if you update comments at the same time that you update the code? – Jon Bentley Feb 06 '20 at 03:58
  • 1
    @JonBentley because we don't have time to always do that, or we miss the comment since it's above. Or we think it's still relevant while it is no longer or.... – paul23 Feb 06 '20 at 04:01
  • 2
    @paul23 I could see that being an issue if your code is littered with comments, but if you've used comments properly (concise, useful, and used only where necessary) then the amount of time it takes to review and/or change a comment should be a tiny fraction of the total time you spend typing, which itself is typically a small fraction of the total time you spend programming. For example if your comment says "This line is necessary because of X" and you go ahead and modify the line anyway, then it should be obvious to you that the comment needs updating, and doing so takes a few seconds. – Jon Bentley Feb 06 '20 at 04:09
  • 1
    I totally agree. I've learned over the years that good documentation, in contrast to good code, is often redundant. It's good practice to put the explanation and the backstory into code comments *and* into JIRA tickets. It must be available quickly, without too much redirection. I also believe that the typical `i++; // incrementing i` example, often used as the motivation for the "best practice" referred to by the OP, is nothing but a strawman; I've never in my life seen a comment like that (with the obvious exception of automically generated Javadoc/Doxygen stuff). – Christian Hackl Feb 06 '20 at 12:12
  • @ChristianHackl Usually a straw man, but I've seen `i++; // count down`. – StackOverthrow Feb 06 '20 at 17:19
  • 44
    Putting ticket numbers in source code works up until the point that the business decides to switch bug tracking systems. I can't even tell you how many times in my career I've come across comments saying "See bug XYZ" for a bug tracking system that had gone out of use years ago. Put the information in the comments - one less level of indirection for reader to deal with and no risk of the information disappearing. – 17 of 26 Feb 06 '20 at 17:20
  • 7
    @paul23 *because we don't have time to always do that...* Of course you do. You might as well say you don't have time to check the files back into version control, or run the compiler. It's part of the job. You can choose to do a half-assed job if you want, but it's disingenuous to complain that you've got half-assed results when you do a half-assed job. Of course you have. If you leave the comments out then the half-assed nature of the job may only be noticed when the code is found to be unmaintainable because no-one can figure it out, but it doesn't change that it's a half-assed job. – Graham Feb 06 '20 at 17:35
  • Having a comment that gives backstory is a good idea while dealing with messy code but I think the end goal should be cleaning it up, which will most likely obviate the comment. I can't imagine a scenario where the code would have to remain inherently unintuitive due to the problem being solved – Josh Johnson Feb 06 '20 at 20:01
  • 2
    Your comment about WHY vs HOW I think is really useful. Thanks so much I think it clarifies why this comment is ok where many other one-liners are not. – Adam B Feb 06 '20 at 20:07
  • @17of26: That's an interesting problem. Of course, a business shouldn't do that without archiving all the information in the system; it's like throwing away your entire Git history. However, it reinforces the point I made elsewhere, that documentation should normally be redundant. – Christian Hackl Feb 07 '20 at 06:32
  • 1
    The 3 Ws of comments: *What*, help a future reader understand a tricky piece of code that is not self-explanatory, because it's not always easy/feasible to reach it, should be the rarest of all, *Why*, help a future reader understand the reasons for the current code, and outlines when clean-up becomes possible (a reason disappeared/changed), *What else*, help a future reader understand the alternatives that were envisaged and didn't cut it, perhaps the most valuable of the Ws! *Note: I don't count "documentation" comments here, they are useful too!* – Matthieu M. Feb 08 '20 at 17:24
  • QWERTY: I don’t fix bugs because of a ticket, but because they are bugs. A ticket number is rarely useful. – gnasher729 Feb 13 '20 at 09:36
  • A ticket number is good - but not sufficient. Companies/projects change bug trackers and under circumstances where the old database is _not_ imported to the new database. It happens _often_. For that matter, it _also_ used to happen that SCM was changed and you frequently lost all history therefore all commit messages, and who-changed-what (blame). (That happens less now that git is ubiquitous.) Better is to provide a _unit test_ - and then make sure that unit test is commented with _reasons_ why the behavior needs to be the way it is. The unit test won't get lost, plus is operational. – davidbak Feb 14 '20 at 22:41
  • 1
    @JoshJohnson Easy example that I've done countless times: You have code that looks a bit odd because you're working around a bug in a third party API. "The documentation for API X says you must call Foo() before Bar(). However, we found that this causes a crash unless you call UnrelatedMethod() in between them." – 17 of 26 May 05 '20 at 14:37
78

You answered this yourself in the very first sentence of your question - comments are almost always a bad idea for maintainability. There are times when a quick note in the code is a good thing to include so that way a future developer can understand why something was done that way it was. This could prevent a change that introduces a problem.

Generally, I do prefer readable code and organized commits (with good commit messages) to comments in the code, simply because I've seen too many times where the comments aren't maintained as the code around them changes. But there are times when comments are appropriate. You may have just found a case where refactoring to a method is less clear than a one line comment. With fewer, more relevant comments, future developers will be more likely to read them and maintain them with the code.

Thomas Owens
  • 79,623
  • 18
  • 192
  • 283
  • 67
    *I've seen too many times where the comments aren't maintained as the code around them changes.* Comments are code though. If the comment weren't changed, the code was not competently changed. It's like seeing someone drive into a road sign and saying "let's get rid of road signs". No, the answer is not to allow incompetent people to demolition-derby your code, because road signs are a good thing, and if they're screwing this up then they're screwing up everything else too. – Graham Feb 06 '20 at 02:23
  • 48
    *Generally, I do prefer readable code and organized commits (with good commit messages) to comments in the code* **well, if you had to choose one or the other, sure...** but it seems like doing both is the programmer's *job*. Imagine a lawyer saying "I prefer filing motions on-time to showing up in court" um, it's literally your job to do both absolutely 100%. – Harper - Reinstate Monica Feb 06 '20 at 03:05
  • 3
    *"I do prefer readable code and organized commits to comments in the code, simply because I've seen too many times where the comments aren't maintained"* is equivalent to *"I do prefer comments in the code to readable code and organized commits simply because I've seen too many times where the code is not readable"* in the sense that they both make the mistake of thinking that there is a binary choice between readable code and maintained comments. As those above me have said, a good programmer should *do both*. – Jon Bentley Feb 06 '20 at 03:55
  • 3
    @Graham comments cannot be tested though. And to follow your road sign analogy: I was once taking a course on traffic management, and the police told me "well it's important you don't go against the flow, the road themselves without signs already let users feel who can go first etc". Code too shouldn't need comments. – paul23 Feb 06 '20 at 03:56
  • 22
    the tl;dr of this is that comments are good for the _why_, and not the _how_ – Baldrickk Feb 06 '20 at 10:29
  • 2
    @paul23 Much code is more practical to review than test too. And to continue the road sign analogy, do you believe the policeman was saying that there was no need for give way signs, stop signs, traffic lights, or directions to other places? For sure, in places where the code is utterly unambiguous, no comments are needed for those lines. It is not possible to write code where the entire program is unambiguously clear though, any more than it would be possible to drive from New York to LA just by "feeling who can go first". – Graham Feb 06 '20 at 10:50
  • 40
    @Baldrickk: Comments are even better for the *why not*. For example, `// not calling XYZ() here because of it causes too much RAM usage with production data`. You cannot simply replace that with a named function. – Christian Hackl Feb 06 '20 at 12:06
  • 1
    @Graham - This is why part of my policy on code reviews is to not only look at the diff. The actual code needs to be looked at. It should become obvious that comments were not updated with the code. – Logarr Feb 06 '20 at 21:03
  • 2
    I downvoted this for "almost always". I can agree that comments are _often_ bad for maintainability, especially when done instead of giving variables and functions good self-documenting names. But "almost always" is a stretch. – Monty Harder Feb 06 '20 at 23:08
  • @MontyHarder the scale of how often comments are the wrong way depends a bit on what you are used to. I've seen code bases where 100% of comments where utter rubbish, because they were outdated or just stated what the next line of code does ("//prints version number" over echo "$version" etc.). Given such code-bases, yes (almost) all comments that people came up with were just adding clutter or plain misleading and thus the code was better off without them. The cases where you need background information or a "why does this look like it's broken but isn't" should be low in most good projects. – Frank Hopkins Feb 07 '20 at 10:34
  • @ChristianHackl Are you sure? `XYZExceptWithProductionDataDueToRAMConstraints()` seems like a convenient name. – MechMK1 Feb 07 '20 at 12:56
  • @MechMK1 That works well if there's an `if` statement (although I'd probably prefer a shorter version like `AvoidExcessiveRamUsageInProduction()`). However, if an otherwise expected line _isn't there at all_ in the code, a comment makes more sense than adding a method that does nothing. – R. Schmitz Feb 07 '20 at 13:47
  • 1
    I personally prefer comments explaining something unusual over using weird method names. `if(this.Environment != Environment.Production) DoStuff(); //Don't call DoStuff() in the production environment because ...` is better than `DoStuffExceptInProduction()`. Even better would be if `DoStuff()` would raise a compile-time warning to not use this method in production, as it will cause issues. – MechMK1 Feb 07 '20 at 13:52
  • @MechMK1 Well, if you think it's "weird", it just means _you_ are not used to it. That will differ from person to person. **In general**, I would expect an English speaker with a programming background to not bat an eye at "_Avoid excessive RAM usage in production_". – R. Schmitz Feb 07 '20 at 14:08
  • 2
    @Graham "_Comments are code_" - Are they _really_, though? I guess this is up to different viewpoints, but if the application 100% does what it's supposed to do... I'm not gonna tell my manager that the code is broken because there's 2 outdated comments. – R. Schmitz Feb 07 '20 at 14:12
  • @R.Schmitz Yes, I'm not used to it, for better or for worse. But if I look at some popular and - by my own opinion - well-designed frameworks, I see methods like `DoThing()` or `DoComplexThing(withArg)` and less like `DoComplexThingUnlessArgIsNullThenThrowException(withArg)`. – MechMK1 Feb 07 '20 at 14:16
  • @MechMK1 I prefer `DoComplexThing()` without a comment `// Do complex thing unless arg is null then throw exception`. Much, much clearer and not repeating what is already in the code. – R. Schmitz Feb 07 '20 at 14:51
  • @R.Schmitz I generally agree, but it depends on the specific "exception case". If it's something the "average programmer" would intuitively grasp (e.g. supplying `null` may throw an exception), then a comment is optional. If it's something that's not obvious (e.g. beeps below 37 Hz throw an exception) then I would add some comment in a logical place explaining why that is the case (e.g. where the exception is thrown) – MechMK1 Feb 07 '20 at 15:04
  • @R.Schmitz Yes, really. Your application is also supposed to be reviewable and maintainable by another engineer, and that's what makes our job "software engineering" and not "randomly hacking stuff together". :) In that sense, it does *not* do what it's supposed to do. For a more functional issue, I might also not tell my manager that the code is "broken" because I missed checking memory allocation succeeded. (Hardly ever going to fail, after all.) But I'd track it (formally or informally) as something where the code is incorrect and should be fixed; and the same for the dodgy comment. – Graham Feb 07 '20 at 15:04
  • 5
    @MechMK1: Yes, I'm sure. `XYZExceptWithProductionDataDueToRAMConstraints()` is long and convoluted, and also wrong, because the point of the comment is that `XYZ` is *not* called at all, even though it would seem a logical choice there at first glance. Turning that simple comment into a function is a solution in search of a problem. – Christian Hackl Feb 07 '20 at 15:13
50

... read that comments are almost always a bad idea for future maintainability.

It's great that you found that. You now know one "reference source" which is written by someone who is utterly incompetent and whose opinion is less than worthless. They're not only wrong, they're actively damaging code for people like yourself who believe them. Or possibly this wasn't what they were trying to say, and you've misunderstood them and gone off on a wild goose chase.

Comments certainly should not try to explain what a line of code does (unless it's genuinely complex and obscure, perhaps). A coder can be expected to read "++i;" and not need a comment saying "increment counter". Function names too should say what the function does, without a lot of extra comments needed.

But comments should explain why a line of code does what it does, or any other details which can't be seen from the code itself. If the comment above "++i;" said "it is now safe to increment counter after interrupts have completed", that's a good comment. If the function has a comment saying what the units are for its return value, that's a good comment. It would certainly be possible to write a Word document to explain all these details instead; but capturing them in comments alongside the code is a key part of making code self-documenting. It's much easier for a separate Word document to get out of step, after all.

You say that "refactoring/renaming has made these comments be out-of-date or in the wrong spot". Then almost by definition, you haven't really refactored, you've just swung a very unmethodical axe at the code without understanding it. Refactoring needs the code to be properly understood - and that means you need to have read and understood the comments too. And if you read and understood the comments, then you knew to move them or update them when you swung the axe. The fact you're finding these misplaced comments is a strong clue that your refactoring was not very effective.

Based on the examples you gave there, I can pretty much guarantee you've ruined your code. Never mind - you can always roll back to the previous version in your version control system. (You do version-control your code, don't you? If not, start today.)

It's not the end of the world. Learn from this mistake and move on.

Graham
  • 1,996
  • 1
  • 12
  • 11
  • 5
    I haven't read such a strong and negative opinion on uncle bobs book clean code in a while... I wonder did you read it? – findusl Feb 06 '20 at 09:28
  • 25
    @findusl I've read Uncle Bob. In principle I agree with him. His primary point is that comments shouldn't repeat what the code already says, and that is absolutely right. He does lean on it too hard though, and the message that tends to get through (as the OP has misunderstood it) is just "comments bad". In that, I think the way he's put it across is actively harmful and damaging to good software engineering, yes. He's coming to it from a PoV of improving practise for existing skilled coders who can tell when they're needed, and this is too easily misunderstood by newbies who can't. – Graham Feb 06 '20 at 10:59
  • 14
    @findusl ... His summary on comments is "Use as explanation of intent; Use as clarification of code; Use as warning of consequences." and that's absolutely right. But that gets masked by statements like "every comment represents a failure to make the code self explanatory". I would like to say that last statement is [not even wrong](https://en.wikipedia.org/wiki/Not_even_wrong), but really it's [wronger than wrong](https://en.wikipedia.org/wiki/Wronger_than_wrong) because he knows better. – Graham Feb 06 '20 at 11:07
  • 11
    @findusl ... [This blog post](https://blog.cleancoder.com/uncle-bob/2017/02/23/NecessaryComments.html) for example shows him giving an example of how they're needed, because the code cannot capture it. If you asked him if he thought that piece of code was a failure, of course he'd say it wasn't. But when he says "every comment represents a failure", that's exactly what he's saying about other people's code. That is clearly, demonstrably, wrong by his own admission, and Robert Martin in my eyes is a less competent authority for making that statement. – Graham Feb 06 '20 at 11:09
  • 4
    I think you missunderstood his point. Its not the code or comment that is the failure. It is the one writing it. I prefer this quote from his book: "Every time you write a comment, you should grimace and feel the failure of your ability of expression." And I am pretty sure when he wrote the code in the blog post you referenced, he grimaced every time and felt his own failure to do better. The code is not the failure, the code is the best he could do here. It's his own failure that he cannot do better. – findusl Feb 06 '20 at 11:48
  • 4
    Basically imho what he is saying, if you have the choice to make clean code with or without comments, do it without comments. If you do not have that choice and you need to use comments, grimace, feel your failure and then write the comments. – findusl Feb 06 '20 at 11:51
  • 10
    @findusl: You should be wary of anyone who spreads dogmas about programming as if they were absolute truths, especially when it's apparently those people's business model to publish a lot of books which seem to attract readers with their promise to reduce an exceedingly complex and heterogeneous engineering discipline like software development into a set of easily observable commandments. – Christian Hackl Feb 06 '20 at 12:23
  • 17
    @findusl That's precisely the attitude I disagree with. Back in the day, "real programmers" wrote code without comments, and no-one could understand what the hell was going on. There was significant work done to put software engineering on a professional level, and one part of that was to get engineers to add comments which embedded documentation into the code. Using comments to explain something which code cannot explain is ***NEVER*** a failure on any level. In suggesting that it is, I believe he is actively damaging the profession of software engineering and the skills of his students. – Graham Feb 06 '20 at 12:36
  • 5
    Funnily enough, when I read "Clean Code", it sounded to me like "avoid pointless description of the what, rather comment on as high a level as possible, explaining the why and the why-not". Of course, that's what passed my own mental filter. That said, I also had the feeling that some of Uncle Bob's stances were a bit too sweeping. Especially when it comes to function lengths. His ideal of single line functions turns spaghetti code into rice code, imho. I prefer more middle-ground solutions. Ravioli code, so to speak. – cmaster - reinstate monica Feb 06 '20 at 13:36
  • 3
    I must confess, that I dislike the harsh wording in the first paragraph, though. I cannot upvote an answer that harsh. – cmaster - reinstate monica Feb 06 '20 at 13:40
  • 3
    @cmaster-reinstatemonica The harshness accurately reflects the reality with which we all must contend. – StackOverthrow Feb 06 '20 at 17:18
  • 1
    @cmaster-reinstatemonica That's a whole nother subject, yes. Refactoring into smaller functions needs to add value in maintainability, and lots of tiny functions can destroy your overall view of the system behaviour. Especially (as in the OP's example) if you're using it as an alternative to a simple and clear comment describing what the code is doing on that like, it tends to screw with cohesion. As you say, smaller quantities of ravioli are preferable. :) – Graham Feb 06 '20 at 17:21
  • 3
    @cmaster-reinstatemonica I agree that my wording is harsh there. To my mind though, it's only as harsh as it deserves. When the end result is that someone's code has been left in a mess because they've tried to follow some misconceived and largely-discredited rules in good faith, the best advice I could possibly give would be "don't believe them again". Fool me once, etc.. I wasn't specifically thinking of Uncle Bob's rules there TBH - he's just one example. – Graham Feb 06 '20 at 17:27
  • 2
    @Graham If given the choice of no comments and well structure code or bad structured code with comments that try to explain why it's so badly structured or don't even add anything, I take the well structured code any day. That's exactly what such guidelines are meant for, they're not saying 'comments bad' they're saying, 'in this case, instead of comments, do this to improve the code and make them unnecessary' and given they cover most cases where one often sees comments, indeed most comments in existence (in most projects, particular student projects) are unnecessary. – Frank Hopkins Feb 07 '20 at 17:08
  • 1
    @Graham The thing with guidelines is, once you understood them and start to question them on solid grounds, you start to becoming good enough to make up your own mind, but they give you... well... guidelines to go one direction until you figure that you might wanna turn the other way. – Frank Hopkins Feb 07 '20 at 17:10
  • 4
    @FrankHopkins That's a false choice though. Those guidelines don't magically produce well-structured code out of thin air. All you get is badly-structured code, with badly-named fragmented single-line functions, with no comments to help you understand the author's intent. Following those guidelines literally gives you the worst possible outcome. The Cult of Uncle Bob isn't the first magic-bullet craze (Jackson Structured Design, anyone?) and it won't be the last. But it's as wrong-headed as any of the others. – Graham Feb 07 '20 at 17:46
  • 2
    @FrankHopkinsThe ideal is well-structured code with meaningful comments. – StackOverthrow Feb 07 '20 at 17:49
  • 1
    @user560822 Yes, just that 90% of the comments you see in bad code aren't needed when you have well structured code. That's what the guideline says, if you need a comment think twice whether you need it or whether you can restructure, and that makes perfect sense, it's just people overthink those and sure someone will misuse any guideline. – Frank Hopkins Feb 08 '20 at 00:10
  • 2
    @Graham not read and applied in some crazy overdrive mode the guidelines make perfect sense - even if the code is bad, having no comments is most of the time saving me time, as that means I don't run the risk of getting more confused by faulty comments. And sure, in the balancing / direction such guidelines point is always a grain of an historical component. First coders didn't write any comments to save bits then people told them to always write comments. So many took that the wrong way and wrote garbage comments, hence a more nuanced approach leaning a bit against the tide goes the other way – Frank Hopkins Feb 08 '20 at 00:14
  • 2
    @Graham it's interesting though that some people see a bone-headed cult were most people that bring up Bob's guidelines see them rather nuanced - as they are meant. Btw. they come in the context of "make your code readable", where comments are one tool, that is just not the preferred tool, a valid tool, but most of the time unnecessary (most of the time -> imho 90+% of code don't need a comment). – Frank Hopkins Feb 08 '20 at 00:18
  • 2
    @FrankHopkins If you're starting with badly designed code, neither the function names nor the comments will be worth a damn. In practise, function and variable names are very limited in how long and descriptive you can make them. For any function which has more than trivial functionality, the function and argument names are no substitute for a Doxygen comment which also lists units for function arguments and return value, possible exceptions that can be thrown, whether the function is reentrant, whether it assumes some setup has been done first, etc.. – Graham Feb 08 '20 at 01:54
  • 3
    @FrankHopkins The thing is, if Bob's guidelines were nuanced, I wouldn't mind. Like I said when findusl first mentioned him, I agree in principle. My issue is that he *doesn't* have nuance in his guidelines. Describing comments as a "failure of expression" is an uncompromising, extremist viewpoint. – Graham Feb 08 '20 at 01:59
  • 1
    @Graham Did you really just talk about *nuance* while writing in the second sentence of your answer : « Utterly incompetent » and « Opinion is less than worthless » ? Nuance feels obscure for some, it’s only a style of writing. Doesn’t change the fact that any information from any source should be analysed and questioned. – Steve Chamaillard Feb 14 '20 at 22:27
  • @SteveChamaillard That was the word I used, yes. Anyone who believes code should not contain comments is flat wrong. Not just according to me, but also according to organisations such as MISRA and NASA whose business is writing maintainable code. Nuance is where two people disagree only on the relative importance of things, and you can agree that you both have valid points. If one person says, "nope, the sky is definitely pink and black polka-dots and full of leprechauns", you don't have a basis for agreement, because it's trivial to show they're flat wrong. – Graham Feb 15 '20 at 01:08
  • @Graham There is the extreme "_no comments_", there is the extreme "_comments all the time_". Uncle Bob has a nuanced stance with "_comments only if you're unable to make your code clear_". But I think your point is more that people who don't understand the if statement in that sentence, well, they don't even get if statements, so they better comment their code? – R. Schmitz Feb 16 '20 at 16:22
  • @RSchmitz My point is that the "if" in Uncle Bob is more like a full-colour full-page ad saying "no comments ever" with a 4-point disclaimer at the bottom. The main message is extreme; and the "if" is there, but it's hidden away in stuff designed to not make it visible. And to my mind, like I said, that makes the extreme part worse because he clearly *knows* that what he's teaching doesn't work in the real world. – Graham Feb 16 '20 at 17:17
  • @Graham I don't know if shouting propaganda will help your case here. Let's stay with facts, OK? **Clean Code, Chapter 4: Comments, first 2 sentences**: _"Nothing can be quite so helpful as a well-placed comment. Nothing can clutter up a module more than frivolous dogmatic comments."_ Further: _"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."_ (bold emphasis mine) – R. Schmitz Mar 06 '20 at 12:42
16

I hope that the sentence you are referring to comments are almost always a bad idea taken out of context has lost a long list of caveats and exceptions, otherwise the first advice I would give you is to find another guru.

As for that comment, that is shown in the question, the best thing to do is to leave it as it is.

FluidCode
  • 709
  • 3
  • 10
11

I agree with many of the other answers: comments can be a very good thing, if used well.  But here's a slightly different way of looking at it:

“Should I write comments?” is the wrong question.

The right question is: “How can I make this code clear?”

You want to make it as easy as possible for whoever reads the code in future.  (That may be other members of your team; it may well be you, in months or years when you've forgotten it all!)  You want them to know, without puzzling over lots of code or looking through documentation, what the code does, and why it does it.  You don't want them to make understandable mistakes, or hunt through blind alleys.  You want them to see instantly how the code works, how it fits into the bigger picture, where bugs might lurk or improvements might be made.

If you can do that in the code itself, then great!  Judicious selection of variable/property names, function names, blank lines, organisation into classes and functions, consistency, single responsibility, DRY, &c are all great ways to explain what you're doing and make it as easy as possible to read and maintain the code in future.

But that's not usually sufficient.  (I'm certainly not suggesting 50+-character names, such as in the question, which are more likely to make an unreadable mess of things…)  So:

Put anything left over into comments.  Comments are for additional information, all the things that won't fit neatly into names or structure, things that aren't clear to anyone glancing at that file, things that are important to know, hard-won knowledge that you want to preserve.  Far better to put that in comments than lose it.

A lot can be said about how and where to write comments; for example, explanations of the big picture are often better put at the top of the class or function rather than tucked away.  But this isn't the place for those details.  The most important thing is that you try to make your code as clear as possible; comments aren't the only tool for that, nor the first one you should reach for, but they're still important and valuable.

gidds
  • 789
  • 3
  • 8
8

Among these other good answers, I would suggest that the right way to express this requirement is with one or more unit tests:

LineRendererShouldStartDisabledToAvoidArtifactsFirstFrame()

LineRendererFirstFrameShouldBeFreeFromArtifacts()

et cetera

...with your IDE being helpful enough to show the connection between your line of code and the related unit tests.

Roger
  • 287
  • 1
  • 3
  • 1
    ah.. this is probably the best solution, unfortunately my project doesn't do unit testing :( one more case to add to the list to try to convince them! – Adam B Feb 05 '20 at 19:01
  • 24
    @AdamB: even if you will do unit testing, creating a test which will fail if a renderer produces artifacts will become challenging. Sorry, but just because you have a hammer (unit tests), not every problem is a nail. For this specific case, I am pretty sure the best solution *is* a comment. – Doc Brown Feb 05 '20 at 20:17
  • Couldn’t you just write a unit test to make sure it starts turned off “because of artifacts” and not necessarily test the artifacts Themselves? – Adam B Feb 05 '20 at 20:52
  • 4
    @AdamB: then you would have to comment on the test why it is necessary, since the test itself would not be self-explaining in the same way the one-line above isn't. Sorry, but that is a non-solution. – Doc Brown Feb 05 '20 at 22:26
  • Hmm that’s true. I’m convinced. – Adam B Feb 05 '20 at 22:34
  • 1
    @DocBrown Any decent unit test framework will have a way of describing the test (or its failures) and what it hopes to accomplish. The benefit over a simple comment is that it's systematic and will actually break when the code is changed: it doesn't rely on the programmer paying attention and the comment being up to date. Adding a comment is absolutely fine, but calling a unit test a non-solution seems absurd to me. – Celos Feb 06 '20 at 08:30
  • 6
    So the workflow is: 1.) look at code, find strange/unnecessary statement; 2.) remove the statement; 3.) fire up unit-test (perhaps some hours later); 4.) read documentation for failing test that I never saw before 5.) put the strange statement back in. – piet.t Feb 06 '20 at 13:57
  • 1
    The ideal solution is to add a unit test (if possible) *and* leave the comment where it is. – 17 of 26 Feb 06 '20 at 17:48
  • Writing a unit test to document this requirement is good. Another alternative you can do, when writing unit test is too hard is to use assertion. `assert` statement should test that the condition are you expected are what they are, and the assert message should contain the comment that you otherwise would have left. If the comment becomes outdated, the assertion would have failed where regular comments would mislead other people. – Lie Ryan Feb 07 '20 at 03:41
  • 1
    *"...my project doesn't do unit testing"* Yeah, I can see how you might think that removing every comment from your source files would solve all your problems. – A. I. Breveleri Feb 07 '20 at 13:55
  • I'd use unit tests in a slightly different way. If you've written some weird looking code to work around a bug, try to write a unit test that triggers the bug and fails if the bug does *not* still exist. Put a comment in the unit test that says, "If this test fails, you can remove the stupid workaround from FooBar.cs." – StackOverthrow Feb 07 '20 at 17:53
5

The best comment explains something that the code does not express: why it is not done a more obvious or standard way

Starting off with a flag disabled is not unusual or non-standard. Inside of implementation, we use flags and flows and checks. That's normal

As long as the function that uses this code is well-named and does one and only one thing (which encourages it to be short), the assignment does not need documentation.

The function itself (because it changes an external value) might do well to note that it modifies external state. Or the caller should modify the state before calling the functions that depend on it. This allows the benefits of functional programming, even if it is not enforced

Cheers

  • 1
    Bingo. The most interesting information about engineering decisions isn't usually what's good about them, but what was recognized as being bad about the alternatives. A comment like "Doing XX instead would seem like it should be a lot faster and easier, but ends up producing unworkable corner cases that can't be handled reliably without making things needlessly slow and complicated" might save a future maintainer days of wasted effort. – supercat Feb 06 '20 at 16:58
  • 1
    @supercat And don't forget that even without a change of roles, the next time you read this code, months or even years in the future, _you_ will be that "future maintainer". – Monty Harder Feb 06 '20 at 23:16
  • 1
    @MontyHarder: Been there done that. >:*3 Though I should mention another kind of useful comment would be "X might be better, but it wasn't explored, because this worked well enough". If it turns out the current code *doesn't* work well enough, knowing that an alternative hadn't been explored would avoid any unwarranted assumptions that it had. – supercat Feb 06 '20 at 23:38
3

A rule of thumb for me is to comment on code that looks wrong.

It's very easy for another developer, or yourself in the future, to look at such code and think, "Obviously, this should be changed." If you already tried that, and found a hidden problem, leave a comment.

This might be due to a bug in a third-party library (including a reference to your bug report might help remove the work-around when it's fixed). The change might lead to a rabbit hole that you couldn't see your way out of; giving someone else an overview of the hazards ahead could help them see things from a new angle.

It sounds like the example cited in the question is a good example.

erickson
  • 152
  • 5
2

A "why" comment is one of the better uses of comments, and is certainly better than a "why" function name. However, the "why" in your particular example is temporal coupling, and that is definitely worth seriously trying to fix. Temporal coupling in a nutshell is when you have dependencies in time of what order things in a module must be called or set.

How to fix temporal coupling is a relatively difficult problem. Maybe having lineRenderer check internally if it is the first frame. Maybe having separate logic overall for the first frame, so you can put all initialization concerns in the same place. Maybe making it structurally impossible to construct a lineRenderer until the first frame has completed. Basically, making the code impossible to write with lineRenderer enabled incorrectly. It's really difficult to make a specific recommendation without a lot more context.

I like how Martin Fowler puts it in his Refactoring book. Comments are like deodorant. Not a bad smell itself, but usually fixing a bad smell. In other words, the comment is a good thing, but often if you look you can find a better thing.

Karl Bielefeldt
  • 146,727
  • 38
  • 279
  • 479
2

tl;dr Comments themselves can be good! However, comments shouldn't be relied on to enforce program structure when the code could do this more reliably.


It's unnecessarily relying on comments as a substitute for program structure that can be bad.

read that comments are almost always a bad idea for future maintainability.

They probably meant that relying on comments being read-and-understood is a bad idea for future maintainability.


Example: Ensuring that a lineRenderer is re-enabled properly.

For example, adapted from the question statement:

/// Needs to start disabled to avoid artifacts in the first frame.
/// Must be re-enabled after the frame completes.
lineRenderer.enabled = false;

This is useful information, but relying on maintainers to always keep track of miscellaneous requirements like this without ever dropping the ball would seem hazard-prone.

The comment isn't the problem, but rather a symptom – someone put it there because they foresaw a potential problem and they're hoping that the comment prevents it from happening.

So the comment's good, just relying on it alone, rather than creating a logical structure, would be a bad idea for future maintainability.

For example, in C#, we might do something like this:

/// We assume that the line renderer is currently enabled.
/// It must be disabled for the first frame to avoid artifacts.
/// It must be re-enabled after the first frame.

if (!lineRenderer.Enabled)
{
    throw new Exception(
        "Error:  The line renderer was already disabled,"
        + " in contradiction to the assumption that it should"
        + " have been enabled at this point."
    );
}

try
{
    lineRenderer.Enabled = false;
    this.RenderFirstFrame();
}
finally
{
    lineRenderer.Enabled = true;
}

The initial check ensures that lineRenderer is enabled, as our logic assumes this to be true.

The try{}finally{} ensures that lineRenderer is re-enabled even if it throws, assuming that the thrown Exception is caught. This way, if a thrown Exception is caught higher up the call-stack, the catcher isn't responsible stuff like re-enabling lineRenderer.


Conclusion

In short, the point's that comments themselves aren't bad; it's relying on comments to enforce program logic that can be an issue for future maintainability.

Deduplicator
  • 8,591
  • 5
  • 31
  • 50
Nat
  • 1,063
  • 1
  • 8
  • 11
1

Think about comments as one possible form of documentation. Arguably, they're the preferred form because they're immediately accessible when you're looking at the code. Some developers say code should be self-documenting, but I contend that self-documenting code is impossible. To understand why, ponder this:

Without documentation, there can be no such thing as a bug, because there is nothing to define correct behavior.

Sometimes the correct behavior isn't formally documented. Often it's just an assumption. If you encounter a method named Multiply that performs subtraction, it's pretty safe to assume that something is wrong. But if there's no documentation, you're going to have to dig through the program to determine whether you should rename the method or change what it does. If the only documentation is the method name, and some callers are assuming it does what it says, but others are using it based on what it actually does, where's the bug? If there were any documentation at all, even a comment, you could say authoritatively what the intent was and which code needs to be changed. Now imagine what this process would look like with a far less obvious and contrived example.

If there's no documentation, all behavior is based on assumptions. Sure, a method name can be documentation, if it fully describes every precondition, postcondition, and side effect. Not very likely! When you're writing new code, be conscious of the assumptions you're making about what constitutes correct behavior. Try to imagine how obvious those assumptions are going to be to the person who reads it. If there's any doubt, write a comment.

StackOverthrow
  • 646
  • 3
  • 13
  • 1
    Whilst this doesn't directly answer OP, this is really good advice, and for me the number 1 reason to write code doc comments for _every_ function and property. In fact, I encourage my devs to write the comment first, with an empty prototype, then unit tests to enforce the logic explained in the comment, then the function body itself. At all review points, we accept the comment as the gospel expressing the original intent, from outside the function, the comment and the prototype are all we have to go on. As for in-line comments, the same rules apply, comment should express intent and why. – Chris Schaller Feb 16 '20 at 12:13
0

Just a suggestion I haven't seen here yet. I usually write my initial code without many comments, but every time I have a reason to revisit/re-read code (I usually do a few times when I'm initially writing it) if I come across any code that's hard to understand I either refactor it, add comments or improve comments--whatever it takes to ensure that next time I won't have to stop here to understand it. Writing code is like any creative process, you should have a draft, make some tests, make it work, clean it up, make it understandable, etc. Each of these activities is going back and re-visiting what you've already done. It's iterative and part of that iterative process should be to make sure it's as easy as possible to figure out.

One issue to consider though--most programmers don't trust comments enough to even read them. If you comment well and pay attention you'll find that you get asked questions that are already clearly answered in the comments. I have no idea how to address this, so I write comments for myself and when they ask me questions about my code, even though I won't remember what I did, I trust the answer will be right there in a comment and I can look it up and answer them.

Bill K
  • 2,699
  • 18
  • 18
0

As many have said, comments that explain why rather than what are generally a good idea. Your example is a perfect example of a comment that explains why an non-obvious bit of code exists.

If you have this exact line of code, and a copy of the comment, in multiple places, then it would make sense to make a one-liner function in order to avoid repetition.

If this one line isn't at the same conceptual level as the surrounding code, then you might also want to isolate it. For example, if the statements before and after you flip this flag are very high-level concepts, then the need to explain it with a comment might be a symptom of working at the wrong level of abstraction.

Consider this hypothetical context:

CommitRenderingContext(blah, blah, blah);
PrecalculateLookUpTable(parameters);

// Needs to start disabled to avoid artifacts the first frame. Will enable after frame complete.
lineRenderer.enabled = false;

lineRenderer.RenderFrames(frame_count);

The fact that the one-liner is the only statement in the vicinity that needs explanation is a clue that it's out of place in the context of the higher level work going on around it. Presumably, the lineRenderer will be enabled inside a loop hidden index RenderFrames, which means that the two places that twiddle this flag are not only separated by distance, but by conceptual level as well. In that case, it probably belongs inside the RenderFrames method, and the comment would likely be very appropriate there.

If, on the other hand, the context is something like:

lineRenderer.color = user_selected_color;

// We use half the desired thickness because we're going to do two passes.
lineRenderer.thickness = user_selected_thickness / 2;

// Needs to start disabled to avoid artifacts the first frame. Will enable after frame complete.
lineRenderer.enabled = false;

for (int i = 0; i < frame_count; ++i) {
    ... detailed calculations ...
    if (lineRenderer.enabled) {
        ... do something with lineRenderer ...
    } else {
        lineRenderer.enabled = true;
    }
}

Then I wouldn't consider the comment out of place.

Adrian McCarthy
  • 981
  • 5
  • 8