Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Stop commenting your code just to say you did

bradt.ca

Post reply on HN
Print view View original

31–40 of 76 posts

Re: Stop commenting your code just to say you did

#31

Post by mjmahone17 »

SomeCallMeTim wrote: post #25

The example that he gives that I take issue with is the "Constructor" one in a JavaDoc comment. When using documentation tools, every undocumented function spews a warning. And that is as it should be. But some simple constructors don't have any functionality worth documenting. I code as if all warnings should be addressed, and that's true of my documentation generation. If a public function has no documentation, tha…

If you're only documenting after the fact, your function isn't worth documenting: it means you conceptually understood it well enough that you didn't have to think much about it's inputs or outputs. If you did have to think through it, or your code reviewer doesn't understand it, add a comment explaining it. In any other case, commenting on it is probably useless and adding to clutter. Javadocs are painful to read, not because it's a bad idea, but because so often it's things like "toString(): returns a string that is the string of this object". Sorry, I only need to know that function exists and it's return. I want to be able to scan through and see only those functions that are conceptually strange or unique, not the boilerplate that every class has to have, and is identical. If there's a comment on the toString() function, it had better be documenting something interesting, or else it's more useless than nothing.

Re: Stop commenting your code just to say you did

#32

Post by andrewSC »

swanson wrote: post #29
Ellipsis753 wrote: post #27

Earlier quoted context omitted.

It's a pain but check the git history.

Both lines were committed at the same time in a changeset with 15 other files. What now?

Find out who the author was via git blame and ask what they were thinking when writing it. Or in the case that the person who wrote it left, contact someone with some knowledge of the line in question (i.e. the person who either also wrote the majority of the code or is at least familiar with what it does) and ask. If it comes down to it and you're the only one qualified to do something about it, then just remove the line (assuming the code is functioning properly) just how I would approach it.

Re: Stop commenting your code just to say you did

#33

Post by whyme »

I'll suggest there's a missing ingredient here...I found I was able to eliminate many of my comments by asking myself if that someone whom might read my code would be better served with a unit test instead. I also found, however, that this needs to lead to an understanding that unit test are in fact a form of documentation and should be treated as such.

Re: Stop commenting your code just to say you did

#34

Post by ianstormtaylor »

SomeCallMeTim wrote: post #25

The example that he gives that I take issue with is the "Constructor" one in a JavaDoc comment. When using documentation tools, every undocumented function spews a warning. And that is as it should be. But some simple constructors don't have any functionality worth documenting. I code as if all warnings should be addressed, and that's true of my documentation generation. If a public function has no documentation, tha…

+1 I'd much rather side with the "document every function" camp and have a few extra comments, than inevitably degrade to the "document close to nothing" camp and have functions named "normalize" and "resolve" that have to be in their entirety to understand what they do. That being said, I like to drop the `@return void` and `@api public` type entries because they're almost always self-evident, or even not true, like in Javascript where everything is public if exposed and people will reach in no matter what you do.

Writing docblocks can also be a nice way to check yourself from writing bad code. If you find yourself adding 4 different `@param` entries it makes you stop and think, "why is my function taking 4 arguments, it's probably poorly designed".

Re: Stop commenting your code just to say you did

#35

Post by freehunter »

swanson wrote: post #24
carsongross wrote: post #7

God forbid someone gets their thoughts together with comments outlining what a function is going to do and then fills in the code... Or, while deep in thought, someone taps out a bit of redundant commentary on a line or two of their code... Or someone needs a bit of natural language to provide an anchor in their code. The important thing here, I think we can all agree, is that we are smarter than that person.

// don't use hardware acceleration canvas.enableHardwareAcceleration(true); Which is true? The comment is lie. Is this a bug? Was this changed for some reason? Should I remove the comment? Should I toggle the boolean? Why weren't we supposed to use HW acceleration? Why are we using it now? What changed? What value did the comment have in the first place?

I've got some code that looks like

//don't do this

do.this(true)

To me, that signifies that I know the way I'm doing it is wrong, but it works until I can figure out how to do it right.

Yes, I would be a horrible collaborator on a project. Most hackers would be, for the definition of hackers meaning "people who do isomething because it works, not because it's good programming".

Re: Stop commenting your code just to say you did

#37

Post by apdinin »

I can agree with the seeming intention of the OP, but the analogy of adding labels to wires in an electrical box is problematic. How does having blank labels on wires equate to poor commenting?

Good or bad, a comment says something about the code it's commenting (even if the code is self explanatory). A blank label says nothing about the wire.

The better analogy might be having labels on wires that state the color of the wires -- red, blue, green, etc.

Re: Stop commenting your code just to say you did

#38

Post by ww520 »

swanson wrote: post #29
Ellipsis753 wrote: post #27

Earlier quoted context omitted.

It's a pain but check the git history.

Both lines were committed at the same time in a changeset with 15 other files. What now?

Looks up the spec to see the correct behavior, just ask the person did the change, or test it yourself to see why is there a contrasting comment. May be hardware acceleration was meant to be turned off but the guy was testing it by toggling the flag and forgot to set it back. In that case the conflicting comment brings the potential bug upfront.

Re: Stop commenting your code just to say you did

#39

Post by millstone »

swanson wrote: post #24
carsongross wrote: post #7

God forbid someone gets their thoughts together with comments outlining what a function is going to do and then fills in the code... Or, while deep in thought, someone taps out a bit of redundant commentary on a line or two of their code... Or someone needs a bit of natural language to provide an anchor in their code. The important thing here, I think we can all agree, is that we are smarter than that person.

// don't use hardware acceleration canvas.enableHardwareAcceleration(true); Which is true? The comment is lie. Is this a bug? Was this changed for some reason? Should I remove the comment? Should I toggle the boolean? Why weren't we supposed to use HW acceleration? Why are we using it now? What changed? What value did the comment have in the first place?

Sure. Let's say the author intended to disable hardware acceleration, but introduced this bug. A reader might notice the disagreement between the comment and the code, and investigate.

Now take away the comment. You would never know from reading the code that there might be a problem.

This is a bad comment, but it's still more useful than no comment.

Re: Stop commenting your code just to say you did

#40

Post by andrewSC »

freehunter wrote: post #35
swanson wrote: post #24

Earlier quoted context omitted.

// don't use hardware acceleration canvas.enableHardwareAcceleration(true); Which is true? The comment is lie. Is this a bug? Was this changed for some reason? Should I remove the comment? Should I toggle the boolean? Why weren't we supposed to use HW acceleration? Why are we using it now? What changed? What value did the comment have in the first place?

I've got some code that looks like //don't do this do.this(true) To me, that signifies that I know the way I'm doing it is wrong, but it works until I can figure out how to do it right. Yes, I would be a horrible collaborator on a project. Most hackers would be, for the definition of hackers meaning "people who do isomething because it works, not because it's good programming".

Maybe a better way to clarify it's not ideal is to make a TODO. So:

// TODO: Find out a better way to do this

do.this(true)

Post reply on HN