Marco Pivetta (Ocramius)

Fluent Interfaces are Evil

Today, I had again a discussion on IRC on why Doctrine's EntityManager doesn't (and won't) implement a fluent interface. Here are my thoughts on why that's the case.

Recap: What is a Fluent interface?

A Fluent Interface is an object oriented API that provides "more readable" code.
In general, the template for a fluent interface can be like following: 

<?php

interface {InterfaceName}
{
    /** @return self */
    public function {MethodName}({Params});
}

Obviously, PHP doesn't provide return type hints, which means that I limited myself to define a /** @return self */ docblock.

A fluent interface allows you to chain method calls, which results in less typed characters when applying multiple operations on the same object: 

<?php

$foo
    ->doBar()
    ->doBaz()
    ->setTaz('taz')
    ->otherCall()
    ->allTheThings();

When does a fluent interface make sense?

Fluent interfaces make sense in some APIs, like the QueryBuilder , or in general builder objects, especially when it comes to putting together nodes into a hierarchical structure.

Here's an example of good usage of a fluent interface: 

<?php

$queryBuilder
    ->select('u')
    ->from('User u')
    ->where('u.id = :identifier')
    ->orderBy('u.name', 'ASC')
    ->setParameter('identifier', 100);

What's the problem with fluent interfaces?

I've identified some issues while working with fluent interfaces. Here they are listed in descending order of relevance:

  1. Fluent Interfaces break Encapsulation
  2. Fluent Interfaces break Decorators (and sometimes Composition)
  3. Fluent Interfaces are harder to Mock
  4. Fluent Interfaces make diffs harder to read
  5. Fluent Interfaces are less readable (personal feeling)
  6. Fluent Interfaces cause BC breaks during early development stages

Fluent Interfaces break Encapsulation

The entire idea behind a fluent interface bases on an assumption:

In a Fluent Interface, the return value of a method will be the same instance on which the method was called.

First of all, "assuming" facts that are not safely constrained by the language is a mistake.
Additionally, in OOP, you cannot rely on the identity of the returned value of an object, but just on its interface.

What does that mean? Let's make an example with a Counter interface: 

<?php

interface Counter
{
    /** @return self */
    public function count();

    /** @return int */
    public function getCount();
}

Here's a fluent implementation of the interface:

<?php

class FluentCounter implements Counter
{
    private $count = 0;

    public function count()
    {
        $this->count += 1;

        return $this;
    }

    public function getCount()
    {
        return $this->count;
    }
}

Here's an Immutable implementation of the interface: 

<?php

class ImmutableCounter implements Counter
{
    private $count;

    public function __construct($count = 0)
    {
        $this->count = (int) $count;
    }

    public function count()
    {
        return new ImmutableCounter($this->count + 1);
    }

    public function getCount()
    {
        return $this->count;
    }
}

Here is how you use a FluentCounter: 

<?php

$counter = new FluentCounter();

echo $counter->count()->count()->count()->getCount(); // 3!

Here is how you use an ImmutableCounter: 

<?php

$counter = new ImmutableCounter();

$counter = $counter->count()->count()->count();

echo $counter->getCount(); // 3!

We managed to implement an immutable counter even though the author of Counter maybe assumed that all implementations should be mutable.

The same can be seen in the opposite direction: interface author may want to have all implementations immutable, but then people implement a mutable version of it.

Turns out that the only correct way of using such an interface is the "immutable" way, so: 

<?php

$counter = $counter->count()->count()->count();

echo $counter->getCount(); // 3!

This ensures that FluentCounter#getCount() works as expected, but obviously defeats the purpose of the fluent interface.

On top of that, there is nothing that the author of Counter can do to enforce either one or the other way of implementing the contract, and that's a limitation of the language itself (and it's most probably for good!).

None of the implementors/implementations are wrong. What is wrong here is the interface by trying to force implementation details, therefore breaking encapsulation.

Wrapping it up:

  • In OOP, a contract cannot guarantee the identity of a method return value
  • Therefore, In OOP, fluent interfaces cannot be guaranteed by a contract
  • Assumptions not backed by a contract are wrong
  • Following wrong assumptions leads to wrong results

Fluent Interfaces break Decorators (and Composition)

As some of you may know, I'm putting a lot of effort in writing libraries that generate decorators and proxies.
While working on those, I came to a very complex use case where I needed to build a generic wrapper around an object.

I'm picking the Counter example again: 

<?php

interface Counter
{
    /** @return self */
    public function count();

    /** @return int */
    public function getCount();
}

Assuming that the implementor of the wrapper doesn't know anything about the implementations of this interface, he goes on and builds a wrapper.

In this example, the implementor simply writes a wrapper that echoes every time one of the methods is called: 

<?php

class EchoingCounter implements Counter
{
    private $counter;

    public function __construct(Counter $counter)
    {
        $this->counter = $counter;
    }

    public function count()
    {
        echo __METHOD__ . "\n";

        return $this->counter->count();
    }

    public function getCount()
    {
        echo __METHOD__ . "\n";

        return $this->counter->getCount();
    }
}

Let's try it out with our fluent counter: 

<?php

$counter = new EchoingCounter(new FluentCounter());

$counter = $counter->count()->count()->count()->count();

echo $counter->getCount();

Noticed anything wrong? Yes, the string "EchoingCounter::count" is echoed only once!

That happens because we're just trusting the interface, so the FluentCounter instance gets "un-wrapped" when we call EchoingCounter::count().

Same happens when using the ImmutableCounter 

<?php

$counter = new EchoingCounter(new ImmutableCounter());

$counter = $counter->count()->count()->count()->count();

echo $counter->getCount();

Same results. Let's try to fix them:

<?php

class EchoingCounter implements Counter
{
    private $counter;

    public function __construct(Counter $counter)
    {
        $this->counter = $counter;
    }

    public function count()
    {
        echo __METHOD__ . "\n";

        $this->counter->count();

        return $this;
    }

    public function getCount()
    {
        echo __METHOD__ . "\n";

        return $this->counter->getCount();
    }
}

And now let's retry:

<?php

$counter = new EchoingCounter(new FluentCounter());

$counter = $counter->count()->count()->count()->count();

echo $counter->getCount();

Works! We now see the different EchoingCounter::count being echoed.
What about the immutable implementation? 

<?php

$counter = new EchoingCounter(new ImmutableCounter());

// we're using the "SAFE" solution here
$counter = $counter->count()->count()->count()->count();

echo $counter->getCount();

Seems to work, but if you look closely, the reported count is wrong. Now the wrapper is working, but not the real logic!

Additionally, we cannot fix this with a generic solution.
We don't know if the wrapped instance is supposed to return itself or a new instance.
We can manually fix the wrapper with some assumptions though: 

<?php

class EchoingCounter implements Counter
{
    private $counter;

    public function __construct(Counter $counter)
    {
        $this->counter = $counter;
    }

    public function count()
    {
        echo __METHOD__ . "\n";

        $this->counter = $this->counter->count();

        return $this;
    }

    public function getCount()
    {
        echo __METHOD__ . "\n";

        return $this->counter->getCount();
    }
}

As you can see, we have to manually patch the count() method, but then again, this breaks the case when the API is neither Immutable nor Fluent.
Additionally, our wrapper is now opinionated about the usage of the count() method, and it is not possible to build a generic wrapper anymore.

I conclude this section by simply stating that fluent interfaces are problematic for wrappers, and require a lot of assumptions to be catched by a human decision, which has to be done per-method, based on assumptions.

Fluent Interfaces are harder to Mock

Mock classes (at least in PHPUnit) are null objects by default, which means that all the return values of methods have to be manually defined: 

<?php

$counter = $this->getMock('Counter');

$counter
    ->expects($this->any())
    ->method('count')
    ->will($this->returnSelf());

There are 2 major problems with this:

  1. All fluent methods need explicit mocking
  2. We are assuming that a fluent interface is implemented, whereas the implementation may be immutable (as shown before)

That basically means that we have to code assumptions in our unit tests (bad, and hard to follow).

Also, we have to make decisions on the implementation of a mocked object

The correct way of mocking the Counter interface would be something like: 

<?php

$counter = $this->getMock('Counter');

$counter
    ->expects($this->any())
    ->method('count')
    ->will($this->returnValue($this->getMock('Counter')));

As you can see, we can break our code by making the mock behave differently, but still respecting the interface. Additionally, we need to mock every fluent method regardless of the parameters or even when we don't have expectations on the API.

That is a lot of work, and a lot of wrong work to be done.

Fluent Interfaces make diffs harder to read

The problem with diffs is minor, but it's something that really rustles my jimmies, especially because people abuse fluent interfaces to write giant chained method calls like: 

<?php

$foo
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
    ->addTab('tab');

Let's assume that a line is changed in the middle of the chain: 

$ diff -p left.txt right.txt

*** left.txt    Fri Nov  8 15:05:09 2013
--- right.txt   Fri Nov  8 15:05:22 2013
***************
*** 11,16 ****
--- 11,17 ----
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')
+     ->addBaz('tab')
    ->addTab('tab')
    ->addBar('bar')
    ->addBaz('baz')

Not really useful, huh? Where do we see the object this addBaz() is being called on?

This may look like nitpicking, but it makes code reviews harder, and I personally do a lot of code reviews.

Fluent Interfaces are less readable

This is a personal feeling, but when reading a fluent interface, I cannot recognize if what is going on is just a massive violation of the Law of Demeter, or if we're dealing with the same object over and over again.
I'm picking an obvious example to show where this may happen: 

<?php
return $queryBuilder
    ->select('u')
    ->from('User u')
    ->where('u.id = :identifier')
    ->orderBy('u.name', 'ASC')
    ->setFirstResult(5)
    ->setParameter('identifier', 100)
    ->getQuery()
    ->setMaxResults(10)
    ->getResult();

This one is quite easy to follow: getQuery() and getResult() are returning different objects that have a different API.
The problem occurs when a method does not look like a getter: 

<?php
return $someBuilder
    ->addThing('thing')
    ->addOtherThing('other thing')
    ->compile()
    ->write()
    ->execute()
    ->gimme();

Which of these method calls is part of a fluent interface? Which is instead returning a different object? You can't really tell that...

Fluent Interfaces cause BC breaks

Declaring an @return void type allows you to change the method behavior later on.

Eagerly declaring a @return self precludes that choice, and changing the signature of the method will inevitably result in a BC break.

Using void as a default return type allows for more flexible and rapid design while we are prototyping around a piece of our codebase.

Conclusion

I know the article is titled "Fluent Interfaces are Evil", but that doesn't mean it's an absolute.

Fluent interfaces are useful and easy to read in some contexts. What I am showing here is a set of problems that raise when inheriting them or making every piece of your code fluent.

I just want you to think carefully next time you want fluent interfaces in your libraries, especially about the downsides that I have just exposed.

You must have a very good reason to implement a fluent interface, otherwise it's just a problem that you are possibly dragging into your codebase.

Tags: php, oop, patterns