本文章翻譯自 Marco Pivetta 所寫的 Fluent Interfaces are Evil

如果有任何問題,歡迎聯繫或者發 PR


今天,我在 IRC 上面討論為什麼 Doctrine EntityManager 沒有(未來也不會)實作流式接口。以下是我的想法以及說明。


提要:什麼是流式接口?

流式接口 是一個可以提供「可讀性更高」程式碼的設計。

概略地說,流式接口是如下的程式碼:

<?php

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

顯然的,因為 PHP 不支援回傳型態的宣告,所以我只能宣告一個 /** @return self */docblock

(譯註:現在支援回傳型態宣告了,不過這跟本文無關。)

流式接口讓你可以串接方法的呼叫,當你對同一個物件進行多個操作時,可以輸入更少的字:

<?php

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

什麼時候流式接口有道理?

流式接口在某些 API 上面使用很合理,像是 QueryBuilder,或者其他的 builder 都很適合。特別是用來將各個節點放進某個階級結構裡面。

下面是一個使用流式接口的好範例:

<?php

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

流式接口有什麼問題?

我發現了流式接口的一些問題。以下依據重要性列出這些問題:

  1. 流式接口破壞封裝
  2. 流式接口破壞裝飾者模式,有時會破壞合成。
  3. 流式接口比較難 Mock
  4. 流式接口讓差異比對更難讀
  5. 流式接口讓可讀性更差(個人意見)
  6. 流式接口在開發早期,會破壞向下相容

流式接口破壞封裝

流式接口的概念基於以下假設:

在流式接口裡面,方法的回傳值,會是運行該方法的物件本身。

首先,假設語言設計上無法保證的事情,本身就是個問題。

另外。在物件導向程式裡面,你只能確定介面,並不能真正確定回傳值的身份。

什麼意思呢?我們用 Counter 介面做範例:

<?php

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

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

下面是這個介面流式接口的實作方式:

<?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.


結論

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.