What's Wrong With Comments?

Source: comp.object
Date: 05-Oct-99

Problem: Proponents of Extreme Programming (XP) advocate removing most comments from code. Why?

[...] Comments are a sign of bad code in two ways. First of all, you add them to code that isn't clear without them. Good comments are comments that add clarity to code that isn't clear or isn't sufficiently clear without them. There's no point in belaboring the obvious or having a comment that fails to explain the nonobvious. Now imagine taking a good comment and removing it. You are now left with code that is bad, or at least not as good as it could be. In that limited sense, comments are a sign of bad code.

But let's keep in mind that comments are only a means to and end, namely clarity of the code. Rewriting unclear code is another means of achieving that end. All things being equal, extreme programmers prefer code that is clear by itself to code that is clear because of the comments. Surely that is not controversial. If we can refactor unclear code that needs comments for clarity into crystal clear code that doesn't, we refactor. If we find that even after refactoring the code still needs comments, we add them. It is just that we believe this is hardly ever. We believe that most comments can be made redundant by making an effort to make the code itself crystal clear. That is the controversial bit.

[...]

If we are faced with unclear code, we don't take what we feel to be the easy way out by just adding a comment. We'll first try to refactor the code until it's crystal clear. If it isn't crystal clear, we'll think long and hard about why we cannot make it crystal clear. Could it be that there is an abstraction missing? If we find the missing abstraction, we add it and make the code crystal clear. If despite our best efforts we cannot make the code readable, we add a comment as a last resort.

[...]

The XP rule isn't really "Don't write comments", but "Rewrite code that needs comments". Of course, that's easy to say and less easy to do. On the other hand, it is a learnable skill. [...]

Seen from another point of view XPers do write comments, but they try to put these into identifiers. If you try it, you'll see that most comments can be put into identifiers. We prefer that to ordinary comments because it is more readable, especially if you use a good class browser.

Let me give an example. Here is some code a student wrote on our SE practical course where we tried to do XP.

void HourRegistration::TestHTMLGeneration()
{
    SetupTestData();
    string page = Run("Jansen", "APR", "1999");
    ifstream correct_output_file("CorrectOutputForHourRegistration.html");
    string correct_output, magic_marker = "USERNAME";
    char c;
    while (correct_output_file.get(c))
        correct_output += c;
    int start_position = correct_output.find(magic_marker);
    if (start_position == string::npos)
    {
        cout << "Crisis in TestHtmlGeneration" << endl;
    }
    else
    {
        correct_output.replace(start_position, magic_marker.length(), getenv("USERNAME"));
        if (page != correct_output)
            cout << "Error in TestHTMLGeneration" << endl;/* <<
            "real output: " << endl << page << endl <<
            "correct output: " << endl << correct_output << endl;*/

        ofstream real_output("RealOutputForHourRegistration.html");
        real_output << page;
    }
}

These are the changes I've made to it so far. It can be improved further.

void ReadStringFromFile(string& the_string, char* filename)
{
    ifstream the_file(filename);
    char c;
    while (the_file.get(c))
        the_string += c;
}

void SearchAndReplace(string& whole, const string& search, const string& replace)
{
    int start_position = whole.find(search);
    if (start_position == string::npos)
        throw "not found";
    else
        whole.replace(start_position, search.length(), replace);
}

void DetermineCorrectOutput(string& correct_output)
{
    ReadStringFromFile(correct_output, "CorrectOutputForHourRegistration.html");
    SearchAndReplace(correct_output, "USERNAME", getenv("USERNAME"));
}

void HourRegistration::TestHTMLGeneration()
{
    SetupTestData();
    string page = Run("Jansen", "APR", "1999");

    string correct_output;
    DetermineCorrectOutput(correct_output);

    ofstream real_output("RealOutputForHourRegistration.html");
    real_output << page;

    if (page != correct_output)
        cout << "Error in TestHTMLGeneration" << endl;/* <<
        "real output: " << endl << page << endl <<
        "correct output: " << endl << correct_output << endl;*/
}

Instead of extracting the functions, I could have added comments, but I think this is more readable. The local variables in the original function now have two names: one as a local variable and actual parameter and one as a formal parameter. These need not be the same. You can give each a name that is clearest in its context. This way you can make both the code that was extracted into a little function and the code that surrounded it in the original function clearer.

More Info:

WikiWikiWeb Comment Costs And Benefits

Roedy Green, How To Write Unmaintainable Code

Martin Fowler, Kent Beck, John Brant, William Opdyke, Don Roberts, Refactoring: Improving the Design of Existing Code