Opened at 2016-04-27T21:27:45Z
Last modified at 2016-04-27T21:42:49Z
#2782 new task
code reorg: less inheritance, more delegation/composition
Reported by: | warner | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | undecided |
Component: | code | Version: | 1.11.0 |
Keywords: | Cc: | ||
Launchpad Bug: |
Description
In the comments on PR270, meejah pointed out that Client.__init__ is doing a lot of work, and that it might be better to build these nodes with a function (rather than a class constructor) that is *given* the supporting objects (like an IntroducerClient, StorageServer, and !Tub), instead of creating those things itself. Glyph recently pointed me at an enlightening presentation known as The Talk from PyCon2013, that encourages composition over inheritance, which ties in.
I'm not exactly sure what this would look like, but we could start by either merging Node and Client into a single class, or passing Node in as an argument to the Client constructor. Then we might make a client-creating function that builds Storage Servers and Introducer Clients first, then passes them as arguments into the Client constructor.
The basic pattern for doing this refactoring would look like:
So the last thing is the hard part, especially to avoid a "massive refactor everything" type of branch/PR. I think the first step would be to simply look at dependencies. I don't feel I know enough about Node / Client to know if merging them is appropriate etc. It's fine to leave them as a subclasses for now I think -- especially because Node only really has a couple dependencies in the above model: a tempdir and a tub. So, these would have to be passed into Client too so that super() can get called properly.
It should also be possible to "start slow" and move things out one or two dependencies at a time. From a quick grep it doesn't look like Node() is ever constructed by itself, so we should be able to move its dependencies into args-to-Client without making a create_node factory-method.
So a start would be:
This would probably give a good idea of how this would look/work, and would even probably be land-able without anything else changing...From there, it would be a "small matter of programming" to do something similar for Client's dependencies, and a create_client() factory method.