C++ : Problem faced while implementing observer design pattern with smart pointers

Question

I am trying to implement observer design pattern in C++. I want it to be in such a way that the observer need not explicitly remove itself when being destroyed. So, I am trying to use a vector<weak_ptr<Observer>> to maintain the observer list in the Subject class.

I also want the observers to add themselves to the subjects in the respective constructors. So, I am trying an implementation as follows:

The Subject class :
    void addObserver(std::shared_ptr<observerInterface> & observer) {
        m_vObservers.emplace_back(observer);
    }

    std::vector<std::weak_ptr<observerInterface>> m_vObservers;

    ...

The object class:
    observer1::observer1() : observerInterface() {
        ...
        sub1->addObserver(shared_from_this())
        ...
    }

But this code gives a bad_weak_ptr exception since shared_from_this() for a class can only be called once the constructor completes execution.

I want to find out if there is a solution to this problem. I could always add a separate method in the observer class that would execute the addObserver method for all valid subjects. But I want to find out if there is another elegant solution!

Answer 1

I've updvoted static constructors, however I'd like to point out that using an observer factory is also an option:

#include <iostream>
#include <memory>
#include <vector>
#include <algorithm>

struct Observer {
    virtual void notify() {
        std::cout << "Notifying" << std::endl;
    }
    virtual ~Observer(){}
};

struct Subject {
    std::vector<std::weak_ptr<Observer>> observers_;
    
    void subscribe(std::shared_ptr<Observer>&& sp) {
        observers_.emplace_back(sp);
    }
    
    void update() {
        for (auto item : observers_) {
            if (auto observer = item.lock()) {
                observer->notify();
            }
        }
        //Remove stale pointers...
        observers_.erase(
            observers_.begin(),
            std::remove_if(
                observers_.begin(), 
                observers_.end(), [](const std::weak_ptr<Observer>& obs) {
                return obs.lock() == nullptr;   
            })
        );
    }
};

struct ObserverFactory {
    std::shared_ptr<Observer> create(Subject& subject) {
        auto result = std::make_shared<Observer>();
        subject.subscribe(move(result));
        return result;
    }   
};


int main() {
    ObserverFactory observerFactory;
    Subject s;
    
    {
        auto observer = observerFactory.create(s);
        s.update();
    }
    
    s.update(); //Nothing updated
    return 0;
}

Note that the factory can also be abstract, providing various observer types, and the each factory can hide the implementation of the actual observer. This probably also provides slightly better locality because of the use of std::make_shared, although I like and have often used the static constructor solution too.

Answer 2

Use "static constructors":

class observer1 : public observerInterface
{
public:
    explicit observer1(const OtherParam&);

    static std::shared_ptr<observer1> create(
        Subject& subj, const OtherParam& p)
    {
        auto ptr = std::make_shared<observer1>(p);
        subj.addObserver(ptr);
        return ptr;
    }

    // ...
};

If there are several constructors for an observer, it might be easier to make create a variadic template.

If observers should be created only in this way, and you want to prevent a mistake of user code creating a non-shared observer or directly creating a shared_ptr without create(), you can set up a "constructor access key". This lets a class give make_shared permission to create an object by passing the key, but other code can't get any key and can't call any constructors.

class observerInterface
{
    // ...
protected:
    template <class Derived>
    class CtorKey {
    private:
        CtorKey() = default;
        ~CtorKey() = default;
        friend Derived;
    };
    // ...
};

class observer1 : public observerInterface
{
private:
    using MyCtorKey = CtorKey<observer1>;

    // Only if copying the observer makes sense. Otherwise, make the
    // copy functions deleted.
    observer1(const observer1&) = default;
    observer1& operator=(const observer1&) = default;
    observer1(observer1&&) noexcept = delete;
    observer1& operator=(observer1&&) noexcept = delete;

public:
    virtual ~observer1() override = default;

    // Effectively private, since you can't get a MyCtorKey.
    // Constructor definition will just ignore the first parameter.
    observer1(MyCtorKey, const OtherParam&);

    static std::shared_ptr<observer1> create(
        Subject& subj, const OtherParam& p)
    {
        auto ptr = std::make_shared<observer1>(MyCtorKey{}, p);
        subj.addObserver(ptr);
        return ptr;
    }

    // Only if copying the observer makes sense:
    observer1(MyCtorKey, const observer1& src) : observer1(src) {}
    std::shared_ptr<observer1> clone(bool observe_same) const {
        auto ptr = std::make_shared<observer1>(MyCtorKey{}, *this);
        if (observe_same) {
            std::erase(std::remove_if(m_subjects.begin(), m_subjects.end(),
                [&ptr](const std::weak_ptr<Subject> &subj) {
                    if (!subj) return true;
                    subj.addObserver(ptr);
                    return false;
                }),
                m_subjects.end());
        }
    }

    // ...
};