meejah / txtorcon

Twisted-based asynchronous Tor control protocol implementation. Includes unit-tests, examples, state-tracking code and configuration abstraction.
http://fjblvrw2jrxnhtg67qpbzi45r7ofojaoo3orzykesly2j3c2m3htapid.onion/
MIT License
250 stars 72 forks source link

Use factory-methods to create instances #133

Open meejah opened 9 years ago

meejah commented 9 years ago

Several classes use a Deferred attribute .post_bootstrap or similar so that calling code knows "when they're ready". This is pretty hacky and not very nice. JP Calderone has a good blog-post about this: http://as.ynchrono.us/2014/12/asynchronous-object-initialization.html

Instead:

So, for example, instead of creating a TorConfig and waiting for .post_bootstrap code like this is preferred:

class TorConfig(object):
    @classmethod
    @inlineCallbacks
    def from_connection(cls, reactor, connection):
        bootstrapped = Deferred()
        cfg = TorConfig(..., bootstrapped)
        yield bootstrapped
        returnValue(cfg)

...so now "user" code would do this:

def main(reactor):
    con = yield TorControlProtocol.from_endpoint(reactor, TCP4ClientEndpoint("localhost", 9051))
    cfg = yield TorConfig.from_connection(reactor, con)
task.react(main)
meejah commented 8 years ago

There are now TorState.from_protocol() and TorConfig.from_protocol(). Are there any others?

(The goal here is to elminate any need to mess with .post_bootstrap attributes ever, which is a bad pattern for initialization anyway).

meejah commented 7 years ago

We've also added connect and launch which are nice high-level APIs where you don't muck about with .post_bootstrap either.

meejah commented 7 years ago

The "real" way we should do this is to do away with ".bootstrap" altogether. The factory-functions should gather up any data required and pass it to the constructor for TorState etc. This would also make the tests way nicer as they don't need to make a fake protocol just to answer the "bootstrap" queries with the right fake data...

meejah commented 6 years ago

This is partially done (there are factory methods) but we should also: deprecate .bootstrap and do the other refactorings mentioned (i.e. instead of TorState implementing its own "bootstrap" methods, we should query that stuff in the factory-function and pass in the data).