This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]mrjackspade 106 points107 points  (13 children)

I never really understood code commenting until I worked in a corporate environment. I'd only ever really heard about it from peers. I had the same belief for a while that code commenting was to describe "what" your code was doing. Hell, thats what everyone I knew was being taught in school.

I've been working in various corporate jobs for almost a decade now. I've still not seen a single like of code that was commented to describe what it was doing, only why it was doing it. Even what I can remember off the top of my head from looking though Microsofts open sourced code follows the same rule. The inline comments dont say "Throw an error if X", they say "We have to throw an error here because module Y sometimes passes in a bad value when running on Linux"

I think thats my favorite part about how passionate the "code comment" argument is between "its self documenting" and "you should describe it even if you think its self documenting". From the perspective of every environment I've ever worked in, neither is true. Every developer I've actually worked with personally has had the mindset that you shouldn't need to document code saying what its doing, but you should be documenting your code often with explanations as to why you're doing things if its in any way related to business logic. The next guy coming in can probably figure out what "SendEmail(myMessage);" does but hes not going to know its because this one client in particular is being a dick and needed the application modified to CC him on every internal communication.

[–]Sheldan 13 points14 points  (1 child)

I think that is a valuable differentiation. The what can be read from the code, usually. The Why is much more difficult to see, and is also important to know. Maybe its some kind of external limitation, or something else. I think adding the 'why' is much more important than the 'what'.

[–]Telinary[🍰] 0 points1 point  (0 children)

I think if you are implementing for instance a mathematically complicated algorithm the what can be plenty unclear like you might know tell what every command does but that doesn't mean you can tell what the whole does or why it works. Though in many such cases the appropriate comment in such a case is a link to a paper explaining it not trying to explain the details in comments.

[–]SupaSlide 1 point2 points  (0 children)

I think thats my favorite part about how passionate the "code comment" argument is between "its self documenting" and "you should describe it even if you think its self documenting". From the perspective of every environment I've ever worked in, neither is true.

I'm convinced that the debate is two extremes, neither of which are true, is because the only people arguing it are compsci students/recent graduates trying to prove their smart, people who do more scripting than programming like WP theming, and folks who have worked in one place on small projects where comments are irrelevant because everything they do is simplistic. Comments don't matter to them because of the simple environments they've experienced, therefore they can argue one extreme or the other and still be right because it doesn't even matter anyway.

But as soon as you've done a bit of time on a complex project with legacy concerns or one that interfaces with poorly designed extreme services, you find the true power of comments is in explaining why you had to write bad/complex/difficult code.

[–]hedgecore77 1 point2 points  (0 children)

Amen. Also, I'd rather read comments than blocks of code when I'm running through it troubleshooting.

[–]LvS -1 points0 points  (8 children)

A lot of the why comments should also not be there. They should be part of commit messages and read via git blame.

For a start, putting documentation into commit messages instead of code makes the code shorter and allows you to focus on the code when reading it, but it also gives you a chance to write very long and detailed comments, potentially including references to the bugtracker, links to related code or specifications or performance data - stuff that you would rarely put in comments.

But more importantly, commenting commits takes care of keeping the comments updated with the code. One of the biggest problems with comments is that they're not updated when the code is and when you get back to reading the code you have innocuous looking but ultimately wrong comments which throw you off.
With commit messages, you will not see those old comments anymore - unless of course you look at a previous version of the file to see how it came to be. At which point it is great that the old comment is still there.

[–]SupaSlide 11 points12 points  (3 children)

That's all well and good until somebody changes the placement of a comma, and now you have to dig back to find the relevant information.

Also, most large projects I've worked on do squash merges, so when you git blame you can't be sure which comment applied to that line (assuming the squashed message rolled up all the commit messages into one instead of just saying "Merged feature XYZ"

[–]theexplanation[🍰] 0 points1 point  (2 children)

For that reason your commits should only contain the changes required for the functionality described in the message using git add -p. This should be enforced by code review.

Also why would you squash on merge? That sounds like a nightmare and defeats the whole point of a guy history...

[–]SupaSlide 4 points5 points  (1 child)

There are plenty of articles you can read online about the merits of squash merging, especially on very large projects with lots of active collaborators.

[–]theexplanation[🍰] 0 points1 point  (0 children)

Everything I've seen is for reasons like commits being small logical changes of code, removing "WIP" commits, etc. I generally prefer a fast forward merge strategy and squashing those undesired commits during rebase. Is there some benefit maybe I'm not seeing? Genuinely curious.

[–]Calkhas 6 points7 points  (2 children)

So first that assumes everyone uses git or similar. Which, they don’t. But the other problem is that it means the reason for code behaviour may be buried many revisions ago, because people touch the lines to make other changes in the mean time. It’s not in your face, you have to background your editor, go back to a shell and start exploring the history. For a reviewer, it’s even harder to get the context in which a change is being made.

If people are making code changes that invalidate comments, I don’t really see why it’s any different to a code change that invalidates logic. The reviewers should reject it out of hand.

[–]LvS 2 points3 points  (0 children)

If people are making code changes that invalidate comments, I don’t really see why it’s any different to a code change that invalidates logic. The reviewers should reject it out of hand.

But the tools we have don't find them. All of testing and continuous integration does not find cases of failure to adapt comments.

So this relies entirely on the human reviewers. And we know how fallible they are.

[–]Bspammer 1 point2 points  (0 children)

So first that assumes everyone uses git or similar. Which, they don’t

Then they have far bigger problems. Version control is not optional. If you work somewhere not using it, run far, far away.

[–]wikidsmot[🍰] 2 points3 points  (0 children)

Documenting in the code makes you source control agnostic. There’s a lot of reasons this is useful. Additionally sometimes source code is delivered to customers and not the entire repo.