Inheritable Method Callbacks #16
Conversation
In python we can assume everything's successful until there's an exception. There's no reason all the functions have to explicilty return that they had SUCCESS. If there's a None returned it means there's no exception and we can tell C the operation was successful.
We now pass the delayed_usecs to the callback as an argument.
This class has a few methods which can now be overriden without requiring needing to call any set_*_callback functions. An example usage of it was implemented which is similar to chatty_client.
|
Thanks a lot for this PR! This is a really nice idea, I didn't like the return values very much myself. But if we do this, we should go all the way and completely drop the return values from the callbacks. There is only one case where I'm unsure, namely the process callback. If a user wants to stop the callback without there being an exception already, she will have to raise an exception herself. A few exception classes that come to mind: If you somewhat agree, it would be nice if you could make a separate PR for this, then we can discuss the details there. I like this idea! This one I don't like. I don't agree that this is Pythonic, on the contrary, I think it's much too Java-esque! Forcing those poor callback functions onto a class for no good reason doesn't seem like a good idea to me. I'm not saying that I want to forbid users to inherit from
That's not true (the first part is, but the second part isn't). While it's true that the only place where it actually matters performance-wise is the process callback, I still think that it's stylistically ugly to call all callback setters in the beginning (even if none is needed) and then call all those empty callbacks (even if none is needed).
That's a fair point, but I guess most users will only need a handful of callbacks [citation needed ...]. If we can find an example application where deriving from Just as a little side note: I don't like the names "
I don't like this option. I don't think that's an improvement at all, but of course that's highly subjective. Is there any role model for your suggestion? I guess we won't find anything like this in the standard library because the BDFL hates callbacks in general ... |
|
What do you think about #24 as alternative to your first commit here? What about my other comments? |
|
@amstan Do you want to follow up on this? If not, I'll close this at some point. |
2 participants
This is an example what i meant by my comment at #4 (comment).
The only function which could cause a performance penalty is process, and that one needs to be overridden anyway. While I could implement something that detects that nothing inherited most callbacks so don't set_*_callback on them, all the non-process callbacks happen very rarely so I don't think it's worth it.
Spamming set_*_callback in my init is not my idea of readable when i have other stuff to worry about there too.
For the final merge I'm proposing everything under PythonicClient to the Client proper. It even has a way to disable the extra functionality by passing method_callbacks=False. I could work on the docstrings too a little so all set_*callback methods refer to the on* methods for the callback signatures (and move the signatures there while I'm at it).