I am a programmer and architect (the kind that writes code) with a focus on testing and open source; I maintain the PHPUnit_Selenium project. I believe programming is one of the hardest and most beautiful jobs in the world. Giorgio is a DZone MVB and is not an employee of DZone and has posted 638 posts at DZone. You can read more from them at their website. View Full User Profile

Practical PHP Refactoring: Split Temporary Variable

06.29.2011
| 3992 views |
  • submit to reddit

The scenario of today: a variable is being reused for different assignments; the kind of data that gets stored into the variable may be related on a spectrum from number vs. the same updated number to boolean that is substituted by an object graph.

In the cases we consider, we are looking at the latter end of the spectrum, where the variable functionally changes. Updating an $i cycle index or a partial sum is not an issue for us, while assigning it another sum (or the results of other operations) is a smell; assigning it a string or a variable of a different type or object is plain dirty.

Often the temporary variables we address have temporary names, introduced quickly and forgotten there; you may have some reminescences of old C code where each variable had to be tracked for memory usage, and this friction prevented refactoring.

PHP is also friendly with the variable recycling practices, because it does not have a final keyword to prevent variable reassignment; moreover, PHP lets you change the type of each variable at any time (the only static type check is the type hint for method parameters or loose the methods you call on an object, which will make it explode if not present.)

When multiple assignments are ok

Examples from Beck's and Fowler's books are loop variables and collecting variables: it's normal for accumulators and cycle-related variables to change.

In any other case of variables assigned multiple times, they are candidate for splitting. In general, you should split the job of a single variable if it has more than one responsibility:

  • is the name a catchall with conjunctions, like distanceOrHeight? (Very basic check)
  • Is the name of the variable correct after both assignments? Or does it fit only the first, or the second?
  • Would the code between the two assigmnents still be correct after the second? Or it would try to use a percentage in place of a distance?
$distance = 100; // meters
$distance += $previousMovements; // still ok, meters
$distance = $distance / $maximumDistance; // change of type and meaning: smell
  • Was the variable a parameter? It's not nice to change its value since someone may step in the middle of the method and expect @param string $title to still be a string containing a title, and not an object or a long book text. Sometimes one may miss the smell because there is no previous assignment: the value was passed in.

Remember there's no need to save space by micro-optimization and reuse variables: it's not even necessarily true that it would be faster. Feel free to use all the variables you want.

Steps

  • Change the name of the temporary variable at its first assignment (which is also the declaration in PHP). If the name was unique, refactor by introducing two different names to mark the change; when the code will be working again, you may consider to simplify one of them.
  • Change references up to the second assignment.
  • Change name to the second assignment.
  • Change references up to the third assignment. And so on.

This list of steps assumes you are in single sequential flow and scope, for example in a single method. If the code is divided between methods or classes, try inlining everything in a single method before starting.

Execute your fast, unit tests between steps, as always.

Example

The code sample is related to date manipulation. Since the Api of DateTime mostly returns new objects, there are some assignments that are made on the same variable passed in as a parameter?

<?php
class SplitTemporaryVariableTest extends PHPUnit_Framework_TestCase
{
    public function testDatesAreShiftedAtTheEndOfSomeFutureMonth()
    {
        $adjustment = new Adjustment(3);
        $futureDate = $adjustment->apply(new DateTime('2011-06-20'));
        $this->assertEquals(new DateTime('2011-09-30'), $futureDate);
    }
}

class Adjustment
{
    public function __construct($minimumMonths)
    {
        $this->minimumMonths = $minimumMonths;
    }

    public function apply(DateTime $date)
    {
        $date = $date->add(new DateInterval('P3M'));
        $date = $date->setDate($date->format('Y'), $date->format('m'), $date->format('t'));
        return $date;
    }
}

After one split, $date is reassigned only one time, and we get to know what that strange 'P3M' means.

    public function apply(DateTime $date)
    {
        $someMonthsInTheFuture = $date->add(new DateInterval('P3M'));
        $date = $someMonthsInTheFuture->setDate($someMonthsInTheFuture->format('Y'), $someMonthsInTheFuture->format('m'), $someMonthsInTheFuture->format('t'));
        return $date;
    }

After another split, also that 't' date formatter earns a meaning:

    public function apply(DateTime $date)
    {
        $someMonthsInTheFuture = $date->add(new DateInterval('P3M'));
        $lastDayOfTheMonth = $someMonthsInTheFuture->setDate($someMonthsInTheFuture->format('Y'), $someMonthsInTheFuture->format('m'), $someMonthsInTheFuture->format('t'));
        return $lastDayOfTheMonth;
    }

After the refactoring has been completed, it's also time to generalize this code to work with any number of months, in the TDD way.

        $someMonthsInTheFuture = $date->add(new DateInterval('P' . $this->minimumMonths . 'M'));

 

Published at DZone with permission of Giorgio Sironi, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)