Refactoring Code Comments Can Increase Maintainability

The assertion of this post is that code comments are a smell.  Creating obvious, readable code is more maintainable that unreadable code that is heavily commented.

Matt Hinze is teaching Agile Boot Camp, part 1 this week, and one of his lessons is about code readability.  He spoke about code comments being a smell.  When something smells, it doesn't guarantee that it is rotten, but it does create the need to examine more closely to see if any bacteria is beginning to invade the codebase.  In this case, code comments may be a smell or harbinger of maintainability problem.

Let's get the first straw-man out of the way.  Taking an existing codebase and merely deleting all code comments does not make a system more maintainable.   I'm not suggesting you do this for your existing system.  This course of action would be foolish.

When developing code, sometime one finds that a section is less than obvious.  When an ambiguous section of code is discovered, the wrong course of action is to add a comment describing what it does.  The better option is the refactor the code to make it more obvious. 

Problems with code comments:

  • Code comments are not compiled
  • Code comments are not tested
  • Code comments are not verified frequently
  • Out-of-date comments have negative consequences
  • They are likely to remain when the code changes

How, then, do we communicate to the maintainer the purpose of code that is less than obvious?  Consider the following example that takes some ambiguous code with comments and refactors away the comments.

public class SpeakerClickReport
{
    public int Total { get; set; }
    public int IndTotal { get; set; }

    // get ration of individual clicks to total clicks for a given speaker.
    public int GetRatio()
    {
        //Total clicks for all speakers equaling 0
        if (Total == 0) return 0;
        //take clicks for this speaker and get the percentage compared to Total for all speakers
        return (int) (100.0*IndTotal/Total);
    }
}

Notice that the comments help to communicate the intent of the code, although it isn't clear what this code is doing.  When this code changes, these comments will be out-of-date.  Let's consider a refactored example and compare the readability.

public class SpeakerClickReport
{
    public int TotalOfAllSpeakerClicks { get; set; }
    public int NumberOfIndividualClicks { get; set; }

    public int GetPercentageOfTotalForIndividualSpeaker()
    {
        if (TotalOfAllSpeakerClicks == 0)
        {
            return 0;
        }

        var ratioOfTotal = (double)NumberOfIndividualClicks / TotalOfAllSpeakerClicks;
        var percentFormOfRatio = (int) Math.Round(100 * ratioOfTotal);
        return percentFormOfRatio;
    }
}

Notice that none of the code comments are left.  In their place are:

  • More obvious variable names
  • Whitespace used to divide sections
  • More intermediate variables to describe single-step decisions and calculations
  • Public API more obvious
  • Obtaining return value before actually returning it

The second example is more maintainable than the first because as code is refactored, the comments are actually embedded in the code, not ignored by the compiler.  As class members are renamed and moved around, the contextual information will remain and be refined.  The risk of the commenting information being out-of-date and useless is mitigated.  In short, embedding more information into the source code creates more maintainable code.

Code will be read many, many more times than it is written.


Trackbacks

Arjan`s World » LINKBLOG for January 17, 2009 Posted on 1.17.2009 at 3:28 PM

Pingback from Arjan`s World » LINKBLOG for January 17, 2009

Weekly Link Post 77 « Rhonda Tipton’s WebLog Posted on 1.18.2009 at 8:57 PM

Pingback from Weekly Link Post 77 « Rhonda Tipton’s WebLog

Triple Shot Links # 16 | Caffeinated Coder Posted on 1.18.2009 at 9:20 PM

Pingback from Triple Shot Links # 16 | Caffeinated Coder

Limiting Code Comments Increases Maintainability Posted on 1.18.2009 at 11:05 PM

Thank you for submitting this cool story - Trackback from DotNetShoutout

Reflective Perspective - Chris Alcock » The Morning Brew #267 Posted on 1.19.2009 at 2:01 AM

Pingback from Reflective Perspective - Chris Alcock » The Morning Brew #267

Giraffe: Developer Posted on 1.19.2009 at 4:07 AM

Pingback from Giraffe: Developer

Commentless Code is not a Silver Bullet « Giraffe: Developer Posted on 1.19.2009 at 4:11 AM

Pingback from Commentless Code is not a Silver Bullet « Giraffe: Developer

Invalid Argument » Limiting Code Comments Increases Maintainability Posted on 1.19.2009 at 4:27 AM

Pingback from Invalid Argument » Limiting Code Comments Increases Maintainability

igorbrejc.net » Fresh Catch For January 19th Posted on 1.19.2009 at 3:01 PM

Pingback from igorbrejc.net » Fresh Catch For January 19th

Interesting Finds: 2009 01.18~01.21 Posted on 1.20.2009 at 7:37 PM

Web Intro to Caching,Caching algorithms and caching frameworks part 1 - part 2 Readings in Distributed

Interesting Finds: 2009 01.18~01.21 | Web Hosting and Domains Posted on 1.22.2009 at 12:57 PM

Pingback from Interesting Finds: 2009 01.18~01.21 | Web Hosting and Domains

Weekly #0 (beta) | loosely coupled Posted on 1.22.2009 at 1:34 PM

Pingback from Weekly #0 (beta) | loosely coupled

Comments

josh said on 1.16.2009 at 4:13 PM

Your refactored comments are *still* ignored by the compiler. It will not reject "pi = 16;"

Jeffrey Palermo said on 1.16.2009 at 4:21 PM

@josh,

The information is better embedded in the code; therefore, as the code changes, the information is updated with it.

Aaron Erickson said on 1.16.2009 at 5:00 PM

Ok, point taken, but what if the comments are at least humorous, like

//Please don't ever do this, I am only doing this because I have no soul

.. or

//this is black magic, you were not meant to ever understand it

May not be useful, but at least adds humor to code reviews :)

josh said on 1.16.2009 at 9:06 PM

That's a pleasant assertion, but based on the code I've seen over the years, I don't believe it. If you're not taking enough care to update comments, you'll probably do something sloppy regardless.

I do note, however, that any given name will at least be right or wrong everywhere. And your IDE is likely to help you fix all of them at once.

Lee Brandt said on 1.17.2009 at 12:31 PM

Point well taken. I'd also like to add that using BDD specifications also serve as documentation that is never out of date. If comments become out of date, they lie to you and there is nothing letting you know that they're lying. If your specs are out of date then your build will not pass. Readable code and good BDD specs are the best way to document your code.

Great post. Keep em coming. :0)

~Lee

Jef claes said on 1.19.2009 at 5:22 AM

Nice article!

Wesley said on 1.21.2009 at 8:16 AM

And how about code documentation? Do you consider this:

-------------------------------------------------------------------------------

/// <summary>

/// Gets a value indicating whether to render this user control on layouts pages.

/// </summary>

/// <value>

/// <c>true</c> to render on layouts pages; otherwise, <c>false</c>.

/// </value>

protected virtual bool RenderOnLayoutsPage {

get {

return true;

}

}

-------------------------------------------------------------------------------

as commented code or documented code? And would recommend against this as well?

Cheers,

Wes

Jeffrey Palermo said on 1.21.2009 at 8:38 AM

@Wesley,

If the goal is to use Sandcastle to generate API documentation from the source files, then this works well.

I'm not recommending against comments in general, only comments that cannot be made into method names or better variable.

API documentation is a separate concern.

Corey Haines said on 1.23.2009 at 3:07 PM

I'm very encouraged seeing these ideas, and this one in particular, making their way into the mainstream. Thanks for a good post introducing this thought and the techniques surrounding into a much wider audience than it was previously confined to.

As you relay, it is very important to remember that eliminating comments is not a matter of deleting existing comments, but rather a way to reflect on the reasoning behind their existence in the first place. Code smells are not used as a means to find a better smelling aerosol cover-up (just deleting the comments, for example), but, rather, to look for the root of the smell, itself (often a lack of adherence to good design principles).

Through the last couple years, as we've discussed the idea of comments as code smells (I'm a bit less diplomatic and generally give the heading 'comments are evil'), it has become apparent that, other than a very specific, rarely occurring edge-case around buggy third-party APIs that someone gave me, comments are at best a violation of SRP and at worst a piece of live munition in the maintainability of the code.

Corey Haines said on 1.23.2009 at 3:08 PM

It is important, as you write in your response to @Wesley, to differentiate between comments and publicly-exposed API documentation that happens to be in the same format as a comment.