Practical PHP Refactoring: Replace Parameter with Explicit Methods

The refactoring of today breaks up a method into multiple ones: it chops the method in pieces according to the code that is executed in response to one of the parameters. This parameter can only assume several values, or several equivalent class of values; each choice lead to a new version of the method.

Typically, the parameter is boolean, and two results may be provided according to its value. Also Value Objects wrapping integers and fixed strings fall in the same category (finite set of values).

This refactoring is the inverse of Parameterize Method, which takes multiple versions of a method and unify them by adding a distinguishing parameter to the calls.

Why dividing the method in multiple versions?

Since by hypothesis there are only several discrete values for the chosen parameter, it's simple to map to several different methods the original one. This fact enables the refactoring but does not tell us when it is a good direction to follow.

The advantages of multiple methods are clear when a switch() or an if() on a parameter is found in the original method. You can avoid conditional behavior that executes wildly different code depending on the key parameter value.

The interface of the object becomes also clearer as you have a set of public methods which can be named well, instead of a catch-all name like process($actionType, ...).
Finally, this refactoring shortens the parameter list (by removing one of the arguments) and the method body, by choosing only one branch of conditional statements for each new method.

When not to use it?

Fowler specifies in his Refactoring book that you should not introduce explicit methods when you think that the parameters will change in new, unexpected ways. Multiple implementations of a parameter object are easy to add while multiple versions of a method may break the Api of the original object and force you to grep all client code to adjust the calls.

In any case, this refactoring takes the variability of the method and deals with it in the object where it resides. Moving this variability out of the object would be viable with polymorphism, and it is preferrable when you need to preserve extensibility. Yet if you don't need extensibility but just a few different use cases, this refactoring makes you express them in the best way: a single method for each.

Steps

1. Create explicit methods for each of the code paths you want to break the method into. For example, if you have a switch with 3 cases you will create 3 explicit methods.
2. Delegate to these methods from inside the old monolithic one. This step shortens the original method and makes sure that you don't forget anything.
3. Change client code calls to use the new methods.
4. Remove the original method when finished.

The tests must pass in between each step: there is no reason to break the code while applying this refactoring.

Example

The example is based on a forum context: threads are domain objects and are not related to multithreading.

On the Thread object, we have a setter for a boolean parameter. There is also a getter. The open field is false or true depending on the thread's state.
The setter updates the calculated field differently basing on the boolean parameter: we want to eliminate the conditional and provide the client code a more precise Api.

<?php
class ReplaceParameterWithExplicitMethods extends PHPUnit_Framework_TestCase
{
    public function testThreadCanBeClosed()
    {
        $thread = new Thread('Ubuntu on EEE Pc');
        $thread->setOpen(false);
        $this->assertFalse($thread->getOpen());
        $this->assertEquals('[closed] Ubuntu on EEE Pc', $thread->__toString());
    }

    public function testThreadCanBeOpened()
    {
        $thread = new Thread('Ubuntu on EEE Pc');
        $thread->setOpen(true);
        $this->assertTrue($thread->getOpen());
        $this->assertEquals('Ubuntu on EEE Pc', $thread->__toString());
    }
}

class Thread
{
    private $title;
    private $open;

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

    public function setOpen($state)
    {
        $this->open = $state;
        if (!$this->open) {
            $this->label = '[closed] ' . $this->title;
        } else {
            $this->label = $this->title;
        }
    }

    public function getOpen()
    {
        return $this->open;
    }

    public function __toString()
    {
        return $this->label;
    }
}

We transform the setter: close() and open() are better names for methods that do more than a simple assignment to a field. The getter does not contain different branches of execution.

class Thread
{
    private $title;
    private $open;

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

    public function setOpen($state)
    {
        $this->open = $state;
        if (!$this->open) {
            $this->label = '[closed] ' . $this->title;
        } else {
            $this->label = $this->title;
        }
    }

    public function open()
    {
        $this->open = true;
        $this->label = $this->title;
    }

    public function close()
    {
        $this->open = false;
        $this->label = '[closed] ' . $this->title;
    }

    public function getOpen()
    {
        return $this->open;
    }

    public function __toString()
    {
        return $this->label;
    }
}

We call the new methods from the inside of the conditional. The old method is really ugly to see now, but it will be gone in a moment.

class Thread
{
    private $title;
    private $open;

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

    public function setOpen($open)
    {
        if (!$open) {
            $this->close();
        } else {
            $this->open();
        }
    }

    public function open()
    {
        $this->open = true;
        $this->label = $this->title;
    }

    public function close()
    {
        $this->open = false;
        $this->label = '[closed] ' . $this->title;
    }

    public function getOpen()
    {
        return $this->open;
    }

    public function __toString()
    {
        return $this->label;
    }
}

The tests are still passing.

[giorgio@Desmond:practical-php-refactoring]$ phpunit ReplaceParameterWithExplicitMethods.php
PHPUnit 3.5.15 by Sebastian Bergmann.

..

Time: 0 seconds, Memory: 3.50Mb

OK (2 tests, 4 assertions)

Now we can change the calls from the client code (just the tests in this example).

<?php
class ReplaceParameterWithExplicitMethods extends PHPUnit_Framework_TestCase
{
    public function testThreadCanBeClosed()
    {
        $thread = new Thread('Ubuntu on EEE Pc');
        $thread->close();
        $this->assertFalse($thread->getOpen());
        $this->assertEquals('[closed] Ubuntu on EEE Pc', $thread->__toString());
    }

    public function testThreadCanBeOpened()
    {
        $thread = new Thread('Ubuntu on EEE Pc');
        $thread->open();
        $this->assertTrue($thread->getOpen());
        $this->assertEquals('Ubuntu on EEE Pc', $thread->__toString());
    }
}

Finally, we can remove the old version of the method, which is not reached anymore by any call.

class Thread
{
    private $title;
    private $open;

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

    public function open()
    {
        $this->open = true;
        $this->label = $this->title;
    }

    public function close()
    {
        $this->open = false;
        $this->label = '[closed] ' . $this->title;
    }

    public function getOpen()
    {
        return $this->open;
    }

    public function __toString()
    {
        return $this->label;
    }
}