Is code commenting bad?

[iCyborg]

I think what Markbjn means by "totally comprehensible based on the function name" is that as a CONSUMER of the function, you don't need to know what it going on inside the method. Treat it as a "black box". In a function such as this, which has a single strongly typed parameter and a boolean response, with zero outside dependencies, the internals should never have to be worried about again, once the function is written and tested the first time.

I think we can all agree that putting fat comments of the internals details in something like interface header files is a no-no. But this was about commenting those internals.

@Mark
I'm also quite used to this integer->boolean conversion shortcut normally, but for some reason my brain just failed me at this one, sort of like optical illusions can trick our eyes. If you gave me this in an interview, I'd have no problems, but only because I'd be suspecting foul play and wouldn't just skim over it

 

[Ichinisan]

... should have said "not evenly divisible."

Doh! I also noticed that and thought I hast fixed it before posting. Fixed.

 

[iCyborg]

I think it's fine even in serious math articles and textbooks to use just divisible without qualification. Rarely do I see phrases "evenly" or "without a remainder" with divisibility for integers, as the term is meaningless on its own otherwise - every integer is divisible by any other if you can have remainders.
See e.g. http://mathworld.wolfram.com/Divisible.html

 

[AyashiKaibutsu]

I think what Markbjn means by "totally comprehensible based on the function name" is that as a CONSUMER of the function, you don't need to know what it going on inside the method. Treat it as a "black box". In a function such as this, which has a single strongly typed parameter and a boolean response, with zero outside dependencies, the internals should never have to be worried about again, once the function is written and tested the first time.

If they have the source code then there's probably some expectation that they have to maintain it, and therefore commenting would be advised. Personally, All of the code I write is ultimately going to be maintained by someone else.

 

[Markbnj]

Doh! I also noticed that and thought I hast fixed it before posting. Fixed.

I think, as iCyborg pointed out, that you were right on that and I was wrong.

 

[MagnusTheBrewer]

I was always taught that commenting, aside from explaining what a module does, is primarily for ease of reusing code.

 

[Train]

If they have the source code then there's probably some expectation that they have to maintain it, and therefore commenting would be advised. Personally, All of the code I write is ultimately going to be maintained by someone else.

Oh I'm sure there would still be comments inside the function. I was only pointing out that if you used the function, you wouldn't open it to read every comment inside it. You would infer everything it does on its name alone.

 

[IHateMyJob2004]

People don't code because they are lazy.

Software engineers, programmers, whomever should comment their code unless it is painfully obvious what is going on.

People act all "self documenting" and shit. Well, if it took you 5 minutes to write 10 SLOC and a comment would take 20 seconds, is it really the end of the world.

 

[KIAman]

People don't code because they are lazy.

Software engineers, programmers, whomever should comment their code unless it is painfully obvious what is going on.

People act all "self documenting" and shit. Well, if it took you 5 minutes to write 10 SLOC and a comment would take 20 seconds, is it really the end of the world.

Nope, but when you write tens of thousands of lines of code, over commenting every obvious business rule adds up to days lost.

 

[dighn]

Nope, but when you write tens of thousands of lines of code, over commenting every obvious business rule adds up to days lost.

but it might save you and others looking at the code a lot of time down the road

 

[IHateMyJob2004]

Nope, but when you write tens of thousands of lines of code, over commenting every obvious business rule adds up to days lost.

What I find funny is that kind of argument. If someone works 8 hours a day, how much time is spent doing actual work? 6 if you are lucky.

Sorry, I've been down the path of WTF does this uncommented code do one to many times.

 

[IHateMyJob2004]

but it might save you and others looking at the code a lot of time down the road

Ya, maintenance is done for free by people all over the world.

Also, if you are not doing comments, that means you are not doing peer reviews or doing very poor peer reviews because reviewers will understand less of the code. They either pretend that they do get it or they will have more questions to ask. Under scenario one, quality suffers. Under scenario 2, time cost increases.

 

[iCyborg]

What I find funny is that kind of argument. If someone works 8 hours a day, how much time is spent doing actual work? 6 if you are lucky.

Sorry, I've been down the path of WTF does this uncommented code do one to many times.

So it's even more important that those 6 hours are not wasted

With proper variable/function naming the majority of code is readable to a semi-competent programmer and commenting it is just duplicating information, and this cluttering with redundant stuff often makes code less readable. I'd rather see the complete function on one screen, than seeing a wall of text because someone felt the need to tell me that 'if (pObject == 0) return;' makes sure that pObject isn't null before accessing...

 

[Train]

So it's even more important that those 6 hours are not wasted

With proper variable/function naming the majority of code is readable to a semi-competent programmer and commenting it is just duplicating information, and this cluttering with redundant stuff often makes code less readable. I'd rather see the complete function on one screen, than seeing a wall of text because someone felt the need to tell me that 'if (pObject == 0) return;' makes sure that pObject isn't null before accessing...

// pObject cannot be null

doesn't look like a wall of text to me.

 

[IHateMyJob2004]

So it's even more important that those 6 hours are not wasted

With proper variable/function naming the majority of code is readable to a semi-competent programmer and commenting it is just duplicating information, and this cluttering with redundant stuff often makes code less readable. I'd rather see the complete function on one screen, than seeing a wall of text because someone felt the need to tell me that 'if (pObject == 0) return;' makes sure that pObject isn't null before accessing...

Wow. I wish I wrote college level code for a living.

I can only dream of the code I have to deal with being that trivial. I wish I could tell you what I work on in detail. But I don't even care to start. I'll give you a hint:
java based CAD APIs (and some C/C++ APIs) + most complex machines on earth = far from trivial

 

[iCyborg]

// pObject cannot be null

doesn't look like a wall of text to me.

This was just an example, presumably the function would do more than that. So imagine a function with 30 LOC with 30 little useless comments like this, instead of maybe a couple. If something was indeed worth extra explanation, you wouldn't even notice it. Reminds me of a manager that treats everything as super-urgent, so when something is really urgent, his reports don't give it any extra attention.
If the comment provides no extra information to anybody remotely familiar with the language, then it's redundant.

 

[iCyborg]

Wow. I wish I wrote college level code for a living.

I can only dream of the code I have to deal with being that trivial. I wish I could tell you what I work on in detail. But I don't even care to start. I'll give you a hint:
java based CAD APIs (and some C/C++ APIs) + most complex machines on earth = far from trivial

The code you deal with never checks that objects are initialized?
With all due respect, you don't know what I work on nor can you deduce that from a one-line hypothetical example. If you think you can, then you're missing the big picture here.

 

[Markbnj]

// pObject cannot be null

doesn't look like a wall of text to me.

No, but it does look like a completely trivial and unnecessary comment . I don't think anyone here is arguing against commenting when an algorithm needs to be explained, or the high level operation of a method documented. What I don't need to see is exactly what you gave an example of: a few words of text that say essentially exactly what the few symbols of syntax say. I can read and understand the syntax just fine in simple cases like that.

 

[Train]

No, but it does look like a completely trivial and unnecessary comment . I don't think anyone here is arguing against commenting when an algorithm needs to be explained, or the high level operation of a method documented. What I don't need to see is exactly what you gave an example of: a few words of text that say essentially exactly what the few symbols of syntax say. I can read and understand the syntax just fine in simple cases like that.

I didn't see it that way. pObject == 0 is NOT a null check. If pObject were null that would raise an error in most languages. Ideally you would enforce the not-nullness via a contract, or as cleary as prefixing the above with pObject != null, dependsing on the lang, which wouldn't need a comment, of course.

 

[iCyborg]

I didn't see it that way. pObject == 0 is NOT a null check. If pObject were null that would raise an error in most languages. Ideally you would enforce the not-nullness via a contract, or as cleary as prefixing the above with pObject != null, dependsing on the lang, which wouldn't need a comment, of course.

I'm not sure I'm following. How is comparing a pointer to 0 not a null check?
You might not want to enforce not-nullness via compile/run time errors as it might be OK, and you're simply gracefully handling it like in my example.
As for (pObject != null) vs (pObject == null), if the function is non-trivial, I prefer to have it test for 0 and return, then have the whole function body indented as in

if (pObj != NULL) 
{ 
    [I]... long body[/I]
} [I]// end if (pObj!=0) from 40 lines above[/I]

If it's 5-10 lines and/or not further nested, then it's ok either way.

Or is the problem using 0 instead of null? In C++ it's ok, Stroustrup thinks that too:
http://www2.research.att.com/~bs/bs_faq2.html#null

 

[Markbnj]

I didn't see it that way. pObject == 0 is NOT a null check. If pObject were null that would raise an error in most languages. Ideally you would enforce the not-nullness via a contract, or as cleary as prefixing the above with pObject != null, dependsing on the lang, which wouldn't need a comment, of course.

I don't think it matters whether you want to call it a null check or something else. My point is that the code is entirely self-explanatory, and while that one line doesn't constitute a "wall of text," what happens when you carry that philosophy forward is something like this...

// pObject cannot be 0
if (pObject != 0) ...

// need to increment pObject->Foo
pObject->Foo++;

// have to call Bar()
pObject->Bar();

// check that pObject->Status is status_ok
if (pObject->Status == status_ok) ...

... and so on. In each case the comment says the exact same thing as the line of code. No information of any value is added. All that is added is vertical separation between lines of code that makes the whole thing harder to parse and comprehend. Imo, of course.

 

[Stuxnet]

People don't code [comment???] because they are lazy.

Sometimes yes, sometimes it's because overcommenting is just as bad as not commenting at all. Again, a good design with good coding standards substantially reduces the need to comment. The exception is when you're bound to a shitty design/implementation that can't be refactored with a reasonable effort, but in my 15 years in the biz, I've never come across a situation where refactoring was needed but couldn't be justified. You just have to break it down into small projects that can be bundled in with larger efforts so you don't have to go to a budgeting committee and tell them you need $x to fix code. Now, if your system is so fucked that you can't break it down to that level, your problem has shit to do with people who are "too lazy to comment."

Software engineers, programmers, whomever should comment their code unless it is painfully obvious what is going on.

Again, with good design and implementation (read: team experience), you'll inherently produce "painfully obvious" code. If you're not producing such code, someone is doing something wrong. Comments littered throughout code are nothing but duct tape and bubble gum trying to conceal a much more profound problem... or a programmer(s) who lacks confidence and feels he has to justify his approach to anyone who might stumble across his code. The latter has usually been the case in my experience... most comment abuse I've seen is what I call "defensive commenting." "Someone might see this and disagree with me and think I'm stupid, so I'm going to have that argument right here right now - IN THE CODE!"

People act all "self documenting" and shit. Well, if it took you 5 minutes to write 10 SLOC and a comment would take 20 seconds, is it really the end of the world.

Some of these people have architected/maintained enormous systems that generate millions of dollars per year and which are maintained by support groups in entirely separate business entities... yet very few comments beyond the standard autodoc comments are needed to keep it all running smoothly.

And yes, commenting the crap out of your code IS the end of the world (in the sense you used the phrase, that is), because now you need to maintain the code AND the comments, and there's nothing worse than stumbling across code that says one thing and comments that say another, and you're left wondering which one is wrong. Now you have to go on a version control scavenger hunt trying to piece it all together, looking at 50 versions of the stupid thing trying to figure out where the comment and code diverged. 99% of the time, the comment is what's borked and it needlessly sent someone on a wild goose chase.

Do everything you can to reduce, as much as possible, the need to write and maintain inline comments.

Wow. I wish I wrote college level code for a living.

I can only dream of the code I have to deal with being that trivial.

No offense, but you must work in a pretty amateurish shop if this is your attitude toward what's otherwise fairly sound logic. Maybe you just don't know better. Frankly, good code should look like college level code. That's what you get when you write single-responsibility methods with no black box functionality, maintain separation of concerns, and adhere to the basics you learned the first time you wrote college level code. But hey, we're not REALLY writing code unless it's super creative and needs a few paragraphs to explain to the next guy wtf is really going on, right?

If you find yourself having to deal with spaghetti code that needs a 1:1 code:comment relationship, and you can't get anyone to buy in to spending a few manhours/week tidying it up, maybe you should consider going to another company, because an environment like that will SEVERELY stunt your professional growth. I don't mean that in a derogatory manner at all - I've been there. I worked for a shop that employed one hit & run consultant after another and the code turned into a mess right quick. No shop is perfect, but there are situations that are just utterly hopeless. You can't get through a weekend without getting called, you can't explore good practices, etc. You're always stressed out and you're not growing professionally. Recognize a bad situation for what it is, rather than confusing it for what all shops should be doing.

 

[AyashiKaibutsu]

So just to rant in a thread semi related. I was working on a rewrite of code. The guy writes this weird set of logic that checks if a variable is null and then does stuff that he only wants done the first time in a loop. None of this was commented; I had to figure out that he probably only wanted this stuff done once, and then I had to look through the rest of the code and the valid database entries looking to make sure that variable will only be null on the first pass... Then I look at the next code and it's

//Checks if variable is not a proc
if (!IsProc)
{

... RAGE...

 

[Train]

I don't think it matters whether you want to call it a null check or something else. My point is that the code is entirely self-explanatory, and while that one line doesn't constitute a "wall of text," what happens when you carry that philosophy forward is something like this...

// pObject cannot be 0
if (pObject != 0) ...

// need to increment pObject->Foo
pObject->Foo++;

// have to call Bar()
pObject->Bar();

// check that pObject->Status is status_ok
if (pObject->Status == status_ok) ...

... and so on. In each case the comment says the exact same thing as the line of code. No information of any value is added. All that is added is vertical separation between lines of code that makes the whole thing harder to parse and comprehend. Imo, of course.

I guess we are just lookig at this from different levels of obviousness. I assumed the painfully obvious was not what he was trying to comment.

All the texbook simple examples in this thread are not helping the discussion.

 

[Markbnj]

I guess we are just lookig at this from different levels of obviousness. I assumed the painfully obvious was not what he was trying to comment.

All the texbook simple examples in this thread are not helping the discussion.

Guess I misunderstood. I thought you were arguing that the painfully obvious comment was ok because it didn't take up that much space.