On Apr 2, 2013, at 3:52 AM, Hynek Schlawack <h...@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

_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Reply via email to