Expressing intent using fluent code

I love this joke about IT consultants. The short version goes…

An IT consultant drives next to a farm and tells the farmer: “I bet I can tell you how many sheep you have in your field. If I win I can take one home”. So he does: he takes out his laptop, connects to a NASA satellites, grabs a picture of the field, fires up his algorithm that recognizes and counts sheep and comes back with the correct result. The farmer, true to his word, invites him to grab one.
As the consultant is busy pushing the animal into its car the farmer says: “I bet I can guess what your job is. If I win you give me back my animal”. Done deal. The farmer says without hesitation “You’re an IT consultant”. “How did you know?” says the consultant. And the farmer replies: “You came here when nobody asked you. You gave me an answer I knew already and you don’t know sh!t about my business. Now, please, give me back my dog”.

Expressing intent using fluent code

As a developer, I see myself as a professional who tries to bring as much value as possible to the client. I hope you do too! And sometimes that value lies in the details.

For example, I recently was working on a client’s (very) old code base and found this:

public class SomeBatchJob {
    ...
    public void doJob() {
        ...
        if (enterpriseLinkDao.getLink(office, employee, new Date()) != null) {
            ...
        }
    }
}

Looks pretty simple, right? Or maybe not as simple as it should look? What sort of “enterprise link” are we talking about? And what does that “new Date()” mean?

I see a great opportunity to clarify that out.

Everything in its right place

Let’s start by extracting that tiny bit of logic into its own method. We get this:

public class SomeBatchJob {
    ...
    public void doJob() {
        if (enterpriseLinkExists(office, employee, new Date())) {
            ...
        }
    }

    private boolean enterpriseLinkExists(Office office, Employee employee, Date date) {
        return enterpriseLinkDao.getLink(office, employee, new Date()) != null;
    }
}

This may look trivial, but try and attempt reading that method call inside the doJob() method: we have stated the intent of the expression that checks if a result from a DAO method is null. We now express that we are checking whether an enterprise link exists.

But we can do better. Namely: is it really the place where that logic should be? Couldn’t we move that out of that batch job and set it in a class that deals with enterprise links? Let’s try that:

public class SomeBatchJob {
    ...
    public void doJob() {
        if (enterpriseLinks.areLinked(office, employee, new Date())) {
            ...
        }
    }
}

public class EnterpriseLinks {
    private EnterpriseLinkDao enterpriseLinkDao;
    public EnterpriseLinks(EnterpriseLinkDao enterpriseLinkDao) {
        this.enterpriseLinkDao = enterpriseLinkDao;
    }
    public boolean areLinked(Office office, Employee employee, Date date) {
        return enterpriseLinkDao.getLink(office, employee, new Date()) != null;
    }
}

Okay, we’re getting somewhere. The logic has just been made reusable and available to all: DRY that! And we now can guess where methods to check enterprise links are: in an EnterpriseLinks class!

Yeah… but there’s still room for improvement, right? That line if (enterpriseLinks.areLinked(office, employee, new Date())) is still kind of hard to read.

Plus we are still a bit puzzled about that “new Date()” thing… After little more archaeology it turns out that it’s the date from which that employee / office link exists. In most cases, the logic wants to know if that link exists currently. And as you imagine, they write the call and repeat that “new Date()” instantiation every single time. Eek!

How would you like to read that?

The core question every professional developer should ask himself is: “If I was reading this code, how would I like it to be phrased?“.
In other terms, we really want to make the next developer happy, and make it easy for him to read our code. Who knows, he may be a psychopath that knows where you live!

One way we could ideally express our intentions better is: if (employee.isCurrentlyLinkedTo(office)) .
That looks nice enough, but we still have one issue with that. The Employee class is likely a domain model or an entity, so we’d probably rather not have it depend on a DAO (which we will need for our code). Bummer!

What about if (employee(employee).isCurrentlyLinkedTo(office)) ? How does that sound? I think it would make very clear what the intent is, don’t you think! So what does that employee() method imply?

public class SomeBatchJob {
    private Employees employees;

    @Autowired
    SomeBatchJob(Employees employees) {
        this.employees = employees;
    }
    ...
    public void doJob() {
        if (employee(employee).isCurrentlyLinkedTo(office)) {
            ...
        }
    }
    
    private Employees employee(Employee employee) {
        return employees.for(employee);
    }
}

public class Employees {
    private EnterpriseLinkDao enterpriseLinkDao;
    private Employee employee;

    @Autowired
    Employees(EnterpriseLinkDao enterpriseLinkDao) {
        this.enterpriseLinkDao = enterpriseLinkDao;
    }
    
    public for(Employee employee) {
        this.employee = employee;
    }

    public boolean isCurrentlyLinkedTo(Office office) {
        return enterpriseLinkDao.getLink(office, employee, new Date()) != null;
    }
}

What’s in it for us?

That’s a lot of code, considering the statement we started with. But why don’t we go through all the benefits this approach brings us?

• For one, the logic of checking whether the employee and the office are linked is now in its own class. We introduced a cohesive class named Employees, that we naturally query for employee-related matters. Such as whether the employee is linked to a given office. Why the name “Employees”? We could have named the class “EmployeeService” or “EmployeeUtil”, for example. That’s up to you, but I like to try and keep it short and evident, for example by using the plural of the domain model’s name to define classes that work for that model. Again: ideally I would try and set the logic in the model, but in this case we have a DAO dependency which I want to keep separate.
• We have hidden the recurring “now” date: we have named our method accordingly: “isCurrentlyLinkedTo()” clearly tells you that the link situation we’re fetching is the current one.
• We have clarified the purpose of the SomeBatchJob.doJob() method by clearly expressing the intent inside the method’s code. That statement is not so technically-oriented anymore: it declares its purpose.
  I find that precious, my friends: the next time you or some psychopath developer have to go through that code, you won’t spend as much time figuring out what it is meant to do. But you will instead focus on what you have to implement. How about that, eh!

Conclusions

When I refactored and committed that code, one of the “junior” developers initially expressed his disapproval: “Why did you spread the logic around? Now I’m confused!”.

I can relate to his reaction: I’ve added quite a few more lines of code… without even considering the tests to go with it. However I have seen his position change as I walked him through the reasoning behind all this. Even when considering the greater amount of code, the way it becomes fluent, easy to read and reusable can extend the life of a project!

As a consultant, I know I must push for this approach because it actually brings value to my clients, in the short and in the long term.

In other words: instead of bringing an answer he didn’t ask I… try to help that farmer taking care of his herd.

Until next time,

Cheers!

PS: there are probably a thousand more solutions for writing fluent, readable code. More than ever, feel free to share your knowledge in the comments below!