Listener class inheritor - const or non-const reference to class listened to?

Question

A very common pattern for a codebase is a Listener affair like this:

 class Frobulator
 {
 public:
      class Listener
      {
      private:
          // only frobulators can frob
          friend class Frobulator;
          virtual void onFrobbed() = 0;
      }

      void maybeFrob()
      {
          // assume we always frob, but maybe we only do it sometimes
          // and the caller won't know if a call will do it
          for (auto& l: listeners)
          {
               l->onFrobbed();
          }
      }

      void addListener(Listener* l)
      {
           listeners.push_back(l);
      }

 private:
     std::vector<Listener*> listeners;
 }

Then, a class inherits Listener and can register as a listener with the Frobulator. When the Frobulator frobs after a call by some caller (not necessarily the listener), the listeners will be told.

The real question I have then is, should the listener listen "internally", and therefore need a non-const reference to the Frobulator, but manage with a private Listener nature?

 class FrobReactor: private Frobulator::Listener
 {
 public:
      FrobReactor(Frobulator& frobulator_)
           frobulator(frobulator)
      {
           frobulator.addListener(this);
      }

 private:
      void onFrobbed() override
      {
           // react!
      }

      Frobulator& frobulator;
 }

 // and externally ...
 Frobulator theFrobber;
 FrobReactor reactor(theFrobber);
 theFrobber.maybeFrob();

Or should the listener take a const-reference (or even no reference if you don't need it), acknowledging that the FrobReactor doesn't modify the Frobulator, but advertising that it is a Frobulator::Listener and expecting the client code to hook it up:

class FrobReactor: public Frobulator::Listener
{
public:
    FrobReactor(const Frobulator& frobulator_):
        frobulator(frobulator_)
    {
    }

private:
    void onFrobbed() override
    {
        // react!
    }

    const Frobulator& frobulator;
}

 // and externally
 Frobulator theFrobber;
 FrobReactor reactor(theFrobber);
 theFrobber.addListener(&reactor);
 theFrobber.maybeFrob();

Alternatively the addListener method could be made const and the listener list mutable and then the first method also works with a non-const reference, but this feels like a hack.

Is there a "right" way to do this?

Answer 1

I would not store a reference to the observed (or listened-to) Frobulator in the FrobReactor. Rather, I would pass a const reference to the Frobulator instance to the onFrobbed method.

class Listener
{
private:
    // only frobulators can frob
    friend class Frobulator;
    virtual void onFrobbed(const Frobulator& frobulator) = 0;
}

and, adapting maybeFrob:

void maybeFrob()
{
    // assume we always frob, but maybe we only do it sometimes
    // and the caller won't know if a call will do it
    for (auto& l: listeners)
    {
        l->onFrobbed(*this);
    }
}

As for whether addListener should be const or not (and therefore, whether the vector of listeners should be mutable or not), it depends on what you are trying to achieve. I agree that it feels hacky, but on the other hand, if you want to ensure that clients of your API only deal with const Frobulators, then this is one way to do this. Another would be to have a method somewhere in your API that handles adding listeners to Frobulators like so:

void addListener(const Frobulator& frobulator, Frobulator::Listener& listener) {
    Frobulator& nonConst = // obtain non-const reference to this frobulator
    nonConst.addListener(listener);
}

It really depends on your design. Otherwise, if all you wish to do is protect listeners from modifying Frobulators, then passing a const reference to the 'onFrobbed method seems sufficient.

Finally, I would change addListener like so:

void addListener(Listener& listener)
{
    listeners.push_back(&listener);
}

It's a small change, but I prefer only to pass a pointer when I am transferring ownership, unless there are other reasons to pass a pointer. Of course you have to make sure that your listener doesn't get deleted (by falling out of scope) in either case.