“Complex ifs” means anemic models

One of the paragraphs on my last post commented how someone could easily simplify an if: just extracting a variable.

Today, while waking up, I was thinking how to try to simplify all if conditions. Let’s imagine that I have a simple if which checks the condition of a boolean variable:

isReady = ...
if isReady {
do something
}

This is the most comprehensible way to write an if statement. But what about the other ways that we express our ifs? Do I always need to extract variables? What else can I do?
Well, the second most comprehensible if statement is the one which invokes a boolean returning method, which name is usually quite comprehensible:

if client.isReady() {
do something
}

Nice… just as comprehensible as the first example. But what about everything else? Well, it seems to me that every other type of if statement is more complex than it should be, meaning that for me the rule of thumb would be to: “keep your if statement to using only a boolean variable or a boolean returning method”.

What about the “not” modifier? Does it get nasty if we use it? Not if used with the first two examples:

if !client.isReady() {
do something
}

Let’s try another example, where a method returns an integer:

if client.minutesSinceLastAccess() > 30 {
time out this client
}

Well, it is quite easy to read this sentence, although not as easy as the first two examples. But instead of providing access to the “minutesSinceLastAccess” (through a property getter method) we could simply refactor it to tell me whether the client has expired:

if client.hasExpired() {
time out this client
}

This way, the magic number “30” moved within the client type, where it belongs: the client knows when it has expired, no one else should know those rules. Fair enough, right?
From this example, no ifs should be written which make use of methods returning anything appart from boolean.

Well, am I being too strict? What about those situations where we want to use &? For example:

if !client.hasExpired() & client.isFree() {
do something with the client
}

Wow… nasty! Can you see the use of the not operator with the and operator? It gets nasty when trying to read it, meaning that the human being reading it for the first time is more error-prone than reading an if statement as described in the first three examples.
So how do we make that code less error-prone?
Can you notice that both conditions only depend on the state of the object pointed by the client variable? Thats the trick… both conditions should be contained within the client type:

if client.isAvailable() {
do something with the client
}

And the client itself should contain a method which checks for both conditions. Notice that this way its even easier to write unit tests for both parts: this code has only one extra condition to be tested (true or false: totalling two simple tests) and the newly created method has 4 different paths meaning 4 easy tests (with two different outputs).

In a anemic model, the ifs get more complex as seen in the previous example and therefore the tests gets complex too. An easy way to detect such nasty ifs is when you write a test and you have to write another few big test cases for the same method just because that method has a if (a & b).

But what about using the or (|) operator? The same rule as with the & operator applies. One can easily refactor out the code to move it to your model.

“Aha! Got you… I have an example where it’s not so easy to refactor the code into the model:”

if client.isAvailable() & job.shouldRun() {
run the job at this client
}

What should we do in this case? Well, you are checking for two totally unconnected conditions with two different models. client.isAvailable() works completely unaware of job.ShouldRun() (and the other way around) and if they can be invoked synthetically independently, but you require but of those informations you could go to one of those two refactorings.

Check how did you get access to those two variables, job and client. If your method receives as a parameter just one of those variables and does a for to access the other one:

define run(client) {
for ( job in jobs) {
if(client.isAvailable() & job.shouldRun()) {
run the job in this client
}
}
}

In this case, you should have checked for the client status earlier on either with an earlier return or just a check:

define run(client) {
if(!client.isAvailable()) {
return
}
for ( job in jobs) {
if(job.shouldRun()) {
run the job in this client
}
}
}

If your method receives only one of them as a parameter and uses a member variable to access the other one, I am still thinking what one can do. Any suggestions?

If you method receives only one of them as a parameter and invokes another method to access the other one:

define run(client) {
job = jobs.gimmeIt()
if(client.isAvailable() & job.shouldRun()) {
run the job in this client
}
}

You could have done the earlier return strategy again.

If the other method invocation actually retrieves a sequence of jobs:

define run(client) {
jobs = jobs.all()
for(job in jobs) {
if(client.isAvailable() & job.shouldRun()) {
run the job in this client
}
}
}

You could have make the method retrieving this sequence only retrieve the ones you require. And again, easier testing:

define run(client) {
if(!client.isAvailable()) {
return
}
jobs = jobs.theOnesWhichShouldRun()
for(job in jobs) {
run the job in this client
}
}

Well, that probably concludes all possibilites of accessing those two variables. But what happens if both variables are actually used together to find the condition when your if statement should run?

if client.isFree() & job.canBeRunAt(client) {
}

Let’s remove the and operator by simply changing the order of those variables:

if client.isReadyToRun(job) {
run this job
}

Thats much clearer than the previous example. But what if the condition is more complex and involves invoking other conditions on those variables? Remember that it can always be refactored to invoke one condition in each varible. The most amazing refactor would be once you find out that actually a client and a job means something to your project.

If many of your if conditions keep using two types of variables, it means that they represent something (probably stateful) that you want to keep track in many different parts of your application. In our example, a job and a client would represent a ScheduledJob:

define ScheduledJob {
client, job
define canStart() {}
define start() {}
define stop() {}
}

In this case, only after creating a projects code and putting it into production, one was able to find out that a ScheduledJob is actually a different thing than a Job.. (being able now to rename Job to Task, for example).

Summing up, one can always refactor our ifs to make them only use a simple condition: a boolean variable or a boolean returning method. By following this rule, your code is much easier to read and contains less complex conditions, and more easy conditions to test making it more easy to test.

As Kung has noted, this can be all seen as a consequence of the Law of Demeter. A rule should only become a rule after you show why it would improve your code, and those if examples might have shown how your code would get better if you just refactor out any if complexities.

I should also mention the anti if campaign which fights for better usage on ifs. Refactor is one way to improve them (others include other oo patterns).