(Version française: https://dev.to/fredbouchery/dx-setter-wither-and-mutants-4b44)
When I design my PHP code, I spend a lot of time thinking about its DX (Developer eXperience), which is why I appreciate "test before" because it allows me to think about how to use the code before implementing it.
Recently, I was designing a configuration class for a database access layer and here's how I initially planned to use it:
<?php
$conf = (new ConnectionConfig())
->connection('main')
->driver(DriverInterface::DRIVER_MYSQL)
->primary()
->host('127.0.0.1', 3306)
->credentials('root', 'root')
->replica()
->host('127.0.0.1', 3307)
->credentials('readonly', 'readonly')
;
The implementation is a fairly simple fluent interface:
-
connection(string $name)
sets the "$currentConnectionName
" property. -
primary()
orreplica()
sets the "$currentReplicationServer
" property. - Then, when
host()
orcredentials()
is called, I use the "$currentXxxx
" properties to store the value.
Next, I wanted to define how I would read these values, and I started writing this:
<?php
[
'login' => $login,
'password' => $password
] = $conf
->connection('main')
->primary()
->getCredentials()
;
First observation: Ouch... I was too lazy to define a "ServerCredentials
" object to store the login and password, even though an associative array would do just fine. Don't look at me with that stern expression, we've all done it... on lazy days, and as Krän would say: "There are days you shouldn't mess with me! And there are days every day!" But that's not the debate today; let's look at the next problem I want to highlight.
To read the credentials, I used a getCredentials()
getter, while to define it, I used the simple credentials($login, $pass)
method because I found it cooler for my DX.
Actually, from the point of view of the person who will retrieve the definition, it's not usual to have a getter and an assignment method that doesn't start with "set," and it can be confusing when the IDE autocompletes will suggest these two methods.
Obviously, a method with two parameters $login
and $pass
should be the method which assigned the value, but, I still preferred to change that with setters.
Yes, because when you design a good DX, you should not change too much the way developers do things! Even if your idea seems revolutionary, innovative, never done before, it can drastically harm your DX.
Here how I changed the definition:
<?php
$conf = (new ConnectionConfig())
->connection('main')
->setDriver(DriverInterface::DRIVER_MYSQL)
->primary()
->setHost('127.0.0.1', 3306)
->setCredentials('root', 'root')
->replica()
->setHost('127.0.0.1', 3307)
->setCredentials('readonly', 'readonly')
;
And then, a new difficulty came to me: mutants!
Look at this:
<?php
$serverIp = '127.0.0.1';
$login = 'root';
$password = 'root';
$conf = new ConnectionConfig();
$primary = $conf->connection('main')->primary();
$replica = $conf->connection('main')->replica();
// Define hosts
$primary->setHost($serverIp, 3306);
$replica->setHost($serverIp, 3307);
// Define credentials
$primary->setCredentials($login, $password);
$replica->setCredentials($login, $password);
Yes, we designed a way to use our code and developer will do it in a completely different way, it's his right.
However, in this case, there is a rather embarrassing bug: the primary server configuration is never defined. If you don't see why, I'll let you try to understand before reading the explanation that follows.
.
.
.
.
.
.
.
.
.
As I mentioned at the beginning, when we call primary()
or replica()
, we are setting a $currentReplicationServer
property, and since I am using a fluent interface, the $primary
and $replica
variables both contain the same object instance. When calling ->replica()
, the $currentReplicationServer
property will therefore be modified to indicate that we are configuring the replica, which also affects the instance in the $primary
variable (I'll let you reread if you didn't understand it well).
Now, we fall into a classic pitfall in PHP (and other languages): object mutability.
To solve this, auto-cloning is used, the property is changed, and the new instance is returned. So, we go from:
<?php
public function primary(): self
{
$this->currentReplicationConfiguration = 'primary';
return $this;
}
To:
<?php
public function primary(): self
{
$self = clone $this;
$self->currentReplicationConfiguration = 'primary';
return $self;
}
And this technique is applied to all methods, but... it doesn't work. Because in this case:
<?php
$conf = new ConnectionConfig();
$primary = $conf->connection('main')->primary();
$replica = $conf->connection('main')->replica();
What I want is that $conf
contains all the configuration elements, for the "main
" connection, with the primary server and the replica, and finally, we have 3 instances of the configuration class with dissociated values.
And that's the problem with fluent design coupled with stateful/stateless design: If the behavior of a method depends on the call of another one, you are in trouble!
There is a way to partially solve this, by making the developer understand that the method returns a new instance: we will replace the setters by withers:
<?php
$conf = new ConnectionConfig();
$primary = $conf->withConnection('main')->withPrimary();
Using the prefix "with
" indicates that the return will be a new instance of the object, so $conf
is not the same instance as $primary
. The developer must chain his calls otherwise he will not get the result he expects.
In reality, we haven't solved the problem with additional code, but simply by renaming it, because yes, naming is part of the design and DX.
The final code would be:
<?php
$conf = (new ConnectionConfig())
->withConnection('main')
->setDriver(DriverInterface::DRIVER_MYSQL)
->withPrimary()
->setHost('127.0.0.1', 3306)
->setCredentials('root', 'root')
->withReplica()
->setHost('127.0.0.1', 3307)
->setCredentials('readonly', 'readonly')
;
And, if we go back to the access, it would be:
<?php
[
'login' => $login,
'password' => $password
] = $conf
->withConnection('main')
->withPrimary()
->getCredentials()
;
Honestly, I don't think this DX is amazing because in my small brain, I tend to associate "withers" with building rather than access.
It's like squaring the circle, don't you think?
Actually, no, it's simply because I went the wrong way from the start, and it started when I wrote that I was lazy!
- My class should have been called "
ConnectionsConfiguration
" with an "s" and a complete word (Stop taking shortcuts, it's not 7 more characters that will impact your productivity) - There should only be one method "
connection(string $name)
" that returns a "ConnectionConfiguration
" object (without the "s") -
ConnectionConfiguration
has three methodssetDriver()
,primary()
, andreplica()
, the latter two returning mutable instances ofServerConfiguration
. -
ServerConfiguration
contains four methodssetHost()
,setCredentials()
,getHost()
, andgetCredentials()
managingServerHost
andServerCredentials
objects.
In the end, the code will be:
<?php
$conf = (new ConnectionsConfiguration())
->connection('main')
->setDriver(DriverInterface::DRIVER_MYSQL)
->primary()
->setHost('127.0.0.1', 3306)
->setCredentials('root', 'root')
->replica()
->setHost('127.0.0.1', 3307)
->setCredentials('readonly', 'readonly')
;
Except that with this code, I don't have a fluent interface anymore and ... I have a fatal error!
If you look carefully, after the setCredentials()
, I call replica()
, but this method does not exist, because setCredentials()
does not return a ConnectionConfiguration
but a ServerCredentials
. Also, the variable $conf
does not contain a ConnectionsConfiguration
object. In short, without a fluent interface, we would have to write like this:
<?php
$conf = new ConnectionsConfiguration();
$conf->connection('main')
->setDriver(DriverInterface::DRIVER_MYSQL)
->primary()
->setHost('127.0.0.1', 3306)
->setCredentials('root', 'root')
;
$conf->connection('main')
->replica()
->setHost('127.0.0.1', 3307)
->setCredentials('readonly', 'readonly')
;
It's less beautiful than a fluent call chain, but there is less risk of a bug.
For the access to the configuration, it gives :
<?php
$credentials = $conf
->connection('main')
->primary()
->getCredentials()
;
$login = $credentials->login;
$password = $credentials->password;
The most attentive may have noticed that my classes are mutable, and for some, it's heresy, scandal, a shame that should lead me to the stake! To those, I would reply:
Yes, but it would generate more problems than it solves and break my DX. So, I refer you to Krän's mantra: "There are days when you shouldn't mess with me! And there are days every day!".
In summary:
- Place yourself from the user's point of view and not the implementation's
- Don't innovate too much, people have patterns
- Don't be lazy
- Use immutability sparingly
- Be careful with fluent interfaces on stateful objects
- Read Krän, it's a delightfully insane comic book.
Top comments (0)