Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:103354
MIME-Version: 1.0
References: <0047f3c7-3e9e-ef6f-2d7a-e58c316ea751@gmail.com>
In-Reply-To: <0047f3c7-3e9e-ef6f-2d7a-e58c316ea751@gmail.com>
Date: Sun, 21 Oct 2018 23:42:39 +0200
Message-ID: <CADyq6sLROjTjFen+WiwuDeNOWB46vu9UUthncOC0hBBmA=MvEA@mail.gmail.com>
To: Rowan Collins <rowan.collins@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000b673aa0578c404de"
Subject: Re: [PHP-DEV] Composition over inheritance: native support for
 "delegating" methods?
From: ocramius@gmail.com (Marco Pivetta)

--000000000000b673aa0578c404de
Content-Type: text/plain; charset="UTF-8"

On Sun, Oct 21, 2018 at 7:41 PM Rowan Collins <rowan.collins@gmail.com>
wrote:

> Hi,
>
> I've been thinking about how the language could better support
> Composition patterns like Decorators and Proxies, using some simple
> syntax sugar not dissimilar to Traits.
>

I do not think that this syntactic sugar is needed:

 * If you have few methods, writing the delegation code is trivially done,
tested, statically analysed.
 * If you have a gazillion of methods, *something is really wrong* (you
can't decorate something with ~5+ methods without introducing some
unexpected side-effect - that code is already a bit too complex).
Implementing this proposal to aid those scenarios is probably aiding bad
API design.
 * If you need to proxy and add behavior, userland libraries already exist
(mostly `doctrine/common` at
https://github.com/doctrine/common/tree/a210246d286c77d2b89040f8691ba7b3a713d2c1/lib/Doctrine/Common/Proxy,
`goaop/framework` at https://github.com/goaop/framework/tree/2.2.0 and
`ocramius/proxy-manager` at
https://github.com/Ocramius/ProxyManager/tree/2.2.2): if you aren't happy
with those, a bit of `ReflectionClass#getMethods()` grunt work is just
fine. We're still talking about edge cases anyway.
 * If you are proposing this syntax for performance reasons, most current
delegation code has almost no noticeable overhead besides a new stack frame
(possibly something the engine could optimise), and when it is present, it
is to avoid I/O (much heavier anyway)

Example:
>
> class SpecialSessionHandlerDecorator implements
> \SessionHandlerInterface, SpecialInterface {
>      private \SessionHandlerInterface $delegatedHandler delegate {
>         close, destroy, gc, open, read, write
>      };
>
>      public function specialMethod() { ... }
> }
>

One problem with delegating calls this way, is that any new method
implemented by an ancestor will not be delegated by default. A better
solution would be to have a single `delegate` keyword that intercepts all
method calls for methods not declared in this class, as if `__call` was
implicitly implemented:

class SpecialSessionHandlerDecorator implements \SessionHandlerInterface,
SpecialInterface
{
    private \SessionHandlerInterface $delegateHandler delegate;
    public function specialMethod() { ... }
}

If you don't do it this way, you end up with a BC break any time an
ancestor defines new public API. Yes, in this case these are interfaces, so
it would already count as BC break, but if the parent is a class, adding
new methods becomes a BC break as soon as the class is not `final` (I
described it more carefully at
https://github.com/Roave/BackwardCompatibilityCheck/issues/111 - missed to
add that check there, so I'm glad that this issue reminded me of that).

Still, even if you do it this way, implicit delegation of any newly added
API may also not be wished, so it is an open-ended question.

Anyway, I do think that:

 1. the language already has all the tooling required to implement
decorators correctly
 2. the addition of a `delegate` functionality would cause more confusion
for something that is already really trivial to implement/test/read

The suggestion requires some stronger foundations/reasoning in order to be
turned into a valuable RFC.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

--000000000000b673aa0578c404de--