質問

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.
役に立ちましたか?

解決

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.

他のヒント

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.

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

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.

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.

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.

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

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.

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.

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.

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.

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.

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.

ライセンス: CC-BY-SA帰属
所属していません softwareengineering.stackexchange
scroll top