A Guide to Commenting Your Code

I spend a lot of time working with code that someone else wrote. The code has lots of comments, but they actually do little to improve the understandability of the work. I’m here to provide a concise set of examples to demonstrate the proper way to comment your code so that those who follow will be able to understand it easily and get to work.

These examples are in php, but the principles transcend language.

WRONG:

// get the value of the thing
$val = gtv();

RIGHT:

$thingValue = getTheValueOfTheThing();

WRONG:

// get the value of the thing
$val = getTheValueOfTheThing();

RIGHT:

$thingValue = getTheValueOfTheThing();

Oh so very WRONG:

// Let's get the value of the thing
$val = getTheValueOfTheThing();

We’re not pals on an adventure here.

RIGHT:

$thingValue = getTheValueOfTheThing();

You might have noticed that so far all my examples of the proper way to comment your code don’t have comments at all. They have code that doesn’t need a comment in the first place.

Computer languages are not created to make things easier to understand for the machine, they are to make sets of instructions humans can read that (secondarily) tell the computer what to do. So, if the code syntax is for the benefit of humans, treat it that way.

If you have to write a comment to explain what is going on in your code, you probably wrote it wrong. Or at the very least, if you need to write a comment, it means you’re not finished. I write many comments that start TODO, which my tools recognize and give me as a to-do list.

Stopping to come up with the perfect name for a variable, class, or function is an important part of programming. It’s more than a simple label, it’s an understanding of what that symbol means, and how it works in the system. If you can’t name it, you’re not ready to code it.

There is a special category of comments in code called doc blocks. These are massive comments above every function that robots can harvest to generate documentation. It’s a beautiful idea.

Here’s my world (not a standard doc block format but that’s irrelevant):

/*
|--------------------------------------------------------------------------
| @name "doSomething"
|--------------------------------------------------------------------------
| @expects "id (int)"
|--------------------------------------------------------------------------
| @returns "widget"
|--------------------------------------------------------------------------
| @description "returns the widget of the frangipani."
|--------------------------------------------------------------------------
*/
public function doSomething($id, $otherId) {
    $frangipani = getFrangipani($id);
    multiplex($frangipani, $otherId);
 
    return $frangipani->widgets();
}

The difficulty with the above is that the laborious description of what the function does is harmfully wrong. The @expects line says it needs one parameter, when actually it needs two. It says it returns a widget but in fact the function returns an array of widgets. If you were to try to understand the function by the doc block, you would waste a ton of time.

It happens all the time – a programmer changes the code but neglects to update the doc block. And if you’re not using robots to generate documentation, the doc block is useless if you write your code well.

public function getFrangipaniWidgets($id, $multiplexorId) {
    $frangipani = getFrangipani($id);
    multiplex($frangipani, $multiplexorId);
 
    return $frangipani->widgets();
}

Doc blocks are a commitment, and if you don’t have a programmer or tech writer personally responsible for their accuracy, the harm they cause will far surpass any potential benefit.

I have only one exception to the “comments indicate where you have more work to do” rule: Don’t try this at home.

public function getFrangipaniWidgets($id, $multiplexorId) {
    $frangipani = getFrangipani($id);
 
    // monoplex causes data rehash, invalidating the frangipani
    multiplex($frangipani, $multiplexorId);
 
    return $frangipani->widgets();
}

This is useful only when the obvious, simple solution to a problem had a killing flaw that is not obvious. This is a warning sign to the programmer coming after you that you have tried the obvious. Often, when leaving notes like this, and explaining why I did something the hard way, I realize that the easy way would have worked after all. At which point I fix my code and delete the comment. But at least in that case the comment did something useful.

2

Time Not Well-Spent

Here it is, Whiskey-Exemption Thursday, and my weight is on-target so I can even have beer. The purpose of Thursday is to devote an evening to pushing the writing forward, and hang the consequences.

What have I been writing this fine evening? I’ve been trying to come up with the least-objectionable way to emulate Swift’s extensions to Protocols in php. The answer: there is no way.

Begin geek

Coding with php is coding with flint knives and bearskins; the power of php is in its wham-bam-thank-you-ma’am ability to do a quick task and then to go away.

Bless the movers behind php, they’re trying to evolve their language to catch up with the way people are using it these days. If they had known Drupal was coming along, they might not have been so quick-and-dirty before. Drupal might be slightly less awful as a result.

There are design patterns enabled by Swift that I get a little misty contemplating. Being able to add extensions (with executable code!) to protocols is enormously powerful. Having experienced that, I wanted to do the same thing in php, creating a trait “taggable” and having classes that used it automatically injected with the implementation. Injected, not inherited. Ain’t gonna happen.

End geek

At least now I’m writing prose about writing the code rather than writing the code itself. Progress, I guess.

1

Defensive Programming: Put the Guards Near the Gate

We can file this one under “not interesting to pretty much anyone who reads this blog,” but it’s an important concept for writing robust code. This is part of a discipline called Defensive Programming.

Let’s say you build yourself a castle in a clearing in the woods. There is one path to the front gate, and you need to guard it. “Hah!” you think, “I’ll put the guards where the path comes out of the woods, to stop shenanigans before they even get close!” You post the guards out there in a little guardhouse, secure in the knowledge that no bad guys will reach your gate.

Until someone makes a new path. Perhaps when the new path is created the path-maker will notice that there are guards on the other path and put a little guardhouse on the new path as well. But perhaps not.

In software, it’s the difference between code that says, “when all conditions are right, call function x”, and having function x test to make sure everything is OK before proceeding.

Putting the guard by the trees:

    function x(myParameter) {
        myParameter.doSomething();
    }

    thing = null;

    ... other stuff that might or might not set 'thing'

    if (thing != null) {
        x(thing);
    }

This is fine as long as everything that calls function x knows to check to make sure the parameter is not null first. It might even seem like a good idea because if ‘thing’ is not set you can save the trouble of calling the function at all. But if some other programmer comes along and doesn’t know this rule, she might not do the check.

    // elsewhere in the code...

    anotherThing = null;

    ... other stuff that might or might not set 'anotherThing'

    x(anotherThing); // blammo!

Better to move the guards close to the gate:

    function x(myParameter) {
        if (myParameter != null) {
            myParameter.doSomething();
        }
    }

Now when someone else writes code that calls function x, you can be confident that your guards will catch any trouble. That doesn’t mean you can’t ALSO put guards out by the edge of the forest, but you shouldn’t rely on them.

Maybe if I Assert Harder…

When I unleash the testing robots on my code, I very often see messages like this:

Failed asserting that false is true.

It seems like there should be a metaphor in there somewhere.

Pretty Badass…

I just put the following comment in my code:

// now SHIT GETS REAL

1

Haloscan comments to WordPress – the nitty gritty.

As I mentioned in the previous episode, I recently had to move more than 8000 comments from my old comment system, Haloscan, and import them into WordPress. Haloscan served me well back in the day, but they are going away, and all my more recent comments are in the WordPress system anyway. Nice to have them all in one place.

The process turned out to be pretty easy. I found a script for importing comments from a different system, modified it, modified it some more, found a fundamental problem with it, fixed that, and in the end not much of code remained from the example, except the part where the WordPress logo is displayed on the screen. I assume that part came from the code the guy copied to make the code that I copied.

Along the way I learned a couple of things. PHP is a pretty flexible language, but running a loop that sets up 8500 data structures and runs 25500 database queries exposes PHP’s primary weakness: memory management. The whiz kids who invented PHP designed it for a load/compile/execute/exit-and-clean-up flow. Memory allocated during execution is cleaned up when the program is done running (usually when the Web page is delivered). When you try to do heavy lifting with PHP, you have to start paying attention to getting your memory back before the traditional clean-up time.

The code I started with did a direct database query to add the comment to the comments table, but that got things out of sync with other tables. (The posts table keeps track of the number of comments that apply to it, presumably for performance reasons.) I dug into the core WordPress code and found the method they call to post comments, and I made my code call that function. I have no idea what all the bookkeeping chores are that function does, and really I don’t care as long as they get done.

I didn’t worry about performance too much at first (after all, it only has to run once), but one of the database queries I did was really expensive (scanning all the posts for a specific set of characters). Even running on my local server it was slow, and I knew that if I tried something like that on my actual Web host alarms would go off and they’d shut me down for a while. I did a little optimization on that front, and it was enough.

The following script has some Muddle-specific code in it, but it might come in handy for others who need to move Haloscan comments to a new system. The part that parses Haloscan XML is pretty generic and would work for anyone, the part that saves the comments might be useful as a guide as well. The main difference others will have to deal with is where to get proper post_id based on the thread field in the XML. In my case I had a link in each blog episode back to the Haloscan thread.

The HTML bit in the middle of the file is not essential; but it puts a nice WordPress logo on the screen when the script starts up. I inherited that from the script I started with.

NOTE: While this script has code in it specific to me, I am available to customize it for others who need to move their code from Haloscan into another environment, or, for that matter, from any structured source into WordPress. Drop me a line!

<?php
 
if (!file_exists('../wp-config.php')) die("There doesn't seem to be a wp-config.php file. You must install WordPress before you import any comments.");
require('../wp-config.php');
 
function saveCommentToWP($comment, $dbRef, &$postThreads) {
    //echo "here's where the comment save happens <br/><br />";
    $thread = $comment['thread'];
    $postID = $postThreads[$thread];
    if (!isset($postThreads[$thread])) {
        $query = "SELECT * FROM wp_posts WHERE post_content LIKE '%".$thread."%' AND post_status='publish'";
        $postID = $dbRef->get_var($query, 0);
        $postThreads[$thread] = $postID ? $postID : 0;
        if ($postThreads[$thread] == 0)
            echo ("<br />Thread $thread has no post!");
        else
            echo "<br />Thread $thread";
        flush();       // got to have real-time updates!
    }
 
    if ($postID && $postID != 0) {
        $userId = $comment['email'] == '[email protected]' ? 1 : 0;
 
        //set up the data the way wp_insert_comment expects it.
        $wp_commentData = array();
        $wp_commentData['comment_post_ID'] = (int) $postID;
        $wp_commentData['user_id'] = (int) $userId;
        $wp_commentData['comment_parent'] = 0;
        $wp_commentData['comment_author_IP'] = $comment['ip'];
        $wp_commentData['comment_agent'] = 'Haloscan';
        $wp_commentData['comment_date'] = $comment['datetime'];
        $wp_commentData['comment_date_gmt'] = $comment['datetime'];
        $wp_commentData['comment_approved'] = '1';
        $wp_commentData['comment_content'] = $comment['text'];
        $wp_commentData['comment_author'] = $comment['name'];
        $wp_commentData['comment_author_email'] = $comment['email'];
        $wp_commentData = wp_filter_comment($wp_commentData);
 
        $comment_ID = wp_insert_comment($wp_commentData);
 
        //echo ("<strong>saved comment $comment_ID</strong>");
    }
 
    // try to reclaim some memory
    unset($wp_commentData);
    unset($comment);
}
 
header( 'Content-Type: text/html; charset=utf-8' );
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<title>WordPress &rsaquo; Import Comments from RSS</title>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<style media="screen" type="text/css">
    body {
        font-family: Georgia, "Times New Roman", Times, serif;
        margin-left: 20%;
        margin-right: 20%;
    }
    #logo {
        margin: 0;
        padding: 0;
        background-image: url(http://wordpress.org/images/logo.png);
        background-repeat: no-repeat;
        height: 60px;
        border-bottom: 4px solid #333;
    }
    #logo a {
        display: block;
        text-decoration: none;
        text-indent: -100em;
        height: 60px;
    }
    p {
        line-height: 140%;
    }
    </style>
</head><body> 
<h1 id="logo"><a href="http://wordpress.org/">WordPress</a></h1> 
 
<?php
 
// Bring in the data
$reader = new XMLReader();
if ($reader->open('export-8.xml')) {
    $postThreads = array();
    $thread = '';
    while ($reader->read()) {
        //echo "<br />read node type: ".$reader->nodeType.';     '.$reader->name.': '.$reader->value;
        if ($reader->nodeType == XMLReader::ELEMENT && $reader->name == 'thread') {
            $thread = $reader->getAttribute('id');
        }
        if ($thread) {
            if ($reader->nodeType == XMLReader::ELEMENT && $reader->name == 'comment') {
                // begin building comment
                $comment = array('thread' => $thread);
                $reader->read();
                while ( !($reader->nodeType == XMLReader::END_ELEMENT && $reader->name == 'comment') ) {
                    if ($reader->nodeType == XMLReader::ELEMENT) {
                        $property = $reader->name;
                        $reader->read(); // assumes text element following element tag has the data
                        $comment[$property] = $reader->value;
                    }
                    $reader->read();
                }
                saveCommentToWP($comment, $wpdb, $postThreads);
            }
        }
    }
    $reader->close();
}
 
?>
 
 
</body>
</html>

1