65

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.
  • 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 5 at 17:30
  • 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 6 at 2:44
  • 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 6 at 3:02
  • 16
    Your "name" is just a comment with the spaces removed. – jamesqf Feb 6 at 3:27
  • 27
    Your example is one of the most valuable comments, imho. Just keep it. – cmaster - reinstate monica Feb 6 at 13:21

13 Answers 13

77

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.

  • 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 6 at 2: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 6 at 3:05
  • 22
    the tl;dr of this is that comments are good for the why, and not the how – Baldrickk Feb 6 at 10:29
  • 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 6 at 12:06
  • 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 7 at 15:13
188

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.

  • 9
    Best of both worlds is a short comment describing the issue and the issue number with the full story. – Qwertie Feb 6 at 0: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 6 at 2: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 6 at 3:23
  • 43
    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 6 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 6 at 17:35
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.

  • 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 6 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, but really it's wronger than wrong because he knows better. – Graham Feb 6 at 11:07
  • 11
    @findusl ... This blog post 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 6 at 11:09
  • 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 6 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 6 at 12:36
14

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.

9

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.

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.

  • 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 5 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 5 at 20:17
  • 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 5 at 22:26
  • 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 6 at 8: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 6 at 13:57
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 6 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 6 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 6 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.

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.

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.

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.

  • 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 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.

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.

Not the answer you're looking for? Browse other questions tagged or ask your own question.