流式接口是邪惡的
本文章翻譯自 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);
流式接口有什麼問題?
我發現了流式接口的一些問題。以下依據重要性列出這些問題:
- 流式接口破壞封裝
- 流式接口破壞裝飾者模式,有時會破壞合成。
- 流式接口比較難 Mock
- 流式接口讓差異比對更難讀
- 流式接口讓可讀性更差(個人意見)
- 流式接口在開發早期,會破壞向下相容
流式接口破壞封裝
流式接口的概念基於以下假設:
在流式接口裡面,方法的回傳值,會是運行該方法的物件本身。
首先,假設語言設計上無法保證的事情,本身就是個問題。
另外。在物件導向程式裡面,你只能確定介面,並不能真正確定回傳值的身份。
什麼意思呢?我們用 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:
- All fluent methods need explicit mocking
- 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.