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: Remove Setting Method

12.06.2011
| 4485 views |
  • submit to reddit

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.

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

Comments

Pablo Morales replied on Tue, 2011/12/06 - 11:14am

Maybe is something special skill for the MS boys. But you need follow the code standing after talking about refactoring. http://pear.php.net/manual/en/standards.naming.php

Giorgio Sironi replied on Wed, 2011/12/07 - 5:30am in response to: Pablo Morales

PEAR coding standard are a bit outdated as the distinction between private and public by hungarian notation ($this->field and $this->_field) was necessary in PHP 4, but not anymore in PHP 5. Personally I follow a similar approach to the Symfony 2 coding standards:

http://symfony.com/doc/2.0/contributing/code/standards.html

Pablo Morales replied on Wed, 2011/12/07 - 9:52pm

Is not for necessary, is because pear + Zend Framework working for the rigth way to work. Pear + Zend Framework understand wich the best choice was use _ in private method and properties, is not outdated. Hungarian notation is when you put the type identification in the variable name, like iThisIsANumber, or sThisIsAString, http://en.wikipedia.org/wiki/Hungarian_notation If you don't like pear use Zend Framework codign standar :P http://framework.zend.com/manual/en/coding-standard.naming-conventions.html Or watch other language like python

Alayn Gortazar replied on Sun, 2011/12/11 - 11:28am

Wouldn't we achieve this inmutability simply restraining setter's visibility to protected? In this way we could also maintain a simpler and more readable constructor.

Giorgio Sironi replied on Sun, 2011/12/18 - 7:48am in response to: Alayn Gortazar

Indeed would be a change with a smaller impact. Since in this case the setter was a one-liner, so I felt compelled to apply Inline Method too.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.