Practical PHP Refactoring: Consolidate Conditional Expression

In this new article we continue to tackle conditional expressions, and their evolution towards polymorphism.

In the scenario of today, multiple conditions lead to the same result: returning a value, or executing some code. If only some of the conditions lead to the same operations, you can usually reorder them and apply this refactoring to the subset of operations that is common between the conditionals.

The goal is extracting the various conditions into a readable method, possibly easy to transport away from the object in order to reach a polymorphic solution.

Why a single conditional?

The duplication is in the operations to execute, and in the control structures containing them: when an if() is a smell, imagine a series of them! Also, there is probably coupling between the different conditions as they lead to the same result. Modifying one could lead to modify the others to cover the remaining cases.

As a result, you extract in a method a single concept, representing the occurring of all or any (or a particular combination of) conditions. This method would be easier to move in a Factory class or a leaf object in case you want to refactor to polymorphism.

Polymorphism?

These conditions should be at least encapsulated in leaf objects, which do not compose other objects nor execute external logic. This is in fact a small scale refactoring, to be executed on a single class.

A possible further step towards polymorphism can be taken in case the conditionals are only based on the state of the object and not external parameters. You will be able to replace the class with multiple classes and move the evaluation of the condition at instantiation time, where you choose one of the two (or more classes). But that's a story for another refactoring.

Steps

A prerequisite for applying this refactoring is that no side effects should be started from the execution of conditionals. If the conditions are comparisons or query methods, they should not have any (Command Query Separation). The side effects may be in the code to execute: for example, if each then block adds something to a variable, this refactoring (unifying the blocks) is not viable.

1. Replace the various ifs with a single expression, connected with or, and and !.
2. Extract a method, encapsulating the condition in it.

Naturally, you should execute unit tests between steps to ensure corner cases are still covered.

The consolidation in a single expression usually follows this heuristic:

• Nested ifs become separeted by and.
• Ifs in sequence become separated by or.

Example

We start from the same logic of the previous example, but expressed with multiple conditionals. They have no side effects, since the blocks to execute consist only of return statement and the conditions themselves are just expressions to evaluate.

<?php
class ConsolidateConditionalExpression 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) {
            return "<span class=\"evidence\">In evidence: $this->name</span>";
        }
        if ($this->recentlyCreated) {
            return "<span class=\"evidence\">In evidence: $this->name</span>";
        } 
        return "<span>$this->name</span>";
    }
}

We transform the conditions into a single one: since they are alternatives, we connect them with an or:

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>";
        } 
        return "<span>$this->name</span>";
    }
}

Next we extract a method containing the condition, giving it the logical name of isInEvidence().

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->isInEvidence()) {
            return "<span class=\"evidence\">In evidence: $this->name</span>";
        } 
        return "<span>$this->name</span>";
    }

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

We could go on by dividing the class into a InEvidenceGroup and OrdinaryGroup pair, but it's difficult to do so with a small example since we should also change the instantiation code (which is not present here apart from the unit tests) to decide which class to use. The conditional would be moved there; here's a sketch:

interface Group
{
    public function __toString();
}
class InEvidenceGroup
{
    /* ... */
    public function __toString()
    {
        return "<span class=\"evidence\">In evidence: $this->name</span>";
    }
}
class OrdinaryGroup
{
    /* ... */
    public function __toString()
    {
        return "<span>$this->name</span>";
    }
}

Since the state is not fixed, we should also change the class of the object if the field changes, transforming it in an immutable Value Object whose methods return a new Group instance.