Discarding setters

Setters, or mutators are used very commonly when working with classes. Almost every person who's ever written a class has also written some setter methods. I at least have written many! They are so common that even most IDEs provide a way to generate getter and setter methods from class properties. It's almost as if you're encouraged to write these sort of methods.

Newsflash: it's a terrible habit. Writing setter methods is a sure way to invite confusion and instability to the codebase. Setter methods are A SPAWN OF SATAN! (See, i avoided the usual "is evil" idiom!)

So, what's so bad about mutators? I need to be able to change the properties in my class somehow anyway, right?

The way i view this, is that setting stuff is all about configuration. I configure objects to some state by changing it's properties. The object is a computer program and i try to think like a computer.

This is a very normal approach in the field of software development. We have total control of these kinds of objects because we can change their inner state at will. But can we really handle the responsibility that comes with tracking that state in an application? This is a huge burden on any developer.

When we have setters, we pretty much also need getters to be able to check that state in different parts of the application. Checking stuff is great and all... Oh wait, i meant it's awful and i pretty much want to hand in my resignation every time i encounter an object whose state can be any combination of n properties.

/**
 * @param JobContract[] $contracts
 * @return Money
 */
public function totalSalaries(array $contracts): Money
{
    $total = new Money();
    foreach ($contracts as $contract) {
        if ($contract->getStatus() !== "active") {
            // continue, throw exception, anything
        }
        if ($contract->getType() !== "monthly") {
            // continue, throw exception, anything
        }
        if ($contract->getEmployee() === null) {
            // continue, throw exception, anything
        }
        if ($contract->getSalary() === null) {
            // continue, throw exception, anything
        }
        $total->add($contact->getSalary()->getAmount());
    }
    return $total;
}

Have you seen code like this in a code base you've worked? There's a huge mess of code that concerns only in deciding if it's possible to safely work with an object of JobContract.

Does setting things really happen a lot in the problem space we're modeling? I doubt it. This is an important shift in thinking. We shouldn't be thinking our models through their attributes. Instead, we should be thinking about their behaviour. When we understand how the objects should behave, we can then start to think about what they need to know to perform their advertised behaviour.

Consider terminating a JobContract:

class JobContract
{
    //...
    private $endDate;
    private $status;
    //...
    public function setEndDate(DateTime $endDate): void
    {
        $this->endDate = clone $endDate;
    }

    public function setStatus(string $status): void
    {
        if (!in_array($status, self::STATUSES)) {
            throw InvalidArgumentException("Invalid status {$status}");
        }
        $this->status = $status;
    }
}

There's no behaviour, just properties. State changes in uncontrolled manner. The business operation of terminating a contract is offloaded somewhere else. Some service decides that configuring the object this way represents a termination of a contract.

class JobContract
{
    //...
    private $endDate;
    private $status;
    //...
    public function terminate(DateTime $terminationDate): void
    {
        $this->endDate = clone $terminationDate;
        $this->status = self::STATUS_TERMINATED;
    }
}

Public API is simpler and reflects intent. State changes are encapsulated. Nobody knows or cares that the class even has a $status property. The business operation is explicit.

There are a lot of cases where the first instantiated status of an object is not the the final status. And there's always a way to change that status in some natural way that does not involve using a setter method.

Setter injection. That's a cool name for a bad practice. Setter injection basically is a form of configuring some object after it has been instantiated.

Something like this:

class HttpTransfer implements LoggerAwareInterface
{
    private $httpClient;
    private $logger;

    public function __construct(ClientInterface $httpClient)
    {
        $this->httpClient = $httpClient;
    }

    public function setLogger(LoggerInterface $logger): void
    {
        $this->logger = $logger;
    }
    //...
}

The purpose of setter injection is to provide a way to inject optional dependencies in to an object. There's for example a PHP-FIG approved PSR that uses setter injection (see LoggerAwareInterface). Or another one: Symfony's ContainerAwareInterface. Developers look up to these standards and frameworks as authorities for good practices so it's a shame that these kinds of things slip through the cracks.

Everything that may or may not be there when you need it adds a lot of unnecessary confusion. Once again, you have to null-check that property to make sure you can use the service. Are those dependencies really optional? Maybe there's some design considerations to do if that really is the case.

How to fix this? Inject everything via constructor. If in some environments you don't need logging, then configure so that you pass a fake instead. These "aware"-interfaces come also with interfaces representing that service. Such as ContainerInterface or LoggerInterface. So go ahead and implement your fakes. It's really that simple.

Services need to be stateless. Entities and value objects on the other hand need to be in a valid state ALL THE TIME. Programming is hard enough without us sabotaging ourselves. I know the subject of setters (and getters) have been written extensively but this issue still seems to persist. Thank you for reading.