Refactoring Reflections

In Don’t Make Me Think, Steve Krug presents the premise that the better a web site’s usability, the more brain cycles the visitor is able to focus on the substance (or intention) of the site. Said another way, the less brain cycles a visitor spends figuring out how a site works (e.g. how to navigate around), the more brain cycles he has available to concentrate on site’s substance. Don’t Make Me Think’s goal is to help the reader refine his web site’s usability to the point that visitors don’t need to think about how to use the site and so can focus all their attention on the information the site was built to convey.

While working through a recent refactoring project, it occurred to me that Don’t Make Me Think’s premise closely parallels one of the main motivations behind refactoring. The better the code base’s quality, the more mental energy the code reader can focus on the intention of the program. The less brain cycles a developer spends figuring out how a bit of code does what it does, the more brain cycles he can spend focusing on what the code does.

Code that made perfect sense when I first wrote it, when I was saturated in the project at hand, doesn’t look nearly so clear when I come back to look at it later. If there is a way I can bring clarity so that future re-readings require less thinking to process, superb! Other times, I structured the code under review the best I could with the skills I had at the time I wrote it. With the passing of time and the growth of my skills, the code I once thought polished now looks a bit more like a collection of rough edges. If I can bring refinement that simplifies and clarifies, excellent!

Often, refactoring is made up of simple changes that incrementally reduce the amount of thinking required to comprehend a code base.

During a recent project, I found a number of cases where a guard condition’s control structure wrapped an entire method.

public function doSomething() {
    if ($this->data) {
        // Numerous lines of code…
        //
        // …and even more code…
        //
        // …there might even be an “else” hidden somewhere in here.
    }
}

When I’d encounter one of these, I had to spend effort tracing the if statement’s braces. Did they really wrap the entire method or did they end somewhere in the middle? Was there an else clause hidden somewhere in the method that I would miss unless I thoroughly traced the braces?

By inverting the guard condition and having it return immediately, I eliminated the need for this extra thinking. In the example below, the guard condition and its associated action fit on one line. Determining what functionality is executed when the guard condition is true is now self-obvious even when quickly skimming over the source code.

public function doSomething() {
   if (!$this->data) { return; }

   // Numerous lines of code...
   // …
   // …
   // ...and even more code.
}

Another potential for refinement I came across had to do with passing unnecessary variables. A particular class that processed a single order item was passed both item and order objects. Examining this section of code required me to think about the order object. Presumably, it represented the order associated with the item but was that assumption correct?

public class ItemProcessor {
    public function __construct($order, $item) {
       $this->order = $order;
       $this->item = $item;
       // …
    }
    // …
}

Changing the item processor to fetch the associated order from the item passed to it both clarified the identity of the order object and simplified the coupling between the calling and called code.

public class OrderProcessor {
    // …
    public function Process() {
        foreach ($this->items as $item) {
            /* 
            Previously, something like 
            $processor = new ItemProcessor($this->order, $item);
            was required.
            */
            $processor = new ItemProcessor($item);
            // do something with $processor….
        }
   }
}

public class ItemProcessor {
    public function __construct($item) {
        $this->order = $item->getOrder();
        $this->item = $item;
        // …
    }
    // …
}

The result saves future code-readers a few brain cycles and it eliminates the possibility that the wrong order object will be passed because the order is no longer being passed.

If complex code can’t be simplified outright, it still may be possible to make it easier to comprehend by breaking its complexity into parts. Each part can be placed in a separate method that’s given a descriptive (self-documenting) name.

During a recent project, I wrote something like this:

if (
   (!$this->renderer->isShipmentSeparately() && $item != $this->parentItem) ||
   ($this->renderer->isShipmentSeparately() && $item == $this->parentItem)
) {
    // do something…
}

A bit of thinking is required to decipher this code fragment’s meaning. It’s certainly not obvious at a glance. The reader must think through the ramifications of each part of the conditional to determine the intention of the conditional (in plain English, what is it checking).

Separating the how and the what (the intention) makes this code much easier to understand. The reader can now focus on comprehending one aspect at a time.

if (!$this->canHaveQuantity($item)) {
    // do something…
}
// …

private function canHaveQuantity($item) {
    return (
        (!$this->renderer->isShipmentSeparately() && $item == $this->parentItem) ||
        ($this->renderer->isShipmentSeparately() && $item != $this->parentItem)
    );
}

Skimming over quantityShipped() after this refactoring, the reader doesn’t have to think through a complex conditional expression to understand the intended functionality—checking if an item can have a quantity. If the logic used to answer that question is of interest, it can be analyzed separately. How the program does what it does and the intention of what the program is trying to do have been separated.

Reiteratively applying the practice of refactoring brings my code closer and closer to the point where the purpose of my code is self-obvious—where the reader can clearly and easily focus on the intent of what my code is doing without spending brain cycles deciphering the mechanics of how my code does what it does.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>