Using weak_ptr to implement the Observer pattern

Question

What I have so far is:

Observer.h

class Observer
{
public:
    ~Observer();
    virtual void Notify() = 0;
protected:
    Observer();
};

class Observable
{
public:
    ~Observable();
    void Subscribe( std::shared_ptr<Observer> observer );
    void Unsubscribe( std::shared_ptr<Observer> observer );
    void Notify();
protected:
    Observable();
private:
    std::vector<std::weak_ptr<Observer>> observers;
};

Observer.cpp

void Observable::Subscribe( std::shared_ptr<Observer> observer )
{
    observers.push_back( observer );
}

void Observable::Unsubscribe( std::shared_ptr<Observer> observer )
{
    ???
}

void Observable::Notify()
{
    for ( auto wptr : observers )
    {
        if ( !wptr.expired() )
        {
            auto observer = wptr.lock();
            observer->Notify();
        }
    }
}

(de/constructors are implemented here but empty, so I've left them out)

What I'm stuck on is how to implement the Unsubscribe procedure. I came across the erase - remove - end idiom, but I understand that it will not work "out of the box" with how I have setup my Observable. How do I inspect the weak_ptr elements in the observers vector such that I can remove the desired Observer?

I'm also looking for some advice on what the parameter type should be for my Un/Subscribe procedures. Would it be better to use std::shared_ptr<Observer>& or const std::shared_ptr<Observer>&, since we will not be modifying it?

I really do not want to have Observables owning their Observers, as it seems to betray the intentions of the pattern, and is certainly not how I want to structure the rest of the project that will ultimately be making use of the pattern. That said, an added layer of security / automation that I am considering is to have Observers store a mirror vector of weak_ptr. An Observer on its way out could then unsubscribe from all Observables it had subscribed to, and an Observable on its way out could erase the back-reference to itself from each of the Observers observing it. Evidently the two classes would be friends in such a scenario.

Answer 1

What I'm stuck on is how to implement the Unsubscribe procedure.

I suggest to store observers in a std::list because it's iterators are not invalidated on container modification. Then in subscribe in observer you store iterator to it and in unsubscribe you use the iterator to remove the element. But of course you can use std::vector and std::remove_if as suggested in another answer.

Now about all that *_ptr stuff. In C++ RAII is your friend so use it. Get rid of public unsubscribe method. Instead observer must unsubscribe itself in it's destructor. This simplifies things very much: no more locking of weak pointers: if observer has been deleted, then it is not on the list. Just don't forget to protect observer list with a mutex if you have multithreaded application. If you use this design, then Observable would need only plain pointers to Observers and there will be no requirements how Observers must be stored.

class Observer {
public:
    void subscribe(std::function<void()> unsubscribe) {
        unsubscribe_ = std::move(unsubscribe);
    }

    virtual ~Observer() {
        unsubscribe_();
    }
private:
    std::function<void()> unsubscribe_;
};

class Observable {
public:
    void subscribe(Observer* observer) {
        std::lock_guard<std::mutex> lock(observablesMutex_);
        auto itr = observers_.insert(observers_.end(), observer);
        observer->subscribe([this, itr]{
            std::lock_guard<std::mutex> lock(observablesMutex_);
            observers_.erase(itr);
        });
    }

private:
    std::list<Observer*> observers_;
    std::mutex observablesMutex_;
};

Note: for this code Observers must always be destroyed before the Observable.

---

Update: if you get more used to C++ lambdas you may find that having std::function as observer is more convenient in many cases than having a special class hierarchy. In this case you API can be like this:

class Handle {
public:
    explicit Handle(std::function<void()> onDestroy)
        : onDestroy_(std::move(onDestroy)) {}

    Handle(const Handle&) = delete;

    Handle(Handle&&) = default;

    virtual ~Handle() {
        onDestroy_();
    }
private:
    std::function<void()> onDestroy_;
};

class Observable {
public:
    Handle subscribe(std::function<void()> observer) {
        std::lock_guard<std::mutex> lock(observablesMutex_);
        auto itr = observers_.insert(observers_.end(), observer);
        return {[this, itr]{
            std::lock_guard<std::mutex> lock(observablesMutex_);
            observers_.erase(itr);
        }};
    }

private:
    std::list<std::function<void()>> observers_;
    std::mutex observablesMutex_;
};

Answer 2

You can use std::remove_if with std::erase like this:

void Observable::Unsubscribe( std::shared_ptr<Observer> observer )
{
    std::erase(
        std::remove_if(
            this->observers.begin(),
            this->observers.end(),
            [&](const std::weak_ptr<Observer>& wptr)
            {
                return wptr.expired() || wptr.lock() == observer;
            }
        ),
        this->observers.end()
    );
}

You should indeed pass observer as const std::shared_ptr<Observer>&.