Hacker News new | ask | show | jobs
by Karrot_Kream 2729 days ago
There's a lot of talk about comments becoming stale and code being self documenting in the replies which makes me wonder: do people genuinely not read comments and just made code changes without updating comments? And do reviewers not look at the context of the surrounding code and just let commits in? What's the point of having code reviews then?
4 comments

It's very easy to miss comments, since they aren't type-checked, nor are they unit-testable.

Comments don't need to be near the code they affect. They don't even need to be in the same file - consider if you've organized your code by features, but the comment relates to a layer than spans multiple features.

I once spent two hours on figuring out why a log file wasn't being modified when there was an error. I knew the location of the file but it just wasn't showing that error.

Eventually I tracked down this line:

    // writes to the log file at c:\...\xyz.log
    AppendToLog(message);
Yeah, that was the correct path and everything, and yet the line wasn't executing!

Eventually I looked inside the AppendToLog method. It was writing to another file in a completely different path :)

That was when I stopped bothering to read comments. They always lie, and I couldn't even blame the programmer who changed the AppendToLog method -- the comment wasn't inside the method, it was on a call. I can't honestly expect someone who changes a method to look for all the places where that method is called and make sure any existing comments match the change.

Can't you modify the comment to improve overall hygiene instead of emerging with code nihilism?
By all means, feel free to improve that comment :)
Comments straddle this weird line between code and human process. They're in the source code files themselves, and there is an appeal to trying to evaluate a project by looking at the source code alone and trying to gauge abstract technical merit. But I think the real truth here is that comments are a tool in the service of a development process, which includes things like having code reviews, having code reviewers be sufficiently motivated (intrinsically or extrinsically) to care in useful ways and neither nitpicking nor rubber-stamping, having motivated people on the project in the first place, having shared values about what code you're going to write and what code you're not, having tests and doing the operational work to keep tests running, retaining people on the team, etc.
I'd say it's mostly a meme. If a code review doesn't involve reviewing comments around the modified code, it's a bug in the process.

Elsewhere in this thread Ousterhout's book is mentioned; I like his advice about always placing comments in the most obvious places and as close to the code they affect as possible. This way, you can't miss them, and and it's hard to forget to update them.

Everyone can have comment blindness to some extent, but I've worked with two people who auto-collapsed docstrings and didn't read and hence update comments, which is enough (one person writing code without updating comments/docstrings and one person inadequately reviewing). Sure, the problem only appears in a bit of the code, but it means people stop trusting all the comments.
> auto-collapsed docstrings

Woah, that sounds like a pretty dumb feature. Auto-collapsing whole functions is useful, but auto-collapsing docstrings sounds like a recipe for disaster. People write docstrings and inline comments for a reason.

No. Autocollapsing doc strings is not dumb at all, it’s an amazing feature. Most of the time in my experience the doc strings are completely useless when you are writing code, they may be useful only for the caller because they give you info in the autocompletion (and most of the time is just “get this value” “set this value”) and they can be used to automatically generate api docs. If they are extensively used in a private project that no one will call from a different one I will auto collapse them. They take so much space for nothing and they slow me down terribly. And the projects in which I have seen this behaviour had horrible methods naming, clueless architecture and completely arbitrary method subdivision. The doc strings where just the wrong solution for the wrong problem.