Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I used to be a fan of requiring XML comments for documentation. I've since changed my mind for two main reasons:

  1. Like good code, methods should be self-explanatory.
  2. In practice, most XML comments are useless noise that provide no additional value.

Many times we simply use GhostDoc to generate generic comments, and this is what I mean by useless noise:

    /// <summary>
    /// Gets or sets the unit of measure.
    /// </summary>
    /// <value>
    /// The unit of measure.
    /// </value>
    public string UnitOfMeasure { get; set; }

To me, that's obvious. Having said that, if there were special instructions to include, then we should absolutely use XML comments.

I like this excerpt from this article:

Sometimes, you will need to write comments. But, it should be the exception not the rule. Comments should only be used when they are expressing something that cannot be expressed in code. If you want to write elegant code, strive to eliminate comments and instead write self-documenting code.

Am I wrong to think we should only be using XML comments when the code isn't enough to explain itself on its own?

I believe this is a good example where XML comments make pretty code look ugly. It takes a class like this...

public class RawMaterialLabel : EntityBase
{
    public long     Id                      { get; set; }
    public string   ManufacturerId          { get; set; }
    public string   PartNumber              { get; set; }
    public string   Quantity                { get; set; }
    public string   UnitOfMeasure           { get; set; }
    public string   LotNumber               { get; set; }
    public string   SublotNumber            { get; set; }
    public int      LabelSerialNumber       { get; set; }
    public string   PurchaseOrderNumber     { get; set; }
    public string   PurchaseOrderLineNumber { get; set; }
    public DateTime ManufacturingDate       { get; set; }
    public string   LastModifiedUser        { get; set; }
    public DateTime LastModifiedTime        { get; set; }
    public Binary   VersionNumber           { get; set; }

    public ICollection<LotEquipmentScan> LotEquipmentScans { get; private set; }
}

... And turns it into this:

/// <summary>
/// Container for properties of a raw material label
/// </summary>
public class RawMaterialLabel : EntityBase
{
    /// <summary>
    /// Gets or sets the id.
    /// </summary>
    /// <value>
    /// The id.
    /// </value>
    public long Id { get; set; }

    /// <summary>
    /// Gets or sets the manufacturer id.
    /// </summary>
    /// <value>
    /// The manufacturer id.
    /// </value>
    public string ManufacturerId { get; set; }

    /// <summary>
    /// Gets or sets the part number.
    /// </summary>
    /// <value>
    /// The part number.
    /// </value>
    public string PartNumber { get; set; }

    /// <summary>
    /// Gets or sets the quantity.
    /// </summary>
    /// <value>
    /// The quantity.
    /// </value>
    public string Quantity { get; set; }

    /// <summary>
    /// Gets or sets the unit of measure.
    /// </summary>
    /// <value>
    /// The unit of measure.
    /// </value>
    public string UnitOfMeasure { get; set; }

    /// <summary>
    /// Gets or sets the lot number.
    /// </summary>
    /// <value>
    /// The lot number.
    /// </value>
    public string LotNumber { get; set; }

    /// <summary>
    /// Gets or sets the sublot number.
    /// </summary>
    /// <value>
    /// The sublot number.
    /// </value>
    public string SublotNumber { get; set; }

    /// <summary>
    /// Gets or sets the label serial number.
    /// </summary>
    /// <value>
    /// The label serial number.
    /// </value>
    public int LabelSerialNumber { get; set; }

    /// <summary>
    /// Gets or sets the purchase order number.
    /// </summary>
    /// <value>
    /// The purchase order number.
    /// </value>
    public string PurchaseOrderNumber { get; set; }

    /// <summary>
    /// Gets or sets the purchase order line number.
    /// </summary>
    /// <value>
    /// The purchase order line number.
    /// </value>
    public string PurchaseOrderLineNumber { get; set; }

    /// <summary>
    /// Gets or sets the manufacturing date.
    /// </summary>
    /// <value>
    /// The manufacturing date.
    /// </value>
    public DateTime ManufacturingDate { get; set; }

    /// <summary>
    /// Gets or sets the last modified user.
    /// </summary>
    /// <value>
    /// The last modified user.
    /// </value>
    public string LastModifiedUser { get; set; }

    /// <summary>
    /// Gets or sets the last modified time.
    /// </summary>
    /// <value>
    /// The last modified time.
    /// </value>
    public DateTime LastModifiedTime { get; set; }

    /// <summary>
    /// Gets or sets the version number.
    /// </summary>
    /// <value>
    /// The version number.
    /// </value>
    public Binary VersionNumber { get; set; }

    /// <summary>
    /// Gets the lot equipment scans.
    /// </summary>
    /// <value>
    /// The lot equipment scans.
    /// </value>
    public ICollection<LotEquipmentScan> LotEquipmentScans { get; private set; }
}
share|improve this question
2  
I would argue XML is a terrible choice for this purpose. It's way too verbose and general for the use at hand. Check out reStructuredText and sphinx for a markup language that embeds into comments without making them unreadable. –  Lattyware Oct 22 '12 at 15:24
1  
@Lattyware: VisualStudio supports this style by default, no extra plugins or tools are necessary. Comments generated this way are immediately visible in pop-up tooltips. –  FrustratedWithFormsDesigner Oct 22 '12 at 16:25
    
@FrustratedWithFormsDesigner I would say that getting a plugin is worth it to make your code far more readable. Built-in support for reST with tooltips like that is in PyCharm, so I'm sure plugins exist for other languages in other IDEs. Obviously if you have a project where everything is documented in this way, I'm not suggesting you start splitting the way it's done, but for new projects, I just think it's so horrible to read and maintain. –  Lattyware Oct 22 '12 at 18:27

3 Answers 3

up vote 18 down vote accepted

If your comments only look like this:

/// <summary>
/// Gets or sets the sublot number.
/// </summary>
/// <value>
/// The sublot number.
/// </value>

Then yes, they are not all that useful. If they read something like this:

/// <summary>
/// Gets or sets the sublot number.
/// Note that the sublot number is only used by the legacy inventory system.
/// Latest version of the online inventory system does not use this, so you can leave it null. 
/// Some vendors require it but if you don't set it they'll send a request for it specifically.
/// </summary>
/// <value>
/// The sublot number.
/// </value>

Then I'd say they have value. So to answer your question: Comments are necessary when they say something that the code does not say.

An exception: it is good to have comments on anything that is publicly accessible if you're writing a library/API that will be available to the public. I hate using a library and seeing a function named getAPCDGFSocket() with no explanation of what an APCDGFSocket is (I'd be happy with something as simple as This gets the Async Process Coordinator Data Generator File Socket). So in that case, I'd say use some tool to generate all comments and then manually tweak the ones that need it (and please make sure your cryptic acronyms are explained).

Also, getters/setters are generally bad examples for "are comments necessary?" because they are usually quite obvious and comments aren't necessary. Comments are more important on functions that perform some algorithm where some explanation of why things are being done they way they are could make the code much more understandable and also make it easier for future programmers to work with.

...and finally, I'm pretty sure that this question is relevant for all styles of comments, not just those that are formatted using XML (which you are using because you're working in a .NET environment).

share|improve this answer
2  
+1 - GhostDoc is a starting point for me to turn the first version, which is boilerplate, into the second version, which contains detailed domain knowledge. –  Jesse C. Slicer Oct 22 '12 at 14:29
4  
+1 for the why part. The DRY principle applies - do not repeat yourself, and if the code already does a pretty good job of describing the what part, your comments should focus on explaining something else (typically the why). –  Daniel B Oct 22 '12 at 14:41
    
@DanielB or maybe you don't need comments at all ;) I agree for the most part with this answer except the word necessary in "Comments are necessary when they say something that the code does not say." I think if the code says everything needed then you don't need more information in comments even if the comments give information not in code. –  Jimmy Hoffa Oct 22 '12 at 15:06
1  
@DanielB - XML Comments in .NET are primarily intended for situations where the end user programmer of a library or service doesn't have the source code available to them. –  jfrankcarr Oct 22 '12 at 16:00
2  
@Lattyware - XML comments integrate seamlessly with Visual Studio's Intellisense, a big time saver as compared to looking up stuff in a separate document. –  jfrankcarr Oct 22 '12 at 18:41

The comments that look useless to users who can read the code become rather useful to users who have no access to the source. This happens when the class is used as an external API by people outside your organization: the HTMLs generated from your XML docs is their only way to learn about your classes.

That said, a comment that reiterates the method name with added spaces between the words remains useless. If your class is going to be used outside of your organization, you need to document at lest the valid ranges for your values. For example, you should say that setting UnitOfMeasure to null is illegal, that the value supplied to the setter must not contain spaces at the beginning or at the end of the string, and so on. You should also document the range of LabelSerialNumber if it differs from that of a plain Int32: perhaps it does not allow negative numbers*, or does not allow more than seven digits. Your internal users may take it for granted, because they look at serial numbers day in and day out, but the external users may be genuinely surprised to see an exception from what looks like an innocent setter.


* ...in which case uint may be a better choice

share|improve this answer
1  
It's not just for when you don't have the source. If your editor can parse them (like Visual Studio does with Xml Comments), they can give information as mouseover/popups without requiring you to navigate to a different file. A 1 line range validator that limits an int to a narrower range is obvious when you go to the file where the setter is implemented; but having "FrobableID must be be between 0 and 1000" appear when you start typing "myFrobable.Fro..." and the autocomplete kicks in us a helpful reminder. –  Dan Neely Oct 22 '12 at 15:49

You are absolutely right about avoiding such useless comments. They make reading the code more difficult instead of making it easier, and are taking too much space.

In my practice people who write comments with getters/setters, tend to omit comments when those are really necessary (like building a 20-line sql-query for a component with no documentation).

I write comments when there are some other obvious solutions _ I indicate why exactly this approach has been used. Or when its hard to get the idea without knowing all of the details _ I briefly list the details that are necessary to understand the code.

The example you bring is more of writing comments to say that one writes comments rather than making others' (and theirs also) life easier.

BTW you can improve your ability of writing comments by returning to your old code and trying to comprehend it (you might even not recognize your own code in 2-3 months _ its absolutely like reading someone else's code). If you do this painlessly, than everything's just fine.

share|improve this answer
    
I don't know anyone who goes to effort to write comments on getters/setters anymore. If you're using almost any modern IDE (and even advanced text editors can support this via plugins), getters and setters can usually be documented very easily with a mouse-click or two, or the right keystroke (if it's been configured). Sometimes they're generated automatically when you generate code based on a database schema or WSDL... –  FrustratedWithFormsDesigner Oct 22 '12 at 14:36
    
@FrustratedWithFormsDesigner, the person I was talking about was to leave the company, and I believe all those comments on getters/setters were done by that person to show that he/she did some effort to leave some documentation –  superM Oct 22 '12 at 14:39
    
Were all the bogo comments entered after the person gave notice? I've seen people create empty/useless xml comments all over the place as a boneheaded way to stop VS from generating "Missing xml comment on publicly visible Foo" warnings. –  Dan Neely Oct 22 '12 at 20:20
    
@Dan Neely, I guess that person didn't really care and just added comments to say that comments are added. We usually don't pay much attention to comments, but if someone is to leave and is working on a component, it is a must to write clear readable code. –  superM Oct 23 '12 at 10:33

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.