The Joel on Software Discussion Group (CLOSED)

A place to discuss Joel on Software. Now closed.

This community works best when people use their real names. Please register for a free account.

Other Groups:
Joel on Software
Business of Software
Design of Software (CLOSED)
.NET Questions (CLOSED)
TechInterview.org
CityDesk
FogBugz
Fog Creek Copilot


The Old Forum


Your hosts:
Albert D. Kallal
Li-Fan Chen
Stephen Jones

Good code has 35% comments?

http://binstock.blogspot.com/2007/05/comments-how-many-should-you-have.html

Lets not discuss rigging metrics, but I definitely fail on this one.  I usually use overly verbose variable names and only comment the 'weird' stuff.
Grant Send private email
Wednesday, May 30, 2007
 
 
It's really 33.456% comments.
son of parnas
Wednesday, May 30, 2007
 
 
Bad Code<=33.455% comments.
Anon Ranter Send private email
Wednesday, May 30, 2007
 
 
I call BS.

Good code contains enough comments to explain the *intent* of the code.

Depending on the language, the libraries used, the verbosity, the complexity of the algorithm, etc., the amount of comments required to do that can be tons or very little.
Sunil Tanna
Wednesday, May 30, 2007
 
 
Comment inflation happens whenever management measures it, but absent that a significant comment block before each function really helps code quality - but only if the comments describe something interesting.

Comments that merely try to explain in English the same logic that is in the code of the function are generally a waste of everyone's time.  Good commenting serves 3 purposes:

1) Describe how to call this function if you know nothing about it.  It's important to document requirements on arguments (input or output) that the compiler can't enforce -- no point in repeating the information already in the function signature!  It's also important to clearly describe what happens to any output arguments when an error is returned.

2) Describe how this function fits into the big picture.  Is there a similar function elsewhere in the same API?  Describe when you'd call this one instead of that one.  This is perhaps the most useful sort of comment because you can't figure it out by just looking as the source code for the function itself.

3) Very rarely, a function implementation is genuinely hard to understand even for someone skilled in the field.  If you use an API or datastructure that's not used commonly, note this and give a reference to a thorough description (i.e., link to wikipedia, page number in a book or journal that should be widely available).

Most programmers object to commenting simply becuase most comment styles merely waste everyone's time with information that's clear from the source code.  Focus on what *isn't* clear from the source code and comments suddenly seem like a good use of your time.
Skorj
Wednesday, May 30, 2007
 
 
+1 for Sunil. I call bullshit on this as well.

Personally I'd rather see fewer GOOD comments than a lot of comments that are outdated, don't describe the real functionality, incomplete or poorly written.
QADude Send private email
Wednesday, May 30, 2007
 
 
Agreed, one thing you cannot get from reading the code (or pseudo-code) is what the programmer INTENDED the code to do, from a higher level perspective.

So "i++" may be commented "move to the next location" (well, duh) or may be commented "prepare to process the next information node" (more helpful).

Particularly with IF and WHILE keywords

if (i == maxval)
{
can be commented
/* THEN we've reached the end of the list,
  so prepare to do...
*/
AllanL5
Wednesday, May 30, 2007
 
 
AllanL5,

even these comments are superfluous and only repeating what happens in the code. These comments are much more useful:

int maxval; // Number of elements in the list
int i; // Running index in the list
Secure
Wednesday, May 30, 2007
 
 
Supposedly that number comes from 'average open source projects', which doesn't really match any open source project I've had to write a patch for.  My guess is it's all javadoc boilerplate.
Grant Send private email
Wednesday, May 30, 2007
 
 
Secure,

Even YOUR comments are superflous.  How's about this?

    int num_elements;
    int i;

'num_elements' is a variable AND a comment.

'i' should never be anything BUT a loop counter.

This has been posted many a time, but I'll repeat for the newbies:  Good code should be self-documenting (see Fowler's "Refactoring", page 87).
No comment
Wednesday, May 30, 2007
 
 
Of course, even having those variable isn't clear.  Prefer instead:

std::for_each

To clearly express the intent.

Wednesday, May 30, 2007
 
 
Hows this for a rule of thumb?  Comments are only useful if they start off with:

TODO:
XXX:
BAD VOODOO AHEAD
WORKAROUND/FIX FOR BUG #
ABANDON HOPE ALL YE WHO ENTER
Grant Send private email
Wednesday, May 30, 2007
 
 
Geez you missed a big one there:

HACK
zax
Wednesday, May 30, 2007
 
 
> Good code should be self-documenting

The examples with variables are arguably useless comments, or very nearly so.

But I'd love to know how information about _why_ a programmer made a certain choice can be expressed in code without comments.

Also, I think it'd be fair to say that in lower level languages comments are relatively more important.  For example if you've got an assembler routine for a complex algorithm, you generally want a comment every 10/20/30/50/chunk of instructions just so you can quickly skip to through the irrelevant bits of code and find the bit you're interested in reading.
Sunil Tanna
Wednesday, May 30, 2007
 
 
> > Good code should be self-documenting

Please post some of this mystical self-documenting code. Let's see how self-documenting it really is to someone who knows nothing about your system.
son of parnas
Wednesday, May 30, 2007
 
 
It's hard to make a short example on the fly, but I agree with "secure"s comment, and my point remains the same -- from reading the code, you get the mechanics of what is going on.  However, the 'larger purpose', the 'intent' (like we're sorting this data BECAUSE xyz, or we're traversing this data structure in order to do xyz) is what comments can tell you that the code cannot.

No matter how cleverly you write your variable names.
AllanL5
Wednesday, May 30, 2007
 
 
I'm confused. Wouldn't one of the criteria for declaring a piece of code "really good" be the ability to understand it without comments?

If that's so, wouldn't we say:

"Great code has 0% comments"?
Bruce
Wednesday, May 30, 2007
 
 
> Please post some of this mystical self-documenting code. Let's see how self-documenting it really is to someone who knows nothing about your system.

Or, you could post some of your code and let people here try to rework it so it needs fewer/no comments.
Anon
Wednesday, May 30, 2007
 
 
> Or, you could post some of your code and let people here try to rework it so it needs fewer/no comments.

Those making a claim need to make a falsifiable test. Let's see a good chunk of self documenting code. Not a function. But at least 10,000 lines.
son of parnas
Wednesday, May 30, 2007
 
 
"""
> > Good code should be self-documenting

Please post some of this mystical self-documenting code. Let's see how self-documenting it really is to someone who knows nothing about your system.
"""

I think there's some context involved with the original statement.  It's not saying that you don't need to write *any* documentation.  I hate the 8 million projects that use javadoc or doxygen or whatever as their ONLY form of documentation.  A listing of API signatures doesn't count as comprehensive documentation!

That being said, even with a well-written and well documented system and a competent developer, there's going to be a learning curve to get up to speed on a new code base.
Grant Send private email
Wednesday, May 30, 2007
 
 
10% is good: average one line comment per 10 lines of code
Ben Bryant Send private email
Wednesday, May 30, 2007
 
 
"Instead of imagining that our main task is to instruct a computer what to do, let us concentrate rather on explaining to human beings what we want a computer to do." — Donald Edwin Knuth
Mikkin
Wednesday, May 30, 2007
 
 
I'm with SoP on this one.  But I'd settle for 1500 LOC.  500 LOC wouldn't be too bad.

Though there's a point -- too small a sample of 'self-documenting' code could be 'too simple' a solution, so it doesn't NEED any comments.

Again -- code does not communicate 'intent', the 'why' of why it's written the way it is.  Because the computer (and compiler) doesn't CARE 'why' it's written the way it is.  But humans DO care, especially if they have to maintain it.
AllanL5
Wednesday, May 30, 2007
 
 
+1 to Skorj, for the most part. But with a couple of exceptions.

Comments that repeat what the code is doing in English are not always a waste of time. Unless the code is sufficiently simple to be understood at a glance, describing it in comments serves two purposes. 1) When bug hunting you can skip over that piece of code if the function it carries out isn't relevant to what you are looking for. You can usually decide that in a few seconds from a comment, while it might take several minutes from the code; 2) Finding a piece of code that doesn't do what its comments said it does make it a prime candidate for closer inspection during debugging. You'll never know that if you don't comment it.

Whoever said "good code is self documenting", that's only true if you are writing trivial code. If all you're doing is placing buttons on a screen then sure, it explains itself. Anything more complex requires comments.
DJ Clayworth
Wednesday, May 30, 2007
 
 
P.S. I have never in twenty years of coding found code with too many comments. Anything that encourages coders to write more comments is good.
DJ Clayworth
Wednesday, May 30, 2007
 
 
HA!

So I search for "self documenting code" and there's a link to the Code Complete website - Chapter 32 resources.  One link is "Real World Code Examples".  I try to click, they make me sign up.  So I sign up, and this page is nothing but a link to SourceForge!

UGH!

Anyway, "self documenting" is read aloud as "(as) self documenting (as possible)".  And ironically enough, McConnell has a checklist for comments in self documenting code.  The one that pretty-much sums it up: Does every comment count?

Anyway, here's an example of the 'good project' from the original article:

http://svn.apache.org/viewvc/xmlgraphics/fop/tags/fop-0_93/src/java/org/apache/fop/render/AbstractRenderer.java?revision=492289&view=markup

Those comments are a joke.
Grant Send private email
Wednesday, May 30, 2007
 
 
Most comments are crap. Most code is crap.

Which should people spend their time improving?

35% comments/code is, as a general rule, too high.

Code can't be completely self documenting but often (like in the above examples) code can often be made much more self documenting than it is.
somebody else
Wednesday, May 30, 2007
 
 
So "i++" may be commented "move to the next location" (well, duh) or may be commented "prepare to process the next information node" (more helpful).

Particularly with IF and WHILE keywords

if (i == maxval)
{
can be commented
/* THEN we've reached the end of the list,
  so prepare to do...
*/

AllanL5's examples are a bit scary.

Why not use meaningful variable names instead of (what should be) superfluous comments?
somebody else
Wednesday, May 30, 2007
 
 
"code can often be made much more self documenting than it is"

I never intended to imply that comments should be used as a substitute for clear code. In fact I would sum my position as:

"Write code that is so clear it doesn't need comments. Then comment it".
DJ Clayworth
Wednesday, May 30, 2007
 
 
DJ Clayworth, my comment wasn't directed at you.

"Comments that repeat what the code is doing in English are not always a waste of time."

I'd say it would (sometimes) be OK to summerize what the code is doing. Typically, when people talk about "comments that repeat", they are talking about comments that are nearly a one-to-one match of the code. These kinds of comments are often what you get when you tell people that their code must have comments in it!

"Write code that is so clear it doesn't need comments. Then comment it"

Yes.
somebody else
Wednesday, May 30, 2007
 
 
I remember some java-based code tool that parsed your code and gave you "errors" above and beyond what the compiler gave you. Mngmt decided to publish the "errors" on the company portal and we were required to correct everything. This code:

private void setHours(int hours)
{
this.hours=hours;
}

...had something like 50 errors in it, most of which related to comments. So...

/**
 * Sets the hours.
 *
 * @param hours The hours.
 */
private void setHours(int hours) {
//setting the hours the hours
  this.hours = hours;
}

...notice new whitespace also.

I think it took me 2-3 weeks of steady typing to "fix" everything.
mynameishere
Wednesday, May 30, 2007
 
 
Yes, I agree my examples suck.  I was trying to illustrate (Very badly) that the "intent" of code was not being communicated by the code. 

But those examples were too short to be useful, and did not illustrate what I wanted to illustrate.  Doesn't change the point -- code does not communicate 'intent', 'why' the code is doing what it is doing.
AllanL5
Wednesday, May 30, 2007
 
 
And note, in "mynameishere"'s comments, nowhere does he say WHY you want to 'set the hours', what 'setting the hours' is done for, what PURPOSE 'setting the hours' achieves.

Thus implementing what we all agree are 'pointless' or 'useless' comments, which duplicate 'what' the code is doing, while adding no information you couldn't get from the code already.  And leaving out the useful information comments COULD have given you.
AllanL5
Wednesday, May 30, 2007
 
 
Forgive the long post.

The following code is 7.5% comments (7 of 93 lines). Reasons why 7.5% is sufficient:
- Long variable names
- Higher level language (C#)
- Leans on libraries with clear names
- All developers use an IDE that lets you hover over an object to see its description, and optionally right-click on that object to see all kinds of metadata.

I'm willfully violating about a dozen binding contracts and probably a few federal laws by posting this, but son of paranas asked for an example of 'mystical self-documenting' code, and I felt compelled to respond. This particular snippet required zero explanation was it was handed off to another developer who happened to be a new employee.

[Note: formatting will invariably be totally screwed due to line wrapping. You should be able to copy paste into notepad to get a better sense.]

--

using System;
using System.Collections.Generic;
using System.Configuration;
using System.Text;
using PC = GratisInc.Services.ProductCatalog;
using GratisInc.Services.ProductCatalog.Merchants;
using GratisInc.API.Amazon;
using Ecs = GratisInc.API.Amazon.AwsEcsGb;

namespace GratisInc.Tools.Product.Amazon.SpiderGB
{
    class Program : ProgramBase
    {
        static void Main(String[] args)
        {
            Int32 merchantId = MerchantIds.Amazon_co_uk;

            Log("Execution of {0} v{1} started.", ProgramName, ProgramVersion);

            try
            {
                Int32 maximumNodesToCrawl = Int32.Parse(ConfigurationManager.AppSettings["MaximumNodesToCrawl"]);
                Int32 nodesCrawled = 0;

                // Create the objects needed.
                AmazonSpiderSetingsFactory assFactory = new AmazonSpiderSetingsFactory();
                PC.ProductFactory pFactory = new PC.ProductFactory();
                ECommerceGB amazon = new ECommerceGB();

                // Get the nodes to crawl.
                List<AmazonSpiderSetings> nodes = assFactory.ListReadyToIndex(merchantId);
                Log("The spider found {0} nodes that need to be crawled.", nodes.Count);

                if (nodes.Count > 0)
                {
                    // Firstly, we need to get all of the merchant attributes. Save this in a dictionary.
                    Dictionary<String, Int32> merchantAttributes = new PC.MerchantAttributeFactory().GetNameDictionary(GratisInc.Services.ProductCatalog.Merchants.Amazon.MerchantId);

                    // Now, crawl each node in the list.
                    foreach (AmazonSpiderSetings node in nodes)
                    {
                        if (nodesCrawled >= maximumNodesToCrawl) break;
                        Log("Crawling the '{0}' node. [BrowseNodeId={1}]", node.Name, node.BrowseNode);

                        // Get the Amazon ASINs.
                        String[] asins = amazon.ListPages(node.BrowseNode, node.SearchIndices, node.PageStart, node.PageEnd);
                        Log("--Contains {0} items.", asins.Length);

                        // For each ASIN, add it to the database if it is new.
                        foreach (String asin in asins)
                        {
                            if (!pFactory.Exists(GratisInc.Services.ProductCatalog.Merchants.MerchantIds.Amazon_co_uk, asin))
                            {
                                try
                                {
                                    Ecs.Item item = amazon.LoadItem(asin);
                                    PC.Product p = AmazonConvertGB.ItemToProduct(item, merchantAttributes);
                                    pFactory.InsertFull(p, PC.Source.Spider);
                                    pFactory.AssignDepartment(p.Id, node.DepartmentId);
                                    if (pFactory.IsValid(p.Id))
                                    {
                                        pFactory.Activate(p.Id);
                                        Log("----{0} inserted with id {1} and activated.", asin, p.Id);
                                    }
                                    else
                                    {
                                        Log("----{0} inserted (non-active) with id {1}.", asin, p.Id);
                                    }
                                }
                                catch (Exception ex)
                                {
                                    Log("----{0} did not insert. {1}", asin, ex.Message);
                                }
                            }
                            else Log("----{0} already exists", asin);
                        }

                        // Update the node's LastIndexed date.
                        node.LastIndexedOn = DateTime.Now;
                        assFactory.Update(node);
                        Log("--Crawl complete.");
                        nodesCrawled++;
                    }
                }
                Log("Finished crawling for now.");
            }
            catch (Exception ex) { Log("Exception. See log for details."); LogException(ex); }

            Log("Execution complete.");
            Terminate();
        }
    }
}
PWills Send private email
Wednesday, May 30, 2007
 
 
PWills, You missed a comment:

// Baby got back!
 assFactory.Update(node);

:)
Ryan Smyth Send private email
Wednesday, May 30, 2007
 
 
PWills:

I thought the point behind "self-documenting code" was code with NO comments, because the 'code' was itself 'self-documenting'.

Now, you have a reasonably good example there of useful comments.  So, 7.5% comments are probably sufficient -- 35% would be way overkill.

But some comments ARE necessary and useful.
AllanL5
Wednesday, May 30, 2007
 
 
"Though there's a point -- too small a sample of 'self-documenting' code could be 'too simple' a solution, so it doesn't NEED any comments."

You think that the best place to specify the overall workings of a system is in the code? Isn't that why people create various specification documents.

The most a coder should comment is the intentions of a procedure, not how the solution should work.

So one procedure should be all that is required to demonstrate self documentation.

And very often it is more than enough to make the procedure name descriptive enough to self document it. And with re-factoring tools this is a plausible strategy.
zax
Wednesday, May 30, 2007
 
 
Sorry PWillis, but I'd consider all those log statements to be the equivalent of comments.
Grant Send private email
Wednesday, May 30, 2007
 
 
>> Good code should be self-documenting <<

While writing self-documenting code is an honourable endeavour, you can say stuff with comments that are impossible to say with code. Here are some real-life examples:

// Make sure that strong name signature is non-zero.
// This checks for a CLR 1.x bug where a hacker can skip
// strong name verification by zeroising a single byte at
// the start of the strong name signature.

// P/Invoke to locate SQL Servers on the network using
// ODBC.
// Doesn't use SQLDMO because of COM installation and
// registration nightmares - SQLDMO has multiple
// dependencies.
// Doesn't use Win32 NetServerEnum() because it returns
// Windows server names, not SQL Server names.
// Also NetServerEnum() doesn't work so well in a
// workgroup environment.
// Note that ODBC is installed by default on Win 2000 and
// upwards (our target operating systems).

// P/Invoke to check validity of assembly strong name.
// Using byte for arguments rather than bool, because
// bool won't work on 64-bit Windows.

// Private constructor because this type has no
// non-static members.
Mark Pearce Send private email
Wednesday, May 30, 2007
 
 
class JoelOnSoftwareReader
 
  def whine
    "Waaaahh!!!"
  end
 
  def moan
    2.times { |s| puts whine }
    complain
  end
 
  def complain
    "This code is not self-documenting!!!"
  end
 
end

reader = JoelOnSoftwareReader.new()
infinity = 1.0 / 0

1.upto(infinity) do |count|
  puts reader.whine
  puts reader.moan
  puts reader.complain
end
N
Wednesday, May 30, 2007
 
 
PWills: What does you code do?  Why should I have to get into the details to know the overview?

Sincerely,

Gene Wirchenko
Gene Wirchenko Send private email
Wednesday, May 30, 2007
 
 
PWillis, those are a good example of useful comments IMO, except this pet peeve:

// Firstly, we need to get all of the merchant attributes.

We "need" to get them? That's nice, but how about we actually get them? (oh... next line). That one should be //Get all the merchant attributes. Just a pet peeve! I agree with DJ above the big value in comments is being able to quickly skip around and find the section of code you're looking for to debug.
John
Wednesday, May 30, 2007
 
 
@Gene Wirchenko:
The code is a command line program that imports products from Amazon's GB (Great Britain) inventory. The name of the program should convey its purpose: GratisInc.Tools.Product.Amazon.SpiderGB. We've lost some context here; in reality when this program's name is viewed in source control with all the other programs, the purpose is clear.

@John:
Yes that's a useless comment. I wasn't arguing that this code is flawless (far from it), merely that it doesn't require 35 comments.

@Grant:
Valid point. Most programs I've worked on have been instrumented in some way (Debug, Trace, or custom Log statements). These are better than comments; they benefit the developer, the operations team, and the end user.
If you count Log() methods as comments, then we are at 20%(19/93). I would be more willing to agree with the statement "Good code is 35% comments and instrumentation".

@Ryan Smyth:
The business object is 'AmazonSpiderSettings', so naturually, I named a variable 'assFactory'. Nobody said software developers can't have fun. ;-)
PWills Send private email
Wednesday, May 30, 2007
 
 
I still at a loss, at how the no-comments people think the Why-commente can be replaced by longer variable or function names.

I'm talking about comments like:

// algorithm = GROW UNTIL TOO BIG
// then shrink until fits


// use my own function because DrawTextEx fails on MS Image Printer


// number of words to print per page
// - must divide by 2 and by 3 because:
Sunil Tanna
Wednesday, May 30, 2007
 
 
I like the commenting technique used in the PWills example. I have heard it called "paragraph commenting". The concept is that you first write pseudo code for whatever you are implementing. You then convert each line of pseudo code into real code. Each line of pseudo code then corresponds to a chunk of real code, and blank lines are inserted to separate the chunks. The blank lines make the chunks look like text paragraphs, so they're called paragraphs. The initial pseudo code is retained in the form of an introductory comment for each paragraph.

Using this technique gets addictive, and you might find yourself adopting the same structure even if you don't start off by writing pseudo code. The idea remains to document the intent of each paragraph, leaving the code itself to "self-document" the detailed implementation of the intent. Every line of code belongs to a paragraph, so every line of code is effectively commented, but with dramatically less effort and more benefit than actually adding a comment for each line.
Bill Forster Send private email
Wednesday, May 30, 2007
 
 
"@Gene Wirchenko:
The code is a command line program that imports products from Amazon's GB (Great Britain) inventory. The name of the program should convey its purpose: GratisInc.Tools.Product.Amazon.SpiderGB. We've lost some context here; in reality when this program's name is viewed in source control with all the other programs, the purpose is clear."

"should" is a lovely word.  At times, it is practically synonymous with "is not".

I should not have to look at other things to determine what one thing is doing.  Your first sentence above is a good start though.

Sincerely,

Gene Wirchenko
Gene Wirchenko Send private email
Thursday, May 31, 2007
 
 
+1 BS.  I'd say 10-15% is a more accurate value.

"P.S. I have never in twenty years of coding found code with too many comments. Anything that encourages coders to write more comments is good".

I have.  I disagree that more equals better.  Cluttering the code with useless comments only teaches people not to read comments (or trust them).

Thursday, May 31, 2007
 
 
I've seen code with too many comments as well.

The kind of people who comment "break;" with "/* End this madness!!!*/"


Every.

Damn.

Time.


And really nothing else of any use. Just those sorts of things. That's worse than uncommented.
Katie Lucas
Thursday, May 31, 2007
 
 
It all depends on the people handling the code. The wider (and more uninformed about the business domain) the audience is, the more comments you need. If your company has a high turnover and you constantly need people to understand the code, the more they will benefit from extensive documentation. If you don't, your documentation doesn't need to be as much.
Jakob Magiera
Thursday, May 31, 2007
 
 
"I still at a loss, at how the no-comments people think the Why-commente can be replaced by longer variable or function names."

has anyone actually said that (non sarcastically)? quite a few have said that less is more, which id agree with, but 0 comments is an indefensible position i think.
jk
Thursday, May 31, 2007
 
 
@Gene:
"I should not have to look at other things to determine what one thing is doing."

I vehemently disagree. Context is everything. There are two occassions that a developer will look at a block of code in isolation:
[a] school
[b] threads like this

Apart from those two situations, a block of code will *always* exists in a larger context. Therefore it's reasonable and prudent to expect the reader to use "other things" to determine what "one thing" is doing.
PWills Send private email
Thursday, May 31, 2007
 
 
> has anyone actually said that (non sarcastically)? quite a few have said that less is more, which id agree with, but 0 comments is an indefensible position i think.

I thought the person who signed as NoComment said that. But maybe I missed the sarcasm.
Sunil Tanna
Thursday, May 31, 2007
 
 
"Or, you could post some of your code and let people here try to rework it so it needs fewer/no comments."

How would you rework the following to require no comment:  (LOCKCRITICAL has an automatic destructor that releases the lock that it grabbed in the constructor)

{  // WARNING: do not remove empty braces as it is required to release the lock
  LOCKCRITICAL _cl(&g_mainlock);
  ...
  ...
}
...
...
Donald Duck
Thursday, May 31, 2007
 
 
When working with someone else's code, I really do not care why he chose one approach out of the many available.  If I am looking at his code it is for one of two reasons.  One, it does not work correctly and I need to change how it functions.  Two, what the code needs to do has changed and I need to change how it functions.

If you really feel a need to brag about the time and effort you went into developing a piece of code, do it over beer and pizza.  Don't litter the code telling me why you decided to do something, I have more important things to be concerned about.
Wayne M.
Thursday, May 31, 2007
 
 
Not knowing why is exactly how critical knowledge gets lost when code is refactored.

The DrawTextEx work-round comment is exactly the sort of thing that is a key bug fix, not detected until in production or necessarily by a testing suite,  BUT would likely be removed when a developer refactors/simplifies the code unless a comment tells him why the code is this way
Sunil Tanna
Thursday, May 31, 2007
 
 
Donald Duck,  how about putting the command sequence inside a static function?  This would guarantee that the sequence is always implemented correctly rather than relying upon a comment.
Wayne M.
Thursday, May 31, 2007
 
 
Donald Duck:

use boost::mutex::scoped_lock

The name says that it locks in the scope.  A developer then knows the intent.

Further, placing the critical section in it's own function as was suggested above makes the intent even MORE clear.

Comments depend on the type of project and the expected used and maintainance expectations for the future.

Thursday, May 31, 2007
 
 
@Gene:
"I should not have to look at other things to determine what one thing is doing."

'I vehemently disagree. Context is everything. There are two occassions that a developer will look at a block of code in isolation:
[a] school
[b] threads like this

Apart from those two situations, a block of code will *always* exists in a larger context. Therefore it's reasonable and prudent to expect the reader to use "other things" to determine what "one thing" is doing.'

Of course, but that is NOT what I am talking about.  I should not HAVE TO look at other things to see what I am looking at does.  I may well look elsewhere for that additional context.

In another area, I once complained to someone about his handwriting on something that I was supposed to be able to read.  The person told that it was obvious what it said if I would look at another part (how various characters were made).  My reply to that was that if I had to look elsewhere to be able to read it, then it was not obvious.

Typical situation: What does the routine that I am looking at do and why?  How easily can this be answered?  If there is appropriate commenting, I can read the comment and not have to get mired in the details.  I might then skip the whole thing, because it is not what I am looking for.  I should not have to pick it all apart to find this out.

I do not read code nearly as often as I skim it looking for something.  I do not want to spend a lot of time looking.  Good comments really help.

Sincerely,

Gene Wirchenko
Gene Wirchenko Send private email
Thursday, May 31, 2007
 
 
>> One, it does not work correctly and I need to change how it functions. <<

Maybe it *is* working correctly, and it's your thinking that's faulty. That's happened to me several times, and a good comment can tell me why I'm wrong.
Mark Pearce Send private email
Thursday, May 31, 2007
 
 
Good post, Gene. That's exactly what comments are for.
DJ Clayworth
Thursday, May 31, 2007
 
 
DJ Clayworth: Thank you.

Sincerely,

Gene Wirchenko
Gene Wirchenko Send private email
Thursday, May 31, 2007
 
 
35% comments is almost certainly too much. Personally I find that comments break up the flow of the code and can easily lengthen a function to the point where it's no longer easy to read. Comments are not free.

And of course I particularly loathe great big block headers stating that a constructor is a constructor, belongs to the class it belongs to, and doesn't return any parameters.

Feeling the need to add comments to code is also a sign that the code itself perhaps could use improvement. For example....

} //end while

on the end of an enormous loop, where there was difficulty associating the stack of end braces with their correct if, where, else etc further up the function. If your code needs to have a comment, then perhaps it can be changed so that the comment is no longer needed. Make the loop contents into a function, perhaps. The code can often become sufficiently obvious that the comment isn't needed any more.

And there are other cases where there isn't even an option. Interfaces, for example. I comment interfaces heavily to describe the implied contract behind each of the function signatures. Why? There may be several implementations, and how are you to know which one is wrong if they implement different contracts? How do you write a new implementation if you don't know what you are required to do?
Duncan Sharpe
Thursday, May 31, 2007
 
 
It seems that there are a few principles that we all agree on:
[a] A well-place comment improves readability
[b] A superfluous comment hinders readability
[c] 35% seems suspciously high
[d] 0% seems suspiciously low

Any other take-aways from this discussion?
PWills Send private email
Thursday, May 31, 2007
 
 
@Gene:
"I should not HAVE TO look at other things to see what I am looking at does."
I understand the sentiment, but I can't agree. I also suspect that you don't actually believe this. Two examples:

[1] If you're like me, you are very particular about the way you organize documents on your filesystem. Does the title of every document fully convey its purpose? No. A hierarchy of folders is used to fully convey the purpose. Since the only way to access a file is to first traverse some kind of directory tree, you haven't lost any meaning: by the time you arrive at "CodingStandards.doc" you already know that it is for client 'Acme' and is part of the 'Developer Manual'.  A file that is named AcmeDeveloperManualCodingStandardsMay2007.doc is not Good Filing -- the superfluous name hinders readability.

[2] Authors of long pieces of non-fiction use section titles to organize thoughts. We've all read students' papers that use a section title and then regurgitate the same language in the first paragraph of that section. This is not Good Writing. In contrast, I'm currently in the middle of Doug Hofstadter's latest book, and his use of section titles is as excellent as you would expect: a section titled "Bertrand Russell's Second-worst Nightmare" doesn't mention Bertrand Russell until the second-to-last sentence, to great literary affect.

Good Filing and Good Writing both expect the audience to look at other things to see what one thing does. A world in which every 'thing' fully explains its own purpose is not a world I would want to live in.
PWills Send private email
Thursday, May 31, 2007
 
 
In the time it took you people to add 63 posts to this thread you could have written code with at least 36% comments.
Chris Nahr
Friday, June 01, 2007
 
 
"It seems that there are a few principles that we all agree on:
[a] A well-place comment improves readability
[b] A superfluous comment hinders readability
[c] 35% seems suspciously high
[d] 0% seems suspiciously low

Any other take-aways from this discussion?"

[e] even when everyone basically agrees they will continue to argue on teh internet
jk
Friday, June 01, 2007
 
 
"A file that is named AcmeDeveloperManualCodingStandardsMay2007.doc is not Good Filing -- the superfluous name hinders readability."

Until the file is accidentally moved into another folder with a left-click in Windows Explorer, unnoticed by the user, or until you want to send it as a mail attachement.

OTOH, when the complete folder path is repeated in the file name, you will have a hard time when you want to reorganize your folder structure. But how often does this happen in reality, once the hierarchy is grown above a few hundred files?

It depends if the file is an entity of its own, or if it is embedded within a larger context like a single function of a module or like a single chapter of a book, and how likely it is that it will be taken out of its context.
Secure
Friday, June 01, 2007
 
 
>> Any other take-aways from this discussion? <<

Comments should emphasise the "why" aspect.
Mark Pearce Send private email
Friday, June 01, 2007
 
 
Wow!  I didn't expect "good code should be self-documenting" to be such flame-bait!  (Just kidding -- I'm exaggerating...  With a couple of small exceptions, I find the responses in this thread to be well-reasoned and thought-provoking.)

Now, I never said that code should be 0% commented.  That's a straw man, which, of course, is designed to be easy to argue against.

So let me clarify my position (which will in part coincide with some other folks' replies above):

  1) Fast-changing or oft-changing code quickly makes its comments obsolete.  Developers all too often get the change working and don't update the comments.  The next guy (if he even trusts or reads the comments) wastes time trying to reconcile them with the code.
  2) As a corollary to point 1, cut-and-paste developers have a habit of copying comments, changing the code, and leaving behind outdated, incorrect, and misleading comments.
  3) I agree with Grant and nax that comments such as TODO and HACK are useful.  TODO should obvoiusly be temporary, so I don't think that really counts as a "real code comment".  I do use HACK comments, usually if I need to blame some other component for some bad code I had to write.
  4) I also agree that comments are useful to explain WHY you wrote code that is particularly hard to understand.  I'd estimate that for every 5 of those comments I write, I end up deleting 4 of them after I refactor my mess into something that makes sense to the reader.
  5) For those advocating the "paragraph header" use for comments, I tend to convert those "paragraphs" into shorter methods with descriptive names.  Those "headers" then become superfluous.
  6) Comments that explain a variable's purpose should be replaced with good variable names.

All in all, I consider *most* code commenting to be an unnecessary (and sometimes counterproductive) crutch.  It's not that comments are bad.  It's that code that *needs* comments is probably not as good as it should be.

Perhaps one could infer from "good code should be self-documenting" that "BAD code CAN'T be self-documenting".  (That's probably a logical fallacy of some sort, but you know what I mean.)

And no, sorry, son of parnas -- I can't supply a 10,000-line code sample, because 1) all my code belongs to my employer, and 2) I write in Ruby, so my code tends to be shorter and less convoluted anyway. ;)

I agree that PWills' code sample is a pretty good example of reasonably clear code with very few comments.  But in response to allow me to show why I think this good example is actually still OVER-commented.  (I realize all this refactoring would be overkill for a standalone program like this, but consider these as examples of general principles.)

  // Create the objects needed.
  AmazonSpiderSetingsFactory assFactory = ...
  [This comment adds no knowledge.  The code speaks for itself.]

  // Get the nodes to crawl.
  List<AmazonSpiderSetings> nodes = ...
  [Could just call the variable "nodesToCrawl", making the comment redundant.]

  // Firstly, we need to get all of the merchant attributes. Save this in a dictionary.
  Dictionary<String, Int32> merchantAttributes = ...
  [This is a good "why" comment, though maybe such a complicated statement would be better coded as a small method with a meaningful name.]

  // Now, crawl each node in the list.
  foreach (AmazonSpiderSetings node in nodes)
  [Somewhat helpful comment, but perhaps "foreach (... node in nodesToCrawl)" makes it redundant.]

  // Get the Amazon ASINs.
  String[] asins = ...
  [This comment also becomes redundant if you call the variable amazonAsins.  Even better, make a GetAmazonAsins() method.]

  // For each ASIN, add it to the database if it is new.
  foreach (String asin in asins)
  [A good place for a method called "AddNewAsinsToDatabase()".]

  // Update the node's LastIndexed date.
  node.LastIndexedOn = DateTime.Now;
  [A very redundant comment, I'd say.]

Hey, don't blame me.  I've been brainwashed by a few years of reading and re-reading Martin Fowler's "Refactoring" book and blog.  And this has been further reinforced by applying these principles and writing working, maintainable code with minimal comments.  And in case this all comes across as holier-than-thou, I really don't mean it that way.  I know I'm not perfect, but I feel it's worth trying to head in that direction anyway.

Sorry to go on and on, but you all left me with a lot to respond to!
No comment
Friday, June 01, 2007
 
 
>> I also agree that comments are useful to explain WHY you wrote code that is particularly hard to understand.  I'd estimate that for every 5 of those comments I write, I end up deleting 4 of them after I refactor my mess into something that makes sense to the reader. <<

Could you explain which of the 4 "Why" commenting examples I gave above should be removed, and how you would re-structure any associated code to enable their removal?

>> And in case this all comes across as holier-than-thou, I really don't mean it that way. <<

Then why didn't you refactor your argument to be less holier-than-thou? If you can refactor your code, surely you can refactor your words...
Mark Pearce Send private email
Friday, June 01, 2007
 
 
Mark Pearce said much earlier:

>> Here are some real-life examples:

>> // Make sure that strong name signature is non-zero.
>> // This checks for a CLR 1.x bug where a hacker can skip
>> // strong name verification by zeroising a single byte at
>> // the start of the strong name signature.
>>
>> // P/Invoke to locate SQL Servers on the network using
>> // ODBC.
>> // Doesn't use SQLDMO because of COM installation and
>> // registration nightmares - SQLDMO has multiple
>> // dependencies.
>> // Doesn't use Win32 NetServerEnum() because it returns
>> // Windows server names, not SQL Server names.
>> // Also NetServerEnum() doesn't work so well in a
>> // workgroup environment.
>> // Note that ODBC is installed by default on Win 2000 and
>> // upwards (our target operating systems).

>> // P/Invoke to check validity of assembly strong name.
>> // Using byte for arguments rather than bool, because
>> // bool won't work on 64-bit Windows.

>> // Private constructor because this type has no
>> // non-static members.


And much more recently, I said:

>>>> I also agree that comments are useful to explain WHY you wrote code that is particularly hard to understand.  I'd estimate that for every 5 of those comments I write, I end up deleting 4 of them after I refactor my mess into something that makes sense to the reader.

And Mark replied:

>> Could you explain which of the 4 "Why" commenting examples I gave above should be removed, and how you would re-structure any associated code to enable their removal?

Mark, I don't think that your question is really related to my statement.  I'm talking about bad code that *I* wrote and then explained away with comments (and later refactored and thus made the explanatory comments obsolete).  Your examples seem to be talking about legitimate WHYs ("I'm doing this to hacker-proof my code") and HACKs ("I'm doing this because my platform's internal code doesn't work as one would expect").

But, I do realize that none of my remarks addressed your real-life examples.  Not directly, anyway.

So, my answer is:  As far as I can tell, *none* of your comments should be removed, because they are not artificial crutches.  Your comments actually *help* the reader understand what's going on in your code, rather than just *echo* it or even *contradict* it, like so many other comments seem to do.

Like I said before, I'm not advocating 0% comments.  I'm just saying the ones that exist should be justified.  Otherwise, they're just noise.


>> Then why didn't you refactor your argument to be less holier-than-thou? If you can refactor your code, surely you can refactor your words...

Now you just sound funnier-than-thou.  Nice comeback.  :)

But really, I guess it's because English is a lot harder to write than code (for me, anyway).  I *did* refactor my words before I posted them (but I obviously didn't compile them, because I see I left a couple of typos and syntax errors in there).

I suppose I should find a refactoring book that covers English.  But mostly, I just need to practice posting to forums.  I mostly just lurk here, but this particular topic inspired me to participate for once.
No comment
Friday, June 01, 2007
 
 
>> Your examples seem to be talking about legitimate WHYs ("I'm doing this to hacker-proof my code") and HACKs ("I'm doing this because my platform's internal code doesn't work as one would expect"). <<

Thanks for your explanation. I suppose I'm finding it hard to think of obviously illegitimate WHY comments. Can you provide some examples?
Mark Pearce Send private email
Friday, June 01, 2007
 
 
Obviously illegitimate "why" comments?

Well, how about "why" comments that are in the wrong place -- that is, in the program itself.

Some comments, such as what a function or method takes, does and returns, belong in the manual. Others, like why a change was made and who made it, belong in the source control or configuration management system. Still others belong in the specification or other associated documentation.

In the old days, Unix programmers wrote manual pages and source code control systems whereas DOS and Windows programmers did not. So the latter group were more likely to use huge comment block headers to contain this information.

These days, Windows programmers do use these tools, along with IDEs arguably more advanced and certainly easier to use than emacs on Unix, so comment block headers can be reduced if not done away with completely.

Or that would be the case were it not for another trend: the use of tools like javadoc to automatically generate documentation from comments in the code. Too often this leads to poor quality manuals derived from obtuse comments that make the code harder to read.
John L Send private email
Saturday, June 02, 2007
 
 
A good comment explains the 'why' of a piece of code when the 'why' isn't obvious.

An illegitimate why comment occurs when

a) The why is transparently obvious to all, and the comment has no purpose.
b) The why isn't obvious, but that's entirely because the code being commented is so poor. In which case the code should be improved, and the comment will then not be necessary.
c) Your variables and functions are poorly named.

Legitimate why comments occur when either

a) The algorithm being commented is intrinsically complicated, or does something 'clever' that's worth keeping for some real performance reason.
b) You have to do something obscure to work around a problem in code over which you have no control.
c) Your language forces you to use short and obscure naming.
d) You wish to give a short explanation of what something does so that reading the code isn't necessary.
Duncan Sharpe
Saturday, June 02, 2007
 
 
So let's take these one by one.

>> Some comments, such as what a function or method takes, does and returns, belong in the manual. <<

You mean that you actually write a manual describing the code at this level of detail? In 3 decades, I've never done this.

>> Others, like why a change was made and who made it, belong in the source control or configuration management system. <<

Who made a change is a "who" comment, and we're discussing "why" comments.

Recording a "why" comment in the SCM system works for a single change. But as soon as you reach a decently-sized changeset, you're going to need a huge changeset comment to describe every "why". And you can't link each comment to the relevant code. So this only works if you use your SCM in a very artificial manner.

>> The why is transparently obvious to all, and the comment has no purpose. <<

Agreed.

>> The why isn't obvious, but that's entirely because the code being commented is so poor. In which case the code should be improved, and the comment will then not be necessary. <<

Can you come up with some examples of this?

>> Your variables and functions are poorly named. <<

Again, can you come up with some examples of renaming stuff to remove why comments?
Mark Pearce Send private email
Saturday, June 02, 2007
 
 
"Further, placing the critical section in it's own function as was suggested above makes the intent even MORE clear."

That will lead to a lot of single use static functions that will impair code readability.  For example I have a file that contains a list of functions like loadaccountslist() loadaccountstree() etc.  Each of these functions should do critical locks atleast three times (starting, middle and end) to synchronize multiple threads working on the same data.  Now if I were to create static functions to create scoped locking, each of these functions will become atleast 4 functions.  I have to name them with meaningless names like loadaccountsliststartlock() and the code will become difficult to read and modify.  Whereas scoped locking with empty braces with a warning comment will not impact code readability at all.
Donald Duck
Sunday, June 03, 2007
 
 
>> Again, can you come up with some examples of renaming stuff to remove why comments?

Hey Mark, several of my comment-elimination suggestions for PWills' example have to do with renaming stuff.

Do you disagree with those suggestions?  If so, then why do you disagree?  If not, then why don't these satisfy your request for examples?

Just curious.  It's entirely possible that my analysis was just too lengthy and you fell asleep reading it.  ;)
No comment
Sunday, June 03, 2007
 
 
>> Hey Mark, several of my comment-elimination suggestions for PWills' example have to do with renaming stuff. <<

I can't find a single "why" comment in PWIlls example. I'm trying to find examples where variables/functions are renamed to remove a "why" comment, as per Duncan's claim.
Mark Pearce Send private email
Sunday, June 03, 2007
 
 
+1 to jk for "even when everyone basically agrees they will continue to argue on teh internet"

+1 to John L for "Others, like why a change was made and who made it, belong in the source control or configuration management system."

+1 to No Comment for "But in response to allow me to show why I think this good example is actually still OVER-commented."

--

Also, let me say that I am not by *any* stretch suggesting my earlier 93 lines are Great Code. I was merely trying to show that you can communicate intent without 35% comments.
PWills Send private email
Wednesday, June 06, 2007
 
 
>> +1 to John L for "Others, like why a change was made and who made it, belong in the source control or configuration management system." <<

I disagree. This only works if you have a SCM commit for every change. If you commit multiple changes at a time, this won't work - it's simply not scalable.
Mark Pearce Send private email
Friday, June 08, 2007
 
 
I have to say that in general I'm a fan of comments (apart from the glaringly obvious ones).
Certainly comments are needed to clarify the intent of the code, and if developers don't keep those updated in line with code changes, they are just being lazy.

Whilst appropriate choice of class, variable and method names can help a lot, it relies on the assumption that when I read it, I'm thinking like the developer that wrote it - and it therefore introduces a risk that I am wrong in my interpretation of what the code does.

Also, with modern editors, code and comments are in different colours, so it's easy enough to scan code without being distracted by the comments and vice versa.
Bruce Hatton Send private email
Friday, June 08, 2007
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz