Practical PHP Refactoring: Move Method

Following in Fowler's steps, we start a section of refactorings that involve moving features and code between classes and objects. While the changes we have seen so far are local and easy to make for an average disciplined programmer, refactoring is also composed by larger changes (but well-prepared by previous steps); otherwise the design would be eternally stuck in a local minima.

It's also important to execute correctly these larger changes: while you can get away with skipping intermediate steps while refactoring local variables, fiddling with moved methods may cause your whole application to explode if you're not careful (and require a reset/revert which would throw your invested time away.)

Move Method

The first one of these refactoring is also the simplest: Move Method between classes. The assumption is you want to move a method M of class A on another class B. The old method may either vanish, or turn into a delegation to the new one for the time being.

Seeing when and where to move a method is a design ability that we pick up over the thousands of lines of code we write; the refactoring description is only about moving methods and not about the purpose of this choice. Refactorings are hammers, but it's up to you to use these hammers on a nail or on one of your fingers.

There are some rules of thumb that may help you deciding a movement:

• move M to B because B contains the data that M uses: the goal is to improve cohesion.
• Move M to B because you don't want the objects that call M to depend on A, but if they depend on B the picture is simplified. The goal is to decrease coupling, and eliminating an uncomfortable dependency.

Steps

Moving method M from class A to class B requires the following steps.

1. Check what M is using: other methods may have to be moved as well.
2. Check A superclasses and subclasses: they may prevent you from moving the method without a larger-than-you-want refactoring if they contain overridded or overriding versions of M.
3. Declare the method in B. You can change its name if it is redundant with B metadata, like its namespace or class name.
4. Copy code from A to B: if the method uses A heavily, pass it as a parameter and substitute it to $this. Change also the thrown exceptions if there are any, and the catching mechanism is different (e.g. moving login between layers implies different exceptions.)
5. Reference B object from A by injection, or instantiation. This change does not have to be permanent, and in many cases there is already a reference that can be exploited.
6. Turn the old method into delegation.
7. Run the tests at the right level of detail. Unit tests probably need an update before the move, at least in the arrange phase.

An additional step you can take ir removing the original A method. In that case, callers of M must be updated in the reference they get (B instance instead of A instance) and signature (which may be different due to the addition of the A object where needed.)

Remember to test thoroughly before committing a change like a method movement. Since it has a surefire effect on the callers, it can break relatively distant parts of your application.

Example

Initially, this TagCloud class has a bit of feature envy towards the Link one, since it performs work that could be done in the Link class without introducing strange outward dependencies (e.g. an i18n service object).

Note that this example is purely about how to move a method, not about how to choose where to move it: I just happen to find an example that targets a simpler design better to show than an example about a meaningless movement.

<?php
class MoveMethodTest extends PHPUnit_Framework_TestCase
{
    public function testDisplayItsLinksInShortForm()
    {
        $tagCloud = new TagCloud(array(
            new Link('http://giorgiosironi.blogspot.com/search/label/productivity'),
            new Link('http://giorgiosironi.blogspot.com/search/label/software%20development')
        ));
        $html = $tagCloud->toHtml();
        $this->assertEquals(
            "<a href=\"http://giorgiosironi.blogspot.com/search/label/productivity\">productivity</a>\n"
          . "<a href=\"http://giorgiosironi.blogspot.com/search/label/software%20development\">software development</a>\n",
            $html
        );
    }
}

class TagCloud
{
    private $links;

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

    public function toHtml()
    {
        $html = '';
        foreach ($this->links as $link) {
            $text = $this->linkText($link);
            $html .= "<a href=\"$link\">$text</a>\n";
        }
        return $html;
    }
    
    private function linkText(Link $link)
    {
        $lastFragment = substr(strrchr($link, '/'), 1);
        return str_replace('%20', ' ', $lastFragment);
    }
}

class Link
{
    private $url;

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

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

After the movement, the method is simpler because it does not have to traverse a class boundary.

class TagCloud
{
    private $links;

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

    public function toHtml()
    {
        $html = '';
        foreach ($this->links as $link) {
            $text = $link->text();
            $html .= "<a href=\"$link\">$text</a>\n";
        }
        return $html;
    }
}

class Link
{
    private $url;

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

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

    /**
     * The movement change the method in many ways:
     * - simpler name
     * - public visibility 
     * - one less parameter
     * The refactoring makes Link a behavior-rich Value Object, but this is 
     * incidental; if I could have moved a method that used an external 
     * service from Link to TagCloud. However, in these samples I have a 
     * limited space and often many of my objects are Value Objects just 
     * because I wanted to post running examples, and they are a really
     * portable kind of code.
     */
    public function text()
    {
        $lastFragment = substr(strrchr($this, '/'), 1);
        return str_replace('%20', ' ', $lastFragment);
    }
}