Practical PHP Refactoring: Remove Setting Method

A setter for a certain field is present: maybe it's a generated method, or is used for Dependency Injection.

However, in the current state of the code, there are no use cases for it [anymore]: it's either a never called method, or one called only during construction.

The Remove Setting Method refactoring makes you throw away this method and formally preserve the immutability of the object. You know not to expect change in the state of an object only when it has no methods assigning to its private fields (except a constructor).

Why deleting a setter?

If a method doesn't serve any purpose, throwing it away to simplify the design (no matter how simple it already is) is desirable. This is true for any method, but in the case of a setter we may want to shift its responsibility to other methods in order to get rid of a very generic piece of the Api, allowing anyone with a reference to the object to mess with the internal state.

If a method or field has a too large scope (public, while called only inside a class), we can restrain the scope to simplify the assumptions people new to the code make on the design. By people new to the code, I mean also you and me 2 weeks from now.

For example, when applying a further refactoring technique, you will enjoy more freedom as you will only have to check calls inside a class, instead of exploring the whole codebase with grep.

Objects are often used as immutable: by removing the setter you force this immutability (at least for that field). A class where you have removed all setters and state modifying methods produces immutable objects.

The benefits of immutability are out of the scope of this article, and it's not said that the object will reach immutability just by eliminating a single setter. However, it can reach it with the removal of many setters and in this sense this refactoring is a basic tool.

Steps

1. Check the calls to the setter: it should only be called inside the constructor, or never called at all. The target is to make the field immutable: a never called setter is a prerequisite.
2. If there is a call in the construction phase, modify the code to access the field directly.
3. If the setter is called immediately after new(), move the value as a parameter of the constructor. This step covers the usage of setters for Dependency Injection.
4. Remove the setting method.

The test suite should pass all the time, never breaking.

Example

The Question presenter object should render as the title of the question, to use in a link or a list element for example. The setter is called during construction to ensure the text is properly trimmed (trailing spaces are throw away.)

<?php
class RemoveSettingMethod extends PHPUnit_Framework_TestCase
{
    public function testAQuestionIsDisplayedTogetherWithTheNumberOfResponses()
    {
        $question = new Question('How do I install PHP on the washing machine?', 10);
        $this->assertEquals('How do I install PHP on the washing machine? (responses: 10)', $question->__toString());
    }
}

class Question
{
    private $text;
    private $responses;

    public function __construct($text, $responses = 0)
    {
        $this->setText($text);
        $this->responses = $responses;
    }

    public function setText($text)
    {
        $text = trim($text);
        $this->text = $text;
    }

    public function __toString()
    {
        return $this->text . ' (responses: ' . $this->responses . ')';
    }
}

However, we notice that this is just an object created for rendering, not a domain one; we will never have to change its text (we would just create a new object). The setText() method provides a possibility that we don't need nor want: let's eliminate it, starting by the places where it is called.

class Question
{
    private $text;
    private $responses;

    public function __construct($text, $responses = 0)
    {
        $text = trim($text);
        $this->text = $text;
        $this->responses = $responses;
    }

    public function setText($text)
    {
        $text = trim($text);
        $this->text = $text;
    }

    public function __toString()
    {
        return $this->text . ' (responses: ' . $this->responses . ')';
    }
}

Now the setter is not called anymore. We can remove it altogether.

class Question
{
    private $text;
    private $responses;

    public function __construct($text, $responses = 0)
    {
        $text = trim($text);
        $this->text = $text;
        $this->responses = $responses;
    }

    public function __toString()
    {
        return $this->text . ' (responses: ' . $this->responses . ')';
    }
}

The object is now formally immutable, and can be passed everywhere without worrying about aliasing and what the client code will do to it. The class also has fewer methods to maintain.