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: Pull Up Field

12.26.2011
| 5989 views |
  • submit to reddit

We now enter in the territory of generalizations. It's a natural process, as we add more and more tests, for our code to become more general; but after we have made our test pass, the refactoring phase include many ways to deal with generalization.

Refactoring for generalization is meant to solve the problem of duplication: most of these refactoring start from a situation where code (or, to say it better, knowledge) is duplicated in multiple points and end up eliminating it.

I don't have to remind you that code duplication is universally recognized as cancer because the multiple places where a concept (or a rule, a strategy, a requirement) is expressed can easily get out of sync: one of the entities may be updated while the other is not.

The first generalization refactoring we encounter is Pull Up Field: it targets a scenario where multiple subclasses have the same field, or a very similar field that with a name change can become identical to the other copies. The solution proposed by this refactoring (by no means the unique one) is to move up the field into the hierarchy, so that the it is inherited by the subclasses.

Why moving up in the hierarchy?

From the external point of view, nothing will change: method calls will be exactly the same, but the classes will be easier to maintain as only one field definition has to change. There is less code to move, delete and update.

If you apply TDD and the 4 rules of simple design, you may start from a solution containing duplication, and even copy and paste code sometimes (with the idea that most of it will change in the new class or method). But before your next commit, you will have to reduce the duplication as much as possible.

This refactoring allows also to subsequently move methods using the common field: it is in fact a prerequisite for some instances of Pull Up Method.

Steps

  1. Inspect the field's usage: if you keep fields private or protected, you know you will have a limited set of source files where to look and execute a find operation. All operations performed with the field value, or on the field value, should be regarded.
  2. In case it's required, rename the fields to the same identifier (that's the old school name for names referencing to variables and fields).
  3. Create a field in the superclass, which for now will be overridden by the subclasses fields.
  4. Delete the subclass fields.

Note that if the pulled up field maintains the original name, between steps 3 and 4 you may get failing tests if their scope was originally private. It is possible to redefine fields in PHP, but not with stricter visibility. The tests however, won't change at all in any step.

Fowler notes that field may undergo a change in scope during the refactoring: the field must become protected if it was private. This is part of the price to pay for using inheritance.

Example

In the initial state, the author field is duplicated in the two subclasses. We suppose a common superclass already exists, and we will focus on eliminating only the duplication originating from the field definition itself, not from the code that references it. In both cases, other refactorings will target those needs.

<?php
class PullUpField extends PHPUnit_Framework_TestCase
{
    public function testAPostShowsItsAuthor()
    {
        $post = new Post("Hello, world!", "giorgiosironi");
        $this->assertEquals("Hello, world! -- giorgiosironi",
                            $post->__toString());
    }

    public function testALinkShowsItsAuthor()
    {
        $link = new Link("http://en.wikipedia.com", "giorgiosironi");
        $this->assertEquals("<a href=\"http://en.wikipedia.com\">http://en.wikipedia.com</a> -- giorgiosironi",
                            $link->__toString());
    }
}

abstract class NewsFeedItem
{
}

class Post extends NewsFeedItem
{
    private $text;
    private $author;

    public function __construct($text, $author)
    {
        $this->text = $text;
        $this->author = $author;
    }

    public function __toString()
    {
        return "$this->text -- $this->author";
    }
}

class Link extends NewsFeedItem
{
    private $url;
    private $author;

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

    public function __toString()
    {
        return "<a href=\"$this->url\">$this->url</a> -- $this->author";
    }
}

Since the fields already have the same name, we don't have to modify them. We just add a field in the protected scope of the superclass. We can specify a common documentation for all the classes that inherit from it:

abstract class NewsFeedItem
{
    /**
     * @var string  references the author's Twitter username
     */
    protected $author;
}

To make the tests pass again, we should eliminate the fields from the subclasses:

class Post extends NewsFeedItem
{
    private $text;

    public function __construct($text, $author)
    {
        $this->text = $text;
        $this->author = $author;
    }

    public function __toString()
    {
        return "$this->text -- $this->author";
    }
}

class Link extends NewsFeedItem
{
    private $url;

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

    public function __toString()
    {
        return "<a href=\"$this->url\">$this->url</a> -- $this->author";
    }
}

There are no duplicated fields now, so this refactoring stops here. In the next episodes of this series we will remove even more duplication (but not necessarily get fewer lines of code).

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.)