Comments inside the body of a function should apply to the state of the system at the point the comment “executes”.

Here’s an example of poor comment placement:

// Widget is already vibrating, so we update the waveform in place.
// Else the waveform parameters will be set when we start vibrating.
if (waveformParameters != null) {
waveformParameters.Shape = WaveformShape.Square;
widget.UpdateWaveformParameters(waveformParameters);
}

When I encountered this comment, I read it as telling me that the widget is already vibrating. I’m thinking, “How do I know that it is vibrating? Shouldn’t we be checking for that first?”

And then I see the “else” part of the comment, and I get more confused, because why are we talking about what we do if the widget is not vibrating, if the previous sentence told us that we (somehow) already know that that it is vibrating?

Next, I see the if statement, and now it’s checking whether something is null, which I guess tells us whether the widget is vibrating. But the first sentence of the comment said that we knew that it was vibrating.

Oh, I see. The comment is really describing what we know to be true once we are inside the if block.

Here’s a less confusing way of writing the comment.

if (waveformParameters != null) {
// Widget is already vibrating, so we update the waveform in place.
waveformParameters.Shape = WaveformShape.Square;
widget.UpdateWaveformParameters(waveformParameters);
} else {
// Nothing to update right now. We will set the parameters
// the next time we start vibrating.
}

Each comment describes what happens when execution reaches the block of code it is in. I even created a dummy else block to hold the explanatory comment about why it’s okay to do nothing.

If you really want to put the comment prior to the “if” statement, you need to structure it to match the state of the program prior to the “if” statement.

// If the widget is already vibrating, then update the waveform in place.
// Else the waveform parameters will be set when we start vibrating.
if (waveformParameters != null) {
waveformParameters.Shape = WaveformShape.Square;
widget.UpdateWaveformParameters(waveformParameters);
}

The post Code comments should apply to the state of the system at the point the comment “executes” appeared first on The Old New Thing.

  • MotoAsh@piefed.social
    link
    fedilink
    English
    arrow-up
    1
    ·
    edit-2
    21 hours ago

    Ohh executing examples and whatnot in the comments/docs is a good idea. I know a few frameworks/doc tools try that at least on a component level, but of course when you involve whole extra tools, it’s sometimes a big learning curve cost even if the boilerplate/setup is trivial. That would be neat to have functional comment examples and formal unit tests at a language level.

    I think I tend to agree on bad comments. There has definitely been a few memorable occasions where I’ve axed multiple paragraphs of comments simply because they were old and a touch nonsensical after years of updates.

    Yea I cheated a bit by bringing in API documentation, but then in my defense a lot of that I also write, or at least write the formal comments that end up compiled in to those docs. lol It’s also still a bit true when digging in to libraries to contribute, though. If the inline comments suck, most will probably not contribute unless it really legible code. lol

    To me, there is almost always something worth documenting about a function, unless it’s boilerplate or just obvious data handling, of course. Though even then, if it’s at an app or library boundary, there is almost certainly some ‘why’ and limitations/etc to describe. After all, even basic CRUD at an API/app boundary will have multiple known modes of failure that have to be described somewhere.