Agile Zone is brought to you in partnership with:

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 636 posts at DZone. You can read more from them at their website. View Full User Profile

Practical PHP Refactoring: Extract Hierarchy

02.15.2012
| 5638 views |
  • submit to reddit

Suppose you have a God class that is full of ifs and case statements, or boolean flags. Many refactorings try to segregate responsibilities in smaller entities: methods, subclasses, or collaborators.

One option for starting to break up the class is based on inheritance. The end result is many little subclasses, each modelling a special case which was submerged in the original giant class.

The Extract Hierarchy refactoring goes from one complex class to one abstract base class with hooks for subclasses to accomodate special behavior. It is a very large scale refactoring, and supposes you can modify the instantiations of the original class and model special cases in the state of the instances itself.

Most of the time for implementing the refactoring goes into learning which are the correct cases to model as subclasses, more than in the actual extraction. Fowler suggests to perform it in small steps over several days.

Smells

How do you recognize that a large class can be broken up in special cases? For startes, it contains several hundreds lines of code. Furthermore, its methods contain any of this smells:

  • boolean fields, used to decide what to do.
  • Type codes, the generalization of boolean fields to O(N) special cases.
  • If/else chains or switches, especially based on the state of the current object.

Note that subclassing isn't always the better choice for factoring out variation: it's just better than a single class, but only while you're reading code; at runtime all subclasses copy in them the logic of their ancestors.

Steps

  1. Identify a variation you want to extract. It should be an important concern, because subclassing can only address one axis of variability.
  2. Replace the constructor of the class with a Factory Method, to centralize the choice of the class to instantiate.
  3. Create the subclass(es) and rewrite the Factory Method accordingly.
  4. Extract methods trying to separate the variable code (containing conditionals) from the fixed one. These variability methods are the key to understand which special cases are modelled by the class.
  5. Copy the variability methods to the subclasses: each copy can then be dumbed down to only the special case addressed by the subclass.
  6. Declare the original class abstract.
  7. Delete all the bodies of variability methods in the superclass: if their responsibility has been divided correctly into the subclasses, the tests and the client code won't notice.

Example

We start from a view helper representing some visualization of a forum topic: its title, and maybe other combinations of fields. We have two cases tied into a single class, one normal and the other special: a topic can be in evidence or not.

<?php
class ExtractHierarchy extends PHPUnit_Framework_TestCase
{
    public function testAnOrdinaryTopicDisplaysItsTitleAsNormalText()
    {
        $topic = new Topic('OOP in R');
        $this->assertEquals('<div class="title">OOP in R</div>', $topic->title());
    }

    public function testATopicInEvidenceDisplaysItselfAsStronger()
    {
        $topic = new Topic('OOP in R', true);
        $this->assertEquals('<div class="title"><strong>OOP in R</strong></div>', $topic->title());
    }
}

class Topic
{
    private $title;
    private $inEvidence;

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

    public function title()
    {
        $title = '<div class="title">';
        if ($this->inEvidence) {
            $title .= "<strong>$this->title</strong>";
        } else {
            $title .= $this->title;
        }
        $title .= '</div>';
        return $title;
    }
}

We encapsulate the creation logic, for now very simple, into a Factory Method and make the constructor private. We create the two subclasses, and adjust the creation logic to avoid instantiating the generic superclass Topic anymore.

<?php
class ExtractHierarchy extends PHPUnit_Framework_TestCase
{
    public function testAnOrdinaryTopicDisplaysItsTitleAsNormalText()
    {
        $topic = Topic::fromFields('OOP in R');
        $this->assertEquals('<div class="title">OOP in R</div>', $topic->title());
    }

    public function testATopicInEvidenceDisplaysItselfAsStronger()
    {
        $topic = Topic::fromFields('OOP in R', true);
        $this->assertEquals('<div class="title"><strong>OOP in R</strong></div>', $topic->title());
    }
}

class Topic
{
    private $title;
    private $inEvidence;

    public static function fromFields($title, $inEvidence = false)
    {
        if ($inEvidence) {
            return new InEvidenceTopic($title, $inEvidence);
        } else {
            return new OrdinaryTopic($title, $inEvidence);
        }
    }

    private function __construct($title, $inEvidence)
    {
        $this->title = $title;
        $this->inEvidence = $inEvidence;
    }

    public function title()
    {
        $title = '<div class="title">';
        if ($this->inEvidence) {
            $title .= "<strong>$this->title</strong>";
        } else {
            $title .= $this->title;
        }
        $title .= '</div>';
        return $title;
    }
}

class OrdinaryTopic extends Topic
{
}

class InEvidenceTopic extends Topic
{
}

We extract the only method with variability related to the boolean field we want to eliminate.

class Topic
{
    private $title;
    private $inEvidence;

    public static function fromFields($title, $inEvidence = false)
    {
        if ($inEvidence) {
            return new InEvidenceTopic($title, $inEvidence);
        } else {
            return new OrdinaryTopic($title, $inEvidence);
        }
    }

    private function __construct($title, $inEvidence)
    {
        $this->title = $title;
        $this->inEvidence = $inEvidence;
    }

    public function title()
    {
        $title = '<div class="title">';
        $title .= $this->decoratedTitleText();
        $title .= '</div>';
        return $title;
    }

    protected function decoratedTitleText()
    {
        if ($this->inEvidence) {
            return "<strong>$this->title</strong>";
        } else {
            return $this->title;
        }
    }
}

We copy it into the subclasses:

class OrdinaryTopic extends Topic
{
    protected function decoratedTitleText()
    {
        if ($this->inEvidence) {
            return "<strong>$this->title</strong>";
        } else {
            return $this->title;
        }
    }
}

class InEvidenceTopic extends Topic
{
    protected function decoratedTitleText()
    {
        if ($this->inEvidence) {
            return "<strong>$this->title</strong>";
        } else {
            return $this->title;
        }
    }
}

We specialize the two copies to not use the field.

class OrdinaryTopic extends Topic
{
    protected function decoratedTitleText()
    {
        return $this->title;
    }
}

class InEvidenceTopic extends Topic
{
    protected function decoratedTitleText()
    {
        return "<strong>$this->title</strong>";
    }
}

Now we can delete the original variability method's body (but not the signature), and declare the base class abstract. A further simplification also deletes the $this->inEvidence field.

abstract class Topic
{
    protected $title;

    public static function fromFields($title, $inEvidence = false)
    {
        if ($inEvidence) {
            return new InEvidenceTopic($title);
        } else {
            return new OrdinaryTopic($title);
        }
    }

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

    public function title()
    {
        $title = '<div class="title">';
        $title .= $this->decoratedTitleText();
        $title .= '</div>';
        return $title;
    }

    /**
     * @return string
     */
    abstract protected function decoratedTitleText();
}
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.)