Practical PHP Refactoring: Hide Method

In the scenario we address today, a method is not used from outside a class, or it's called only from a limited set of classes, such as the current inheritance hierarchy.
This refactoring, Hide Method, modifies the visibility of the method to restrain it as much as possible, making it private or protected.

Why messing with the visibility?

Information hiding simplifies the evolution of your code: you will be able to change the method's name or signature more easily, knowing you only have to look at the current class (or at least, hierarchy of classes) to catch breaks in calls or overrides.

Moreover, the private keyword is resurrecting in the PHP world: a protected-by-default policy was adopted by the first generation of frameworks; but just setting fields and methods as protected is fake extensibility. Just use private if you do not have an existing use case telling you to make it protected or public.

Moreover, fields are always private or protected, as there are no public fields in real object-oriented programming.

Methods use all the varying degrees of scope, but making them protected or private shines light on which is the public protocol of the class (the rest of the methods).

Real scope limitations

Private means accessible only from the same class; but in some languages like PHP and Python not accessible means just accessible with some hassles. If you configure a class member as private, it will be accessible from the same or different object where it is defined, but only by the same class; you will only have to look at a single source file to perform modifications.

A protected members means accessibility is provided only to a class in the same inheritance hierarchy. These rules can be infringed however by standard PHP code.

From PHP 5.3, reflection allows to set the scope of a field. Doctrine 2 uses this to reconstitute objects from a database query result, in probably the only sane reason for this feature apart from debugging.

From PHP 5.4 (currently unstable), closures binding allows to access any private or protected field by simply having a reference to an object.

Now read the instruction manual carefully:

It goes without saying that if you use these techniques to routinely access private members of your objects, you won't be able to rely on scope limitation to evolve the code or perform further refactorings.

In those cases, just make the member public: at least you will know the method or field is accessed somewhere else.

Steps

Fowler suggests to check regularly for opportunities to make a method private. However, his mechanics cannot be used in PHP because they refer to a static language.

I can suggests you my checks instead:

• Execute a grep -r 'methodName(' . to get a feel of where the method is used. This is not maximally reliable due to dynamic calls via call_user_func(), $object->$method() and so on. If the method is used from outside the target scope, you will have to deal with these cases before changing its visibility.
• Check for coverage of your methods in the test suite. You can only change code that is covered, especially in the case where the simple compiling to opcodes process of PHP won't be of any help in detecting private methods called from outside their scope. Add tests to cover the methods, but only indirectly (if you call them from the suite, making them private will break it).
• If there is a call from uncovered code, you should not go on with the refactoring. If the method has tests, isn't it the sign that it has to be extracted as public method on a private collaborator instead?

If everything is alright, the refactoring is simple.

1. Restrict the chosen methods as private or protected.
2. Run the test suite to check they're not called from the wrong place.

Example

In the initial state, a Book presenter object is producing some HTML (you can call it a View Helper if you prefer this name.) A public method is called just inside the class: there is no reason to expose it any further.

<?php
class HideMethod extends PHPUnit_Framework_TestCase
{
    public function testTheBookIsRenderedCorrectly()
    {
        $book = new BookInfo('Robots and Empire', 'Asimov');
        $this->assertEquals('<li>Robots and Empire <em>(Asimov)</em></li>', $book->__toString());
    }
}

class BookInfo
{
    private $title;
    private $author;

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

    public function __toString()
    {
        $authorInfo = $this->authorHtml();
        return "<li>$this->title $authorInfo</li>";
    }

    public function authorHtml()
    {
        return "<em>($this->author)</em>";
    }
}

grep tells us there are no other calls to worry about (obviously; it's an example):

[16:00:40][giorgio@Desmond:~/Dropbox/practical-php-refactoring]$ grep -r 'authorHtml(' *.php
HideMethod.php:        $authorInfo = $this->authorHtml();
HideMethod.php:    public function authorHtml()

Is the method covered? Yes, we see the test calling __toString(), which indirectly calls it... You may want to rely on PHPUnit's automated report in real code.

Change the visibility now:

<?php
class HideMethod extends PHPUnit_Framework_TestCase
{
    public function testTheBookIsRenderedCorrectly()
    {
        $book = new BookInfo('Robots and Empire', 'Asimov');
        $this->assertEquals('<li>Robots and Empire <em>(Asimov)</em></li>', $book->__toString());
    }
}

class BookInfo
{
    private $title;
    private $author;

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

    public function __toString()
    {
        $authorInfo = $this->authorHtml();
        return "<li>$this->title $authorInfo</li>";
    }

    private function authorHtml()
    {
        return "<em>($this->author)</em>";
    }
}

Running the test suite again confirms the scope is not too narrow:

[16:03:50][giorgio@Desmond:~/Dropbox/practical-php-refactoring]$ phpunit HideMethod.php
PHPUnit 3.6.4 by Sebastian Bergmann.

.

Time: 0 seconds, Memory: 3.50Mb

OK (1 test, 1 assertion)