How to handle callback registration

Question

I want to implement a callback handler. Methods should be registered as easy as the following...

std::multimap<Event::Type, std::function<void()>> actions;

void EventManager::registerAction(Event::Type event, std::function<void()> action) {
    actions.insert(std::make_pair(event, action));
}

...which indeed works as intended.

But the problem with that approach is, that it is impossible to deregister a callback...

void EventManager::deregisterAction(Event::Type event, std::function<void()> action) {
    for(auto i = actions.lower_bound(event); i != actions.upper_bound(event); ++i) {
        // if action == i->second
    }
}

...because it's impossible to compare bound functions.

Lazy deregistering also won't work because the function object can't be validated.

void EventManager::handle(Event::Type event) {
    for(auto i = actions.lower_bound(event); i != actions.upper_bound(event); ++i) {
        if(i->second) // returns true even if the object doesn't exist anymore
            i->second();
    }
}

So how should I approach an implementation like this, how can the issues I encountered be avoided?

Answer 1

One rather simple (yet not completely clean) way would be to just return a handle to the callback, which under the hood is just an iterator to the element in the map. The user is then responsible for storing this handle if he wants to deregister it some day.

class CallbackHandle
{
    friend class EventManager;

public:
    CallbackHandle() = default;
    CallbackHandle(const CallbackHandle&) = delete;
    CallbackHandle& operator=(const CallbackHandle&) = delete;

    bool isValid() const { return iter_; }
    Event::Type event() const { return iter_.value()->first; }
    void invoke() const { iter_.value()->second(); }

private:
    typedef std::multimap<Event::Type, std::function<void()>>::iterator Iterator;

    CallbackHandle(Iterator iter) : iter_(iter) {}

    std::optional<Iterator> iter_;
};

CallbackHandle EventManager::registerAction(Event::Type event, 
                                            std::function<void()> action)
{
    return CallbackHandle(actions.insert(std::make_pair(event, action)));
}

void EventManager::deregisterAction(CallbackHandle handle)
{
    actions.erase(handle.iter_);
}

Instead of C++14's std::optional one could as well just use a boost::optional or a mere std::unique_ptr with nullptr as invalid value.

Due to the move-only nature of the handle type and the fact that you have to explicitly move the handle into the deregistering function, you automatically get it invalidated when deregistering and can never have a handle refering to an already deleted callback (Except for the fact of a completely destroyed EventManager object, which would need to be solved by some more intertwining of the two types).

In fact it is similar to Werner's solution, but a bit simpler. It can be the base for providing additional stuff on top of it, like higher level automatic RAII-based deregisterers and stuff, while still having access to low-level manual deregisteration when needed/wanted.

Answer 2

But the problem with that approach is, that it is impossible to deregister a callback...

I've had the following problem in the past. I solved it by using a list to ensure iterators weren't invalidated, and I returned an Unmapper object that was associated with the iterator in the list and unmapped when it went out of scope. Unmap would remove the iterator from the list of functions:

The unmapper had the following flavour:

//Polymorphic destruction being the only purpose....
struct Resource{ virtual ~Resource(){} };

template <class SubjectT, class ListT>
class Unmapper : public Resource
{
  public:
    Unmapper( std::shared_ptr<SubjectT> state,
      typename ListT::iterator ref ) :
      subject_( state ),
      ref_( ref )
    {
    }
    ~Unmapper()
    {
      std::shared_ptr<SubjectT> subject = subject_.lock();
      if( subject )
      {
        subject->unmap( ref_ );
      }
    }

  private:
    std::weak_ptr<SubjectT> subject_;
    typename ListT::iterator ref_;
};

And...

    typedef std::function<void()> StateNotifier;

    class StateImpl :
      public std::enable_shared_from_this<StateImpl>
      //And some others...
    {
      typedef std::list<StateNotifier> MappedCallList;

      //....
      virtual std::unique_ptr<Resource> mapToCall( const StateNotifier& callback )
      {
        MappedCallList::iterator callRef =
          mappedCalls_.insert( mappedCalls_.end(), callback );
        return std::unique_ptr<Resource>( new Unmapper<
          StateImpl,MappedCallList>( shared_from_this(), callRef ) );
      }

      //No brainer...
      void unmap( MappedCallList::iterator i ){ mappedCalls_.erase( i ); }
      //...
    };

Now the user needs to keep hold of the return value of mapToCall until he doesn't need it anymore, then RAII unmaps automagically.

One could easily modify this to use a map. The Unmapper is hidden from the client via the Resource interface, as the client only needs to "unmap" when "mapped" goes out of scope.

I've omitted irrelevant code for brevity. It can also be noted that I had many calls mapped to states, and at a higher level states lived in a map. This is irrelevant, as as with lists, maps iterators aren't invalidated, and can therefore be used during de-registration.