Observer pattern written in C++

Question

I just carefully studied the observer pattern. And I wrote a demo snippet to better undertand it. I am not so sure that if it's the right way to define IObservable& m_observable; in the pure abstract class. Here is the demo code written in C++.

#include<list>
#include<algorithm>
#include<iostream>
#include<string>
class IObserver;
class IObservable
{
public:
 virtual void add(IObserver*) = 0;
 virtual void remove(IObserver*) = 0;
 virtual void trigger() = 0;
 virtual double get_temperature() = 0;
 virtual ~IObservable() = default;
};
class IObserver
{
public:
 IObserver(IObservable& observable):m_observable(observable){}
 virtual void update() = 0;
 virtual ~IObserver() = default;
protected:
 IObservable& m_observable;
};
class IDisplay
{
public:
 virtual void display() = 0;
 virtual ~IDisplay() = default;
};
class WeatherStation : public IObservable
{
public:
 void add(IObserver* observer_ptr) override
 {
 if(std::find(m_observer_list.begin(), m_observer_list.end(), observer_ptr) == m_observer_list.end())
 {
 m_observer_list.push_back(observer_ptr);
 }
 }
 void remove(IObserver* observer_ptr) override
 {
 m_observer_list.erase(std::remove(m_observer_list.begin(), m_observer_list.end(), observer_ptr), m_observer_list.end());
 }
 
 void trigger()
 {
 for(const auto& observer:m_observer_list)
 {
 observer->update();
 }
 }
 double get_temperature()
 {
 return m_temperature;
 }
 void set_temperature(double temperature)
 {
 m_temperature = temperature;
 trigger();
 }
 virtual ~WeatherStation() = default;
private:
 std::list<IObserver*> m_observer_list;
 double m_temperature;
};
class CommonDisplay:public IObserver, public IDisplay
{
public:
 CommonDisplay(IObservable& observable):IObserver(observable){}
 virtual ~CommonDisplay() = default;
 void update() override
 {
 display();
 }
};
class PhoneDisplay:public CommonDisplay
{
public:
 PhoneDisplay(IObservable& observable, const std::string& name): m_name(name),CommonDisplay(observable){}
 void display() override
 {
 std::cout << m_name << "'s phone displayed, temperature: " << m_observable.get_temperature() << std::endl;
 }
private:
 std::string m_name = "unknown";
};
class WindowDisplay:public CommonDisplay
{
public:
 WindowDisplay(IObservable& observable):CommonDisplay(observable){}
 void display() override
 {
 std::cout << "WindowDisplay displayed, temperature: " << m_observable.get_temperature() << std::endl;
 }
};
int main()
{
 WeatherStation station_of_NewYork;
 WeatherStation station_of_Seattle;
 PhoneDisplay jone_phone(station_of_NewYork, "Jhone");
 PhoneDisplay lucy_phone(station_of_NewYork, "Lucy");
 station_of_NewYork.add(&jone_phone);
 station_of_NewYork.add(&lucy_phone);
 WindowDisplay bedroom_display(station_of_NewYork);
 station_of_NewYork.add(&bedroom_display);
 station_of_NewYork.set_temperature(15.6);
}

• Toby Speight: Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. It's best to describe what value this code provides to its user. Something to do with weather stations, yes?

Answer 1

Overuse of abstract base classes

In your code, you have way too many abstract base classes. Most of them are not necessary. In fact, you only need a base class for the observer. Its update() should have a parameter to pass the temperature to the observers. So:

class IObserver {
public:
 virtual ~IObserver() = default;
 virtual void update(double temperature) = 0;
};
class WeatherStation {
...
 void trigger() {
 for (auto& observer: m_observer_list) {
 observer->update(m_temperature);
 }
 }
...
};
class WindowDisplay: public IObserver {
public:
 void update(double temperature) override {
 std::cout << "Temperature: " << temperature << "\n";
 }
};

The observer base class is necessary because m_observer_list needs to store pointers to different types of concrete objects, and IObserver provides a form of type erasure.

Naming

Make sure you give classes meaningful names. What is an IObserver? What is it observing? From the name I can't tell whether it observes temperatures or something else. What if you add more observable types to your program? Name it ITemperatureObserver.

Simplify removing an observer

You can simplify removing an observer by using remove():

void remove(IObserver* observer_ptr) override
{
 m_observer_list.remove(observer_ptr);
}

Prefer std::vector over std::list

Use a std::vector to store the observer pointers. It's stored much more compactly in memory, which also improves its performance.

• John: Thank you for your detailed explanation. How to better understand "you have way too many abstract base classes. Most of them are not necessary."? Could you please explain that for me?
• indi: list::remove() has been around since pre-history; you probably meant the more generic std::erase(m_observer_list, observer_ptr), which is what everyone should be using these days (because it will work for lists and vectors, and most everything else).

Answer 2

Just a minor thing relate to the methods you describe as get_temperature and set_temperature.

double get_temperature()
{
 return m_temperature;
}
void set_temperature(double temperature)
{
 m_temperature = temperature;
 trigger();
}

I think is more compacted, in the way the boost library does, so your code will be:

double temperature() const
{
 return m_temperature;
}
void temperature(double value) 
{
 m_temperature = value;
 trigger();
}