Split functionality of ObservedCallable

[coldfix]

The class currently really handles (at least) two different things:

1. static callables
2. member events
3. (..still trying to figure out, if there is more)

This is the reason for lots of if/else constructs in the implementation. Whenever you see something like that, you should think about splitting the functionality into separate classes.
Personally, I'm an adherent of the philosophy that almost every if/else should be replaced by polymorphism (subclassing/duck typing), as here demonstrated by my favorite speaker very nicely.

EDIT:
Ok, I think I got now, that most of the if/else are from the distinction of different kinds of observers (methods vs. objects) rather than observables (static event vs. instance event). I really think that this should be implemented as polymorphism though, it will make the code much easier to read.

Furthermore, the identifyObserved switch could be implemented using polymorphism.

One more advantage of polymorphism vs if/else constructs is, that your code size grows linearly with the number of features.

[DanielSank]

Ok, I think I got now, that most of the if/else are from the distinction of different kinds of observers (methods vs. objects) rather than observables (static event vs. instance event).

That is correct.

I really think that this should be implemented as polymorphism though, it will make the code much easier to read.

I like what you're saying because I like code to be easy to read... but I don't know what you mean by "polymorphism". The problem is that bound methods and normal functions are different, and I haven't found a way to sort between them without using if/else constructs. Can you give a simple example of what you mean?

To be clear, I'm familiar with the term "polymorphism" but I don't see how to use that idea in the present context, because of the differences in behavior between bound methods and static functions.

[coldfix]

I have an example implementation using polymorphism in the weak-observer branch. You may want to have a look at the CallableProxy and MethodProxy classes. I admit, that all of this took more code and is not as clean as I expected.

[coldfix]

A very simple example is as follows:

class X:
    def __init__(self, switch):
        self._switch = switch

    def foo(self):
        if self._switch:
            codeA()
        else:
            codeB()

x = X(True)
y = X(False)

can often be replaced by:

class A:
    def foo(self):
        codeA()

class B:
    def foo(self):
        codeB()

x = A()
y = B()

[DanielSank]

I looked at your weak-observer code. Do I understand correctly that the user must use the CallableProxy and MethodProxy classes as wrappers when they add observers?

[coldfix]

No. The user must chose to either use connect() or connect_weak().
But I am still working to improve the interface.

[coldfix]

FYI: after adding more and more code to my branch in order to improve the API, I am now thinking about dropping the whole finalization handler (for disconnecting when the object goes out of scope). You would just pass in a weakref.proxy() (or some wrapper object of your choice). If it raises a ReferenceError the subscription can be canceled from within the emit() function (this takes like only 3 extra lines, an additional 10-20 if adding a MethodProxy to the module). The only "memory-leak" here would be the proxy object inside the handler array and only as long as the event is not called.

[DanielSank]

That was my original attempt, but the problem with that is that you can't make a weak ref to a bound method. I want to be able to register both functions and bound methods as callbacks. If you know a way to handle that please let me know as it would significantly simplify my code :)

[coldfix]

I'd just use a MethodProxy class, that does exactly that. Maybe, with a more convenient name.

By the way, I'm just saying how I would do it. If you like to have it different, I think the current way to do it is fine as well. That is if you fix the issues i posted later on, these are real problems.

[DanielSank]

I figured out how to deal with the problem I was having and I've now pushed a major re-organization of the code to the dedicatedClasses branch. I believe this new code at least partially addresses the problems described in this issue.

The video you posted helped me handle a major aspect of the re-organization: handling different observer types with different classes (ie. polymorphism), rather than if/else statements.

The other insight was that it does not make sense to persist an object which is a proxy for a bound method. Let me explain because I think this is interesting. We do not wish to have the descriptor handling observable bound methods keep strong references to the instances which it manages. If it did keep strong references, the instances could never be garbage collected until the class itself is garbage collected. So, we must keep weak references to the instances in the descriptor. Now, the bound methods are supposed to have strong references to the objects to which they are bound, and this creates a problem because the descriptor must keep references to the bound method proxies, and the proxies keep strong references to their instances, and now we have a situation in which the instances can't be garbage collected.

The solution is to simply not persist bound method proxies. The relevant state for each proxy is persisted in the descriptor, but not the proxy itself. The proxies are generated on-demand. You probably already knew all of this, but for me it was an "aha!" moment.

EDIT: Of course, this entire issue would be circumvented if we just store data on the instances themselves. I'm considering doing that because it would also allow the instances to be pickled, whereas in my current strategy this is not so simple.

[DanielSank]

I think this is all now properly handled in the dedicated-classes branch. I'll note here in closing the issue that some of the complexity involved with weak referencing the observers would disappear if we stored the observer information as attributes on the observed objects. I may implement this in the future, but for now the code works and isn't too complex.