Practical PHP Refactoring: Parameterize Method

In the scenario of today, multiple methods executs mostly the same logic: you can see a strong duplication of code between them, or identical delegation steps.

These methods however, are slightly different, so much that the duplication is not obvious. In the case of interest for us, the difference in behavior depends on a few parameters: a different tax percentage, a different starting point in a calculation, or even a string template.

There is a refactoring to address such a situation when the duplication becomes annoying: extracting one or more parameters to unify the different versions of the method. The result is a unique implementation which takes some more input parameters to differentiate between calls.

Beware of this refactoring

You shouldn't parameterize a method when the parameters to extract are boolean, or magic values which trigger the necessity for conditionals inside the method. The idea of this refactoring is to unify the methods bodies, not to put them into the branches of an if() or a switch().

Another case when I'm suspicious of this refactoring is when there is an added value in the semantic of multiple methods. Sometimes isSilverClient() and isGoldClient() are actually two different things, not just a check for more than M and N orders.

The rule of thumb to detect when this refactoring is appropriate is that there would theoretically be dozens of versions of the same method (even if only a pair are implemented):

• applyTax20Percent(), applyTax4Percent(), applyTax40Percent() are very similar. If there should be one for each integer number from 1 to 100, parameterize.
• countPostsOfAdmins() and countPostsOfUsers() may execute widly different queries. Do not parameterize into countPostsOf(/** @var boolean /* $idAdmin) automatically.

In the latter case, I would indeed parameterize the method if it would be the first step towards polymorphism (countPosts(RankingCriteria $userCategory)).

Steps

1. Create a parameterized method which unifies the multiple versions, and has additional parameters to distinguish what to do. Parameters are usually embedded in the name of the original versions, and become formal arguments of the new method (they can take default values.) This shouldn't affect client code for now.
2. Test the method independently. Hopefully, the tests you need to write are also a unification of the old methods test cases.
3. Change the calls, one or a few at the time, to target the new method.
4. When you're finished substituting, you can eliminate the older versions as they're not called anymore. Their tests can go too, if you have ported them onto the new method tests.

Example

We start from a simple object modelling an article of this website. Sometimes articles get popular or in the top list (this is fictional) and so they should be highlighted when displayed. For this purpose, there are two methods on the Article class that keep the $views field encapsulated but allow for popularity tests:

<?php
class ParameterizeMethod extends PHPUnit_Framework_TestCase
{
    public function testTheArticleIsConsideredPopularAfter1000Views()
    {
        $article = new Article('PPR: Extract Method', 1000);
        $this->assertTrue($article->isPopular());
    }

    public function testTheArticleIsConsideredInTheTopRankAfter10000Views()
    {
        $article = new Article('How to be a worse programmer', 10000);
        $this->assertTrue($article->isTop());
    }
}

class Article
{
    private $title;
    private $views;

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

    public function isPopular()
    {
        return $this->views >= 1000;
    }

    public function isTop()
    {
        return $this->views >= 10000;
    }
}

We write a new unit test, which targets a unique method with an additional parameter: the minimum views that an article should have to reach a certain level of popularity.

<?php
class ParameterizeMethod extends PHPUnit_Framework_TestCase
{
    public function testTheArticleIsConsideredPopularAfter1000Views()
    {
        $article = new Article('PPR: Extract Method', 1000);
        $this->assertTrue($article->isPopular());
    }

    public function testTheArticleIsConsideredInTheTopRankAfter10000Views()
    {
        $article = new Article('How to be a worse programmer', 10000);
        $this->assertTrue($article->isTop());
    }

    public function testPopularityIsDecidedByAViewsParameter()
    {
        $article = new Article('How to be a worse programmer', 10000);
        $this->assertTrue($article->isEnoughPopular(10000));
        $this->assertFalse($article->isEnoughPopular(10001));
    }
}

Now we implement the unified method. The implementation can be copied by the other versions, but a parameter needs to be introduced instead of the constant.

class Article
{
    private $title;
    private $views;

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

    public function isEnoughPopular($minimumViews)
    {
        return $this->views >= $minimumViews;
    }

    public function isPopular()
    {
        return $this->views >= 1000;
    }

    public function isTop()
    {
        return $this->views >= 10000;
    }
}

We make the calls uniform, by changing the old calls to reflect the new ones. I'll use constants for these popular values, but the generic method leaves open the possibility for using other values (or to refactor to a strategy in the future)

I would probably delete these unit tests as the code is well-covered by the new one. We leave them in place now just to show how to deal with existing client code.

<?php
class ParameterizeMethod extends PHPUnit_Framework_TestCase
{
    public function testTheArticleIsConsideredPopularAfter1000Views()
    {
        $article = new Article('PPR: Extract Method', 1000);
        $this->assertTrue($article->isEnoughPopular(Article::POPULAR));
    }

    public function testTheArticleIsConsideredInTheTopRankAfter10000Views()
    {
        $article = new Article('How to be a worse programmer', 10000);
        $this->assertTrue($article->isEnoughPopular(Article::TOP));
    }

    public function testPopularityIsDecidedByAViewsParameter()
    {
        $article = new Article('How to be a worse programmer', 10000);
        $this->assertTrue($article->isEnoughPopular(10000));
        $this->assertFalse($article->isEnoughPopular(10001));
    }
}

class Article
{
    private $title;
    private $views;
    const POPULAR = 1000;
    const TOP = 10000;

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

    public function isEnoughPopular($minimumViews)
    {
        return $this->views >= $minimumViews;
    }

    public function isPopular()
    {
        return $this->views >= 1000;
    }

    public function isTop()
    {
        return $this->views >= 10000;
    }
}

Finally, we eliminate the old versions, which are not called nor covered by tests anymore.

class Article
{
    private $title;
    private $views;
    const POPULAR = 1000;
    const TOP = 10000;

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

    public function isEnoughPopular($minimumViews)
    {
        return $this->views >= $minimumViews;
    }
}