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: Decompose Conditional

10.05.2011
| 4681 views |
  • submit to reddit
The terminology we will use in these articles on conditionals will be the following:
  • a conditional is a whole if/else or switch statement and its content. It is composed by a condition and various blocks that are alternatively executed.
  • the various blocks are called then or else block.

This if the first issue of the miniseries on conditional expressions. Throughout it we will see how to simplify and move around conditional expressions like the one found in ifs.

Polymorphism is usually a better alternative than long if chains in object-oriented programming. In fact, we will start from less invasive refactorings that are executed on conditionals, and move on to polymorphic alternatives later on. Not all ifs are evil: but polymorphism allows us to eliminate most of their duplication.

Scenario

In this first scenario, we have a complex conditional that is hard to understand. It may contain many or and and operators, some negations (!), and multiple subconditions. It may also be long, being composed of a then, else if and else branches.

A first solution, which we see today, is to apply Extract Method to the various atomic parts: the condition, and the then or else blocks. The conditional itself is left in place for now, but most of the details are abstracted away.

This refactoring works well mostly in leaf objects: we want to avoid the conditionals from spreading around in the codebase and continuously being repeated.

Why improving a conditional?

We go from a long method, hard to read, to a shorter method, hopefully easier to understand. Moreover, the refactoring forces you to name concepts like the result of the condition, or the actions to be performed.

It's also a nice way of applying a single level of abstraction policy: you extract the same level of detail from the conditions and the actions, avoiding to neglect any of them.

The conditional logic however is not hidden nor trasformed: you highlight the fact that there is a branch in execution in this point.

Steps

  1. Extract Method on the condition. The method is usually named as is... or can...
  2. Extract Method on the then part.
  3. Extract Method on the else part, where present.

Of course, local variables should be passed as parameters of the extracted methods, while fields are usually accessed via $this. The extracted methods are private by default.

Example

In the initial state, we have a presenter (not a domain object) named Group; note the ugly boolean flag. We will see polymorphic solution later in the series.

This kind of refactoring is less invasive, and may be preliminary to a more serious one which divides Group in different objects and makes use of polymorphism.

<?php
class DecomposeConditional extends PHPUnit_Framework_TestCase
{
    public function testAGroupIsInEvidenceWhenManyPostsArePresent()
    {
        $group = new Group('PHP forum', 100);
        $this->assertEquals('<span class="evidence">In evidence: PHP forum</span>', $group->__toString());
    }

    public function testAGroupIsShownAsNormalWhenThereAreNotManyPosts()
    {
        $group = new Group('PHP forum', 10);
        $this->assertEquals('<span>PHP forum</span>', $group->__toString());
    }

    public function testAGroupIsAlsoInEvidenceWhenItHasBeenRecentlyCreated()
    {
        $group = new Group('PHP forum', 0, true);
        $this->assertEquals('<span class="evidence">In evidence: PHP forum</span>', $group->__toString());
    }
}

class Group
{
    private $name;
    private $posts;
    private $recentlyCreated;

    public function __construct($name, $posts, $recentlyCreated = false)
    {
        $this->name = $name;
        $this->posts = $posts;
        $this->recentlyCreated = $recentlyCreated;
    }

    public function __toString()
    {
        if ($this->posts > 50 || $this->recentlyCreated) {
            return "<span class=\"evidence\">In evidence: $this->name</span>";
        } else {
            return "<span>$this->name</span>";
        }
    }
}

One step at a time, we extract the condition:

class Group
{
    private $name;
    private $posts;
    private $recentlyCreated;

    public function __construct($name, $posts, $recentlyCreated = false)
    {
        $this->name = $name;
        $this->posts = $posts;
        $this->recentlyCreated = $recentlyCreated;
    }

    public function __toString()
    {
        if ($this->shouldBeInEvidence()) {
            return "<span class=\"evidence\">In evidence: $this->name</span>";
        } else {
            return "<span>$this->name</span>";
        }
    }

    private function shouldBeInEvidence()
    {
        return $this->posts > 50
            || $this->recentlyCreated;
    }
}

We extract the then branch:

class Group
{
    private $name;
    private $posts;
    private $recentlyCreated;

    public function __construct($name, $posts, $recentlyCreated = false)
    {
        $this->name = $name;
        $this->posts = $posts;
        $this->recentlyCreated = $recentlyCreated;
    }

    public function __toString()
    {
        if ($this->shouldBeInEvidence()) {
            return $this->inEvidenceLabel();
        } else {
            return "<span>$this->name</span>";
        }
    }

    private function shouldBeInEvidence()
    {
        return $this->posts > 50
            || $this->recentlyCreated;
    }

    private function inEvidenceLabel()
    {
        return "<span class=\"evidence\">In evidence: $this->name</span>";
    }
}

Finally, we extract also the else branch:

class Group
{
    private $name;
    private $posts;
    private $recentlyCreated;

    public function __construct($name, $posts, $recentlyCreated = false)
    {
        $this->name = $name;
        $this->posts = $posts;
        $this->recentlyCreated = $recentlyCreated;
    }

    public function __toString()
    {
        if ($this->shouldBeInEvidence()) {
            return $this->inEvidenceLabel();
        } else {
            return $this->ordinaryLabel();
        }
    }

    private function shouldBeInEvidence()
    {
        return $this->posts > 50
            || $this->recentlyCreated;
    }

    private function inEvidenceLabel()
    {
        return "<span class=\"evidence\">In evidence: $this->name</span>";
    }
    
    private function ordinaryLabel()
    {
        return "<span>$this->name</span>";
    }
}

Now we have a readable, small presenter. In the next issues, we will start to think more and more aggressively about a polymorphic solution.

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