Common pattern with Listeners


#1

It’s a common pattern that one has a class subclassing X::Listener, calls x.addListener (this) in its own constructor and x.removeListener (this) in its destructor.

It would be less error-prone if the destructor of X::Listener would remove the listener on its own. This would require the listener to keep a list of what objects it listens to.

An alternative would be to maintain this list only in Debug builds, with an assertion in the destructor that the required removeListener calls have been made. This would enabled catching a class of possible bugs earlier.


#2

I see a good opportunity to embed the desired behavior in a class template called something like ScopedListening.

It could take the broadcaster and the listener as parameters of its constructor, retain them as member pointers or references, and perform the addListener() and removeListener() between them on construction and destruction respectively.


#3

#4

But regardless of any scoped-listeners class, the normal listener class should at least provide the assertion that the required removeListener calls have been made. Btw a simple counter would suffice for it too.

So if for example a programmer adds a listener to a parameter in the gui component, where currently things would crash some mysterious time when the parameter is changed after the gui was closed, the new assertion would catch the bug at the moment the gui is closed. It’s much nicer to get an assertion (“missing removeListener call”)…

It would be nice if easier-to-use classes exist too but all existing classes should be helpful with assertions and thus less error-prone…


#5

Just noticed a case where the user would had been happier if what I suggested was implemented:

The assertion would give him the answer he would had needed instead of needing to ask the forum…


#6

I think the problem with the auto remove in the base class is that it is actually a bit late to remove the listener at that point. If a callback is called after your destructor is finished but before the base class destructor has finished, the behaviour would potentially be a very horrible and hard to track bug. A jassert would be nice though.

I was wondering how it would work but I guess something like this…

class ClassX
{
public:
    class Listener
    {
    public:
        friend class ClassX;
        virtual ~Listener()
        {
            jassert (listenerInstances.isEmpty());
            listenerInstances.call ([this] (ClassX& l) { l.removeListener (this); });
        }
        
        virtual void someCallback() = 0;
        
    private:
        ListenerList<ClassX> listenerInstances;
    };
    
    void addListener (Listener* listenerToAdd)
    {
        if (listenerToAdd)
        {
            listeners.add (listenerToAdd);
            listenerToAdd->listenerInstances.add (this);
        }
    }
    
    void removeListener (Listener* listenerToRemove)
    {
        if (listenerToRemove)
        {
            listeners.remove (listenerToRemove);
            listenerToRemove->listenerInstances.remove (this);
        }
    }
    
    ListenerList<Listener> listeners;
};

#7

That’s a good point - but still the assertion is the key feature to have even if it doesn’t automatically remove the listener.