[Twisted-Python] Advice sought on application evolution
Phil Mayers
p.mayers at imperial.ac.uk
Fri Mar 28 05:22:21 MDT 2008
glyph at divmod.com wrote:
> On 11:06 am, p.mayers at imperial.ac.uk wrote:
>>>
>>> It's great as a high-level prototyping tool and an orchestration
>>> language for distributed conversations with large numbers of steps
>>> and coarse- grained error handling. It's bad as a way of
>>> representing state machines or continuous processes.
>>
>> Why?
>
> Here's something which we probably should have done with a state
> machine, but instead did with deferredGenerator (the predecessor to
> inlineCallbacks).
>
> http://divmod.org/trac/browser/trunk/Quotient/xquotient/grabber.py#L438
Ok.
I would have written it more like this, dump the error handling and let
it propagate upwards to someone who is interested, and obviously clean
it up with inlineCallbacks:
@inlineCallbacks
def grabber(self):
yield self.login(self._username, self._password)
uids = []
yield self.listUID(uids.append)
for idx in range(0, len(uids), batchsize):
uidset = uids[idx:idx+batchsize]
L = self.shouldRetrieve(uidset).sorted()
...etc. and I guess what I'm getting at is why is the above worse than:
def grab(self):
return self.login.addCallbacks(self.login_done, self.login_err)
def login_done(self, loginResult):
uids = []
d = self.listUID(uids.append)
d.addCallback(self.uids_done, uids)
d.addErrback(self.uids_err)
def uids_done(self, unk, uids):
dlist = []
for idx in range(0, len(uids), batchsize):
uid_slice = uids[idx:idx+batchsize]
d = self.sometinymethod(uid_slice)
d.addErrback(self.tinymethod_err)
dlist.append(d)
dlist = defer.DeferredList(dlist)
dlist.addCallback(self.next_method)
...etc.
I'm not disputing the point - I'm trying to figure out why
inlineCallbacks might be inappropriate because I'm using it for just
such an application and I'm finding it greatly superior to the previous
multi-callback chain solution, which frankly was incomprehensible and
error-prone (oops I forgot to return the deferred. again).
On the other hand, I *removed* inlineCallbacks from some lower-level
calls in my SNMP libs as it was clearly cleaner, so another way of
asking the question is "how do I detect that inlineCallbacks is bad code
smell?"
>
> The function's really long, and there's a ton of duplication between all
> of the "except" clauses, and there needs to be a try/except around every
> single deferred call. This probably would have been better implemented
> as a regular state-machine, because that would have allowed decomposing
> the problem further.
>
> Granted, you could blame the factoring here on any number of things. It
> could have been broken into different steps even with deferredGenerator,
> it could have been documented better, it could have stored more state on
> the instance so that you could inspect its state rather than hiding it
> as local variables... but given that this is general stylistic advice I
> think that the point holds.
>
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
More information about the Twisted-Python
mailing list