Are XML Comments Necessary Documentation?

Question

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; }
}

Answer 1

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).

Answer 2

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

Answer 3

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.