[Twisted-Python] Current callback best practices

[Glyph]

On Apr 2, 2013, at 3:52 AM, Hynek Schlawack <hs at ox.cx> wrote:

> My question can be simplified to: Closures yes or no?

"As appropriate".

> They seem *really* handy since they allow to have some data present without handing it through all the time. Eg in my cred checker, I can refer to the user name from the closure instead of passing it around all the time – making the code much cleaner. Also most current examples and callbacks-are-hard-FUD-rebuttals seem to use them.
> 
> OTOH, private methods like `_cbPrintResult` are nicer to test individually.

Ideally, any private method exists in service of some goal of the public interface.  It's best to test them by exercising the case in the public API which they support, rather than calling them directly.  Before you write any such method, you ought to be able to write a failing test for the public interface which doesn't pass because you haven't written the private method yet :).

If you're writing private methods which need their own extensive internal test coverage, it might be good to consider whether what you're writing really needs to be public functionality.

However, even if you're going to have a private implementation detail complex enough to warrant its own test suite independent of the public feature it's supporting – and it is better to have such a suite than to have an explosion of irrelevant complexity in the tests for the public API - it's best to go with a private class or module than private methods on a public class.  Callback methods like _cbSomethingSomethingResult generally indicate an ad-hoc state machine as part of your class.  Chances are, if you're going down this road, you're missing quite a few states, and the ad-hoc state is smeared out across many poorly-encapsulated attributes.  After all, since it's not a closure, your only place to store any resulting state is 'self'.

The best solution at this point is not to try to test _cbWhatever methods directly, but rather, to spin out all the _cbSetDisconnected and _ebFixUpStuff methods into a dedicated class which has its own "public" interface that can be tested, but which is still private for Twisted's compatibility purposes.  That dedicated class can then have an explicit state machine of its own and will have attributes dedicated to just encapsulating the state that it's tasked with maintaining, instead of a random potpourri of different states from different responsibilities.

So I guess my recommendation would be "closures", because any time using private methods as callbacks would be better, it seems like something else has gone wrong :).

> If “yes, closures”: Still using cb/eb prefixes? I don’t see them very often in recent examples. What about addBoth handlers?

If you're using a closure, use a descriptive name.  The 'cb/eb' prefix notation is not particularly illustrative; I often use an english word like 'when'.  whenDisconnected, whenResponseReceived, etc.

The only hard and fast rule is, if you have a closure, don't start its name with an underscore, there's no point to that :).

-g

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://twistedmatrix.com/pipermail/twisted-python/attachments/20130402/d1717425/attachment.htm