[Twisted-Python] composition VS. inheritance

[Colin Dunklau]

On Sat, Jun 27, 2020 at 8:48 AM Ian Haywood <ian at haywood.id.au> wrote:

> In smb I have a SMBPacketReceiver that inherits from t.i.p.Protocol, it
> breaks the incoming TCP stream into logical packets (the analogue of
> LineReceiver in line-based protocols).
>
> I then subclass SMBPacketReceiver to SMBProtocol which does a lot of the
> "heavy lifting" analyzing incoming packets.
>
> I've been told in code review to use composition instead of inheritance,
> which is fine in a general sense but I have difficulty applying to
> twisted-specific tasks.
>
> 1. how to do Factory.buildProtocol? It has to return a t.i.p.Protocol,
> but with composition the Protocol object is a private variable of
> SMBPacketReceiver, in turn a private variable of SMBProtocol.
>
> 2. what to do instead of overriding Protocol.dataReceived and access
> incoming data if not allowed to subclass it?
>
> Now  its not that I cant think of workarounds to these two problems, but
> they're ugly
>
> Is there some recent twisted code using composition that I can look at?


Hi Ian,

Unfortunately we're stuck with inheritance in Twisted. Its developers made
the decision (nearly two decades ago?) to use inheritance (hindsight is
20/20 and all that) and that's permeated through the majority of the
codebase. It's quite difficult to even *imagine* a Twisted without it, let
alone make the necessary (huge and almost certainly breaking) changes to
shift to a composition-oriented design. The good news is that while you're
stuck with inheritance for interacting with the Twisted parts, you're free
to do whatever you want outside of that.

I suggest jumping out of the inheritance world as soon as possible; instead
of having a SMBProtocol inherit from SMBPacketReceiver, make the initial
Protocol subclass as general as possible -- maybe make it just deal with
framing. Then you wired the protocol up with some object(s) that the
protocol interacts with (and interacts with the protocol), and you have
your inheritance escape hatch.

I've been playing with this idea on infobob's functional tests, and will
probably use it in the long-due refactor of infobob's code itself:

- ComposedIRCController
https://github.com/pound-python/infobob/blob/7d6cc51bd6aeba735c6fb081cf042d1157fbc6ca/functional-tests/clients.py#L48
- _ComposedIRCClient subclass
https://github.com/pound-python/infobob/blob/7d6cc51bd6aeba735c6fb081cf042d1157fbc6ca/functional-tests/clients.py#L425

- _IRCClientState
https://github.com/pound-python/infobob/blob/7d6cc51bd6aeba735c6fb081cf042d1157fbc6ca/functional-tests/clients.py#L376

The protocol _ComposedIRCClient (an IRCClient subclass) deals with the
irritating bits of IRC and tells its attached _IRCClientState instance
about incoming protocol events (messages, joins, parts, etc). The protocol
is itself wrapped by ComposedIRCController which is the main interface for
other code to initiate actions which ultimately end up in a method call on
the protocol.

I hope that helps trigger some ideas in your head :)

Colin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20200628/faaeb58e/attachment.htm>