CODEWITHSUNDEEP
phpimmutability
laketuna
laketuna

Reputation: 4080

Letting a setter set a value of a property belonging to another class

Should a setter always be setting values of its direct properties and not of properties of associated objects? For example, I have something like below where the API client takes in an XML:

Class XML {
    private $value;

    // Setter for $value exists here
    ...
}

Class Client {
    private $xml; // XML
    public function __construct(XML $xml)
    {
        $this->xml = $xml;
        ...
    }    
}

I want the $value modified after Client is instantiated because, say, I want to change or add some options to the request. Should I use the setter in XML and re-instantiate the Client when modifying requests, or have a setter in Client directly so that I don't need to re-instantiate Client?

I'm typically prefer the former, but is there a reason why the latter approach would be preferred? I can't think of any except that it's easier to make quick changes.

Upvotes: 0

Views: 55

Answers (1)

lxg
lxg

Reputation: 13107

Generally, if a class has public methods, they are meant to be accessed. So, no harm in using them, you might think.

However, ask yourself: Why does a class have getters/setters in the first place? Accessors are usually found on classes for data objects. The main purpose of such objects is to carry data, they usually don’t do anything – unlike, for instance, a service or a controller class.

Next question: What is a constructor for? A constructor is used to set up the internal state of an object, and it will require it’s dependencies as parameters. In the spirit of loose coupling, dependencies should not be mutable from other classes.

Looking at your code, I guess that you’re writing some sort of XML-based client application and you want the Client class to depend on an instance of the XML class.

Modifying the internal state of the XML object from Client is a bad idea. Other classes depending on that instance will not know about the modification and will possibly get unexpected results.

There are several ways to solve this, depending on the actual task to perform:

1) If you need to set the value only for the context of a single method call of XML, modify the method to take $value as a parameter. But do not set it as an object member inside XML!

class XML
{
    public function doSomething($value)
    {
        …

        // do whatever you need with $value, 
        // but do *not* do something like $this->value = $value
    }
}

2) If the interaction with $value needs to be consistent across several operations, you should extract these operations from XML into an own worker or data object class. XML should then have a factory method to create an instance of this helper class. Then either XML or Client might have methods to use the worker:

class XML
{
    public function createWorker($value)
    {
        return new XmlWorker($value, $this);

        // XmlWorker might even contain a reference to the XML instance, e.g.
        return new XmlWorker($value, $this);
    }

    public function doSomethingWithWorker(XmlWorker $worker)
    {
        $value = $worker->getValue();
        $worker->doWork();
    }
}

class Client
{
    private function doSomething()
    {
        $worker = $this->xml->createWorker($value);
        $worker->doWork();
    }
}

I would recommend to read up on Design Patterns; in this context especially about Loose Coupling & Separation of Concerns, Factory methods and Depedency Injection.

Upvotes: 1

Related Questions