[Twisted-Python] composition VS. inheritance
![](https://secure.gravatar.com/avatar/65e6f25d74f30fc14f39a16027babf10.jpg?s=120&d=mm&r=g)
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? Ian
![](https://secure.gravatar.com/avatar/e589db6c27c54b03de756cae2843dba5.jpg?s=120&d=mm&r=g)
On Sat, Jun 27, 2020 at 8:48 AM Ian Haywood <ian@haywood.id.au> wrote:
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/7d6cc51bd6aeba735c6fb081cf042d1... - _ComposedIRCClient subclass https://github.com/pound-python/infobob/blob/7d6cc51bd6aeba735c6fb081cf042d1... - _IRCClientState https://github.com/pound-python/infobob/blob/7d6cc51bd6aeba735c6fb081cf042d1... 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
![](https://secure.gravatar.com/avatar/65e6f25d74f30fc14f39a16027babf10.jpg?s=120&d=mm&r=g)
On 1/07/2020 1:41 am, Barry Scott wrote:
thanks for these pointers everyone. I have changed the code in my PR to remove the subclass-of-a-subclass. I'm now looking for a committer who is able to review it again. https://github.com/twisted/twisted/pull/1274 Ian
![](https://secure.gravatar.com/avatar/a93db92c60e9d5435d204407264457f2.jpg?s=120&d=mm&r=g)
As others have pointed out, you're stuck with a certain amount of inheritance to use Twisted. However, this can be fairly minimal. For new code trying to follow a "sans-io" approach is good too. The go-to example of this for Twisted is Hyper/H2 and its integration into Twisted Web for HTTP2 support. For nearly every protocol you have similar pieces: framing, parsing, a state-machine and some way to send and receive bytes ("do I/O"). Keeping these separate has a lot of value. The only part of the above where you "need to use inheritance" is the I/O part ("because Twisted"). You don't even technically need to "subclass" because you could just implement the right interfaces .. but in practice, you subclass Protocol. However, you can keep this looking like "just a shim": class Shim(Protocol): def connectionMade(self): # self.transport is valid now self._framing.startup() def dataReceived(self, data): self._framing.feed_data(data) def connectionLost(self, reason): self._framing.shutdown() (This is just one sort of suggestion / psuedo-code here). So your "framing" object collects bytes and when it has "a message" it can pass it along to the parser which validates it and turns it into a thing that it can pass along to the state-machine. All the state-machine needs is some way to indicate "I have messages to send" (e.g. a callback?) which is turned into bytes and you call "shim_instance.transport.write()" with those bytes. So all those "other" pieces can be written without inheritance. At least the parser and serializer can probably just be functions. I recently wrote a relatively simple protocol to try this "sans-io" style and was happy to start with the state-machine (using Automat). This forced some good decisions. Then I worked outwards to the "more boring" bits (framing and serialization/parsing). (Hmm, this should almost just be a blog-post maybe ;) -- meejah
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
One thing I should mention is that while there aren't necessarily great examples of stacking protocols like this at the parsing layer, some parts of Twisted do work by delegation and composition. For example, in twisted web, Resource objects are thoroughly disconnected from the protocol layer, rather than, say, the various 'irc_' methods exposed in the IRC server. So another way to look at this is to extend things like the Resource pattern down into the parser, rather than trying to think of a way to take apart the Protocol object in a wholly novel way. -g
![](https://secure.gravatar.com/avatar/e589db6c27c54b03de756cae2843dba5.jpg?s=120&d=mm&r=g)
On Sat, Jun 27, 2020 at 8:48 AM Ian Haywood <ian@haywood.id.au> wrote:
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/7d6cc51bd6aeba735c6fb081cf042d1... - _ComposedIRCClient subclass https://github.com/pound-python/infobob/blob/7d6cc51bd6aeba735c6fb081cf042d1... - _IRCClientState https://github.com/pound-python/infobob/blob/7d6cc51bd6aeba735c6fb081cf042d1... 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
![](https://secure.gravatar.com/avatar/65e6f25d74f30fc14f39a16027babf10.jpg?s=120&d=mm&r=g)
On 1/07/2020 1:41 am, Barry Scott wrote:
thanks for these pointers everyone. I have changed the code in my PR to remove the subclass-of-a-subclass. I'm now looking for a committer who is able to review it again. https://github.com/twisted/twisted/pull/1274 Ian
![](https://secure.gravatar.com/avatar/a93db92c60e9d5435d204407264457f2.jpg?s=120&d=mm&r=g)
As others have pointed out, you're stuck with a certain amount of inheritance to use Twisted. However, this can be fairly minimal. For new code trying to follow a "sans-io" approach is good too. The go-to example of this for Twisted is Hyper/H2 and its integration into Twisted Web for HTTP2 support. For nearly every protocol you have similar pieces: framing, parsing, a state-machine and some way to send and receive bytes ("do I/O"). Keeping these separate has a lot of value. The only part of the above where you "need to use inheritance" is the I/O part ("because Twisted"). You don't even technically need to "subclass" because you could just implement the right interfaces .. but in practice, you subclass Protocol. However, you can keep this looking like "just a shim": class Shim(Protocol): def connectionMade(self): # self.transport is valid now self._framing.startup() def dataReceived(self, data): self._framing.feed_data(data) def connectionLost(self, reason): self._framing.shutdown() (This is just one sort of suggestion / psuedo-code here). So your "framing" object collects bytes and when it has "a message" it can pass it along to the parser which validates it and turns it into a thing that it can pass along to the state-machine. All the state-machine needs is some way to indicate "I have messages to send" (e.g. a callback?) which is turned into bytes and you call "shim_instance.transport.write()" with those bytes. So all those "other" pieces can be written without inheritance. At least the parser and serializer can probably just be functions. I recently wrote a relatively simple protocol to try this "sans-io" style and was happy to start with the state-machine (using Automat). This forced some good decisions. Then I worked outwards to the "more boring" bits (framing and serialization/parsing). (Hmm, this should almost just be a blog-post maybe ;) -- meejah
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
One thing I should mention is that while there aren't necessarily great examples of stacking protocols like this at the parsing layer, some parts of Twisted do work by delegation and composition. For example, in twisted web, Resource objects are thoroughly disconnected from the protocol layer, rather than, say, the various 'irc_' methods exposed in the IRC server. So another way to look at this is to extend things like the Resource pattern down into the parser, rather than trying to think of a way to take apart the Protocol object in a wholly novel way. -g
participants (5)
-
Barry Scott
-
Colin Dunklau
-
Glyph
-
Ian Haywood
-
meejah