On Sun, Oct 21, 2018 at 7:41 PM Rowan Collins <rowan.coll...@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/