Practical PHP Refactoring: Replace Temp with Query

Here's a scenario: you're using a local variable to save the result of an expression, such as a comparison, an arithmetic operation, or a concatenation of strings.

In many cases, you can extract this code and put it into a new method without harming functionality: the Replace Temp with Query refactoring is a specialization of Extract Method, which acts on very little code. Query is the name we give to this method: we're not talking about databases.

Why should we extract a Query method?

A subtle motivation is suggested by Fowler in the related chapter of his Refactoring book: the scope of a local variable is limited to the current method code in most languages (that's why is called local); this is true also for PHP. Thus keeping information in local variables encourages the method to get longer and longer: it's the only way to reach the local variable. Introducing a method lets you move the code that uses the variable in any other method of the current class, by calling the method on $this.

Replace Temp with Query is used also as propedeutic to other generic Extract Method refactorings: local variables substituted by queries do not become parameters to pass to the brand new methods, so the extraction is simple.

Another use case for this pattern is giving a name to an operation: $this->price * $this->vatRate become $this->getTax(), by far more readable and with the advantage of avoiding a comment.

Constraints

There are some constraints to make this refactoring work quickly, and without involuntary red bars. After all, if extracting a single local variable is going to break a lot of code, the ROI of this refactoring is very low.

• The expression must be free of side-effects: the only result should be assigning the temp.
• The temp must be assigned only once, otherwise your find&replace (automated or manual) will substitute too much. This is very often the case with minimal coding standard rules, but you never know how many spaghetti can fit in a method's local scope.

A clarification: this refactoring is easy only when the local variable is already computed as a function of the object's state (function in the mathematical sense, not programming one). Computing it from input values is a bit trickier, since you have to keep around as local the input values... and that means maybe it's just time to extract objects that represent these input parameters, not methods. We'll see that further in the series.

Steps

1. Find the variable: it should be assigned to only once, with a self-contained expression.
2. Extract the right-hand side of the assignment, in a new method; don't touch the temp for now. Run the appropriate tests (unit level should suffice for intermediate steps.)
3. Replace occurrences of the temp after the assignment and before the end of the scope with a method call: the Query.

The method should normally be private, and its inputs should be values which are not already on $this; the return value is simply the former temp value.

In many guides you find Compile as a step that ensures basic correctness, but PHP is for the most part a dynamic language, with on-the-fly compilation; the only static check you can do is a lint with php -l. In our case and for many dynamic languages, the test suite is our compiler.

Example

The code sample shows you the evolution of a temp variable $username:

<?php
class LoginFormTest extends PHPUnit_Framework_TestCase
{
    public function testFormDisplaysOldUsernameValue()
    {
        $form = new LoginForm(array('username' => 'giorgio'));
        $html = $form->__toString();
        $this->assertTrue((bool) strstr($html, '<input type="text" name="username" value="giorgio" />'));
    }
}


/**
 * For brevity, we implement only the code necessary for the current test 
 * to pass.
 */
class LoginForm
{
    private $data;

    public function __construct(array $data = array())
    {
        $this->data = $data;
    }

    public function __toString()
    {
        if (isset($this->data['username'])) {
            $username = $this->data['username'];
        } else {
            $username = '';
        }
        $this->doSomethingWithTheUsername($username);
        return "...<input type=\"text\" name=\"username\" value=\"$username\" />...";
    }

    private function doSomethingWithTheUsername($username)
    {
        // this method is only here to provide an example of code
        // that will start utilizing the Query instead of the Temp
    }
}

to an extracted method:

/**
 * For brevity, we implement only the code necessary for the current test 
 * to pass.
 */
class LoginForm
{
    private $data;

    public function __construct(array $data = array())
    {
        $this->data = $data;
    }

    public function __toString()
    {
        $username = $this->getUsername();
        $this->doSomethingWithTheUsername($username);
        return "...<input type=\"text\" name=\"username\" value=\"$username\" />...";
    }

    private function getUsername()
    {
        if (isset($this->data['username'])) {
            return $this->data['username'];
        } else {
            return '';
        } 
    }

    private function doSomethingWithTheUsername($username)
    {
        // this method is only here to provide an example of code
        // that will start utilizing the Query instead of the Temp
    }
}

and finally to the elimination of the variable:

<?php
class LoginFormTest extends PHPUnit_Framework_TestCase
{
    public function testFormDisplaysOldUsernameValue()
    {
        $form = new LoginForm(array('username' => 'giorgio'));
        $html = $form->__toString();
        $this->assertTrue((bool) strstr($html, '<input type="text" name="username" value="giorgio" />'));
    }
}


/**
 * For brevity, we implement only the code necessary for the current test 
 * to pass.
 */
class LoginForm
{
    private $data;

    public function __construct(array $data = array())
    {
        $this->data = $data;
    }

    public function __toString()
    {
        $this->doSomethingWithTheUsername();
        return '...<input type="text" name="username" value="'
             . $this->getUsername()
             . '" />...';
    }

    private function getUsername()
    {
        if (isset($this->data['username'])) {
            return $this->data['username'];
        } else {
            return '';
        } 
    }

    private function doSomethingWithTheUsername()
    {
        $this->getUsername();
        // this method is only here to provide an example of code
        // that will start utilizing the Query instead of the Temp
    }
}