Avris

Keeping your classes immutable and stateless makes your code way less prone to bugs. Yet somehow this clean code rule isn’t as popular and as often invoked as SRP, YAGNI, DRY, KISS and others... Maybe it’s because of the lack of a catchy acronym?

Anyways, I’d like to take a look at two examples of when sticking to this rule could save your ass (or at least save you some time debugging).

DateTime

PHP offers a DateTime object that encapsulates, well, a date and time. And it’s pretty cool: it can parse strings like “next monday”, it can understand instructions like modify “+1 week midnight”, the standard library offers support for time zones and for iterating over time (like from “now” to “+1 day” every “+30 minutes”).

But there’s a huge catch involved: DateTime is mutable.

Let’s say you have a User entity:

/**
 * @ORM\Entity
 * @ORM\Table
 **/
class User
{
    /**
     * @var \DateTime
     * @ORM\Column(type="datetime")
     */
    private $createdAt;

    // many other fields...

    public function __construct()
    {
        $this->createdAt = new \DateTime();
    }

    public function getCreatedAt(): \DateTime
    {
        return $this->createdAt;
    }
}

It looks as if you could be sure that createdAt will be stored in the database as exactly the moment when the entity was first created, right? After all there’s no setter given, so unless you hack around with reflections, there’s no way to modify that value ever again, is there?

Well, let’s say in your app it’s important to display, how many Sundays have passed since some stuff happened, and one of them is the registration of a given user.

public function countSundays(\DateTime $datetime): int
{
    $datetime->modify('next sunday 23:59:59');
    $sundays = 0;

    while ($datetime < new \DateTime('now')) {
        $sundays++;
        $datetime->modify('+1 week');
    }

    return $sundays;
}

At first glance this code seems fine as well. We get some value, we use it to calculate some other value, and we return the result. But notice that we keep modifying the input data! If you did it with a string, int or an array, the input would not be modified at all (unless explicitly passed as reference, &$string). But \DateTime, although it’s just a simple value wrapped in a class, is a class after all, so it’s always passed by reference.

So even though you cannot set User’s createdAt to a different object, you can pass that object to some function that will modify the inside of that object. So if you happen to save your User entity to the database after its number of Sundays has been calculated, you will end up with all the users having been registered in the future and constantly changing their registration date.

It might be quite difficult to find the source of this bug. After all, there is no setter, and the connection between this date and countSundays might not be too obvious – maybe hidden in a Twig extension or an external library...

How to mitigate that? Ideally, use DateTimeImmuttable, which “behaves the same as DateTime except it never modifies itself but returns a new object instead”.

Or if you don’t want to adjust your whole project to use DateTimeImmuttable, you might instead remember to clone a DateTime variable before passing it anywhere.

Btw, in the Rust programming language all variables are immutable by default 😍

Invoices import

I had to work on a code like this recently:

invoiceService = new InvoiceService
shop = new Shop

invoices = shop.loadInvoices

for each invoice in invoices {
    invoiceService.saveInvoice(invoice)
}

And somewhere inside saveInvoice, this function was executed:

function runSQL(sql) {
    result = database_exec(sql)
    if result is false {
        log(database_error)
        die
    }
}

The code was PHP, but it’s such a govnokod that the above pseudocode with ~90% of original logic removed, is the best way to show it without crying...

So the problem was: no invoices are generated anymore. Can you see why? If any invoice fails, the script just logs the error and just dies. When it runs again, it tries with the same invoice and dies again, over and over again, never even trying to handle any other invoice.

The underlying error was easy to find and reasonably easy to fix. But we wanted to also make that old, shitty code just a tiny bit better – if one invoice fails, it shouldn’t block all the others anymore.

Instead of die-ing, the runSQL would throw an exception. All the other places, where runSQL is used, would basically work as they used to (uncaught exception would terminate the script anyway), but on loading the invoices we would just catch the exception and carry on with all the other invoices.

What could possibly go wrong?

Well, if you do it on Friday, while starting to get a bit sick, and with InvoiceService class consisting of >1000 lines of messy, overcomplicated code – a lot can go wrong.

All because InvoiceService is stateful. That means running invoiceService.saveInvoice(invoice) might result in different stuff actually being done, depending on the context it was run in. In this case, the class was keeping invoice items (and other stuff) as a private object property – first loading them from somewhere, then saving to the database, and finally cleaning it up.

Obviously, with the try-catch, the last step wouldn’t be executed. So if invoiceService.saveInvoice(invoice1) fails, then invoiceService.saveInvoice(invoice2) wouldn’t really save the invoice2, as name suggests – it would load invoice2 plus some failing items from invoice1... The errors pile up, the invoice items accumulate, and we end up with hundreds of invalid (empty) invoices with the last one claiming that someone owes us 1,9 mln €...

Always immutable and stateless?

Obviously not. Some types of classes you expect to be stateful. What would be the point of a Builder, if it couldn’t hold an internal state? How to implement Events elegantly, if they were immutable?

Form libraries like Symfony Forms or Avris Forms require you to provide getters and setters for your entity – and it reflects how your database probably works: a user might change their username or avatar, so the programmer shouldn’t expect them to be immutable. (Unless you’re using Event Sourcing, then good for you!).

Basically, it’s all about what you would expect from a class, see Principle of least astonishment.

A simple data value objects, like date and time, or monetary value should consistently represent the same value. Services that offer a method handleFoo(Foo) should behave exactly the same for the same Foo, regardless of when were they triggered.

Entities though can be mutable in your project, and that’s ok. Builders, by their nature, are mutable, and that’s fine. You can even see that from the methods they provide: addFoo etc. suggest you shouldn’t expect immutability from them.

I think that Rust’s approach should be followed: immutability by default, mutability explicitly declared. In other terms: write immutable classes, unless you can find a good justification for making one mutable.

And when it comes to finding a nice acronym for this rule... how about JAM?

Justify All Mutants.