4
\$\begingroup\$

I have written my own basic implementation of the Observer pattern. Please code review it as you feel. This is a one file implementation, and any feedback no matter how small is appreciated.

One sticky point is I was unsure on the implementation of RemoveObserver, so this may not be optimal. Could I have used Ranges here to modernise?

#include <iostream>
#include <chrono>
#include <algorithm>
#include <thread>

using std::chrono::high_resolution_clock;
using std::chrono::duration_cast;
using std::chrono::duration;
using std::chrono::milliseconds;

class TimeObserver
{
public:
    virtual void Update(std::chrono::steady_clock::time_point time, std::chrono::steady_clock::time_point startTime) = 0;
};

class Clock : public TimeObserver
{
    virtual void Update(std::chrono::steady_clock::time_point time, std::chrono::steady_clock::time_point startTime)
    {
        duration<double, std::milli> ms_double = time - startTime;
        std::cout << ms_double.count() << "ms\n";
    }
};

class TimeManager
{
public:
    void RegisterObserver(TimeObserver* Observer)
    {
        observerList.push_back(Observer);
    };

    void RemoveObserver(TimeObserver* Observer)
    {
        std::vector<TimeObserver*>::iterator position = std::find(observerList.begin(), observerList.end(), Observer);

        if (position != observerList.end())
            observerList.erase(position);
    };

    void NotifyObservers()
    {
        for (auto& observer : observerList)
        {
            observer->Update(time, startTime);
        }
    }

    void UpdateTime()
    {
        time = high_resolution_clock::now();
        NotifyObservers();
    }

private:
    std::vector<TimeObserver*> observerList;
    std::chrono::steady_clock::time_point time = high_resolution_clock::now();
    std::chrono::steady_clock::time_point startTime = high_resolution_clock::now();
};

int main()
{
    TimeManager timeManager{};
    TimeObserver* clockObserver = new Clock();
    TimeObserver* clockObserver2 = new Clock();

    timeManager.RegisterObserver(clockObserver);

    // Print initial time reading
    timeManager.UpdateTime();

    // sleep for 1s
    std::this_thread::sleep_for(std::chrono::milliseconds(1000));

    // register a second observer
    timeManager.RegisterObserver(clockObserver2);

    // This should print twice as we have 2 observers, should be ~1000ms
    timeManager.UpdateTime();

    // Remove both observers
    timeManager.RemoveObserver(clockObserver);
    timeManager.RemoveObserver(clockObserver2);

    // nothing should print as we have no observers
    timeManager.UpdateTime();
    
    delete clockObserver;
    delete clockObserver2;
}
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

About the observer design pattern

The design patterns from the "Gang of Four" focus heavily on solving problems using classes. However, in modern C++, there are other ways to solve the problem that might have some benefits.

Consider that you had to create two classes for the observer pattern here: an abstract base class TimeObserver, and a concrete class Clock. If you need to observe something else, or want to have an time observer that does something different than printing the time, you have to add more classes. But you are not interested in the classes, you only are interested in the function Update(). So instead you could use std::function to store observer function:

class TimeManager
{
    using Clock = std::chrono::steady_clock;
    using TimePoint = Clock::time_point;
    using Observer = std::function<void(TimePoint, TimePoint)>;

public:
    void RegisterObserver(Observer observer)
    {
        observers.push_back(observer);
    }
    …
    void NotifyObservers()
    {
        for (auto& observer: observers)
        {
             observer(time, startTime);
        }
    }
    …
private:
    std::vector<Observer> observers;
    …
};

And then you could use it like so:

int main()
{
    TimeManager timeManager;

    timeManager.RegisterObserver([](auto time, auto startTime) {
        std::cout << duration<double, std::milli>{time - startTime}.count()
                  << "ms\n";
    });
    …
}

This is slightly more efficient as well, since with virtual functions, two pointer indirections are necessary to call the actual function, whereas with std::function<> there is only a single indirection.

One drawback though is that you no longer have a pointer to an object that you can use as a handle to deregister the observer. However, that brings me to:

Deregistering issues

Your example code is correct. However, what if you had forgotten to delete the observers? Or worse, what if you deleted the observers after registering them but before calling UpdateTime()? What if you register the same observer object twice?

There are several ways in which you can avoid these issues:

All of these types will help manage the lifetime of observer objects in a safe way. They each have their pros and cons.

Use std::erase() to remove items

Instead of calling std::find() and then observerList.erase(…), you can use std::erase() since C++20. This simplifies the code and also handles erasing duplicates:

void RemoveObserver(TimeObserver* observer)
{
    std::erase(observerList, observer);
}

Avoid std::chrono::high_resolution_clock

Unfortunately, it's not well-defined what std::chrono::high_resolution_clock is. You are almost always better of using another clock. For example, std::chrono::system_clock if you care about wall clock time, something which you can compare with system clocks on other machines. Or if you want a clock that is not affected by NTP updates, leap seconds and so on, use std::chrono::steady_clock. Note that the latter is almost always a high resolution clock.

Also, avoid storing the result of std::chrono::high_resolution_clock::now() into a std::chrono::steady_clock::time_point. What would it even mean?

\$\endgroup\$
2
  • \$\begingroup\$ Amazing, thank you so much for the response. I was not aware of a lot of these things. for Avoid std::chrono::high_resolution_clock - I hadn't come across this before, although I haven't used clocks before in C++. Are there any good resources that cover these topics? \$\endgroup\$ Mar 31 at 17:25
  • 1
    \$\begingroup\$ If I had to choose one resource, I would recommend cppreference.com. If you look up std::chrono::high_resolution_clock there, it does mention the issues it has. It also has a section with links to other useful resources. If you have some spare time and instead of reading you just want to watch something, I recommend CppCon talks, or talks of any other C++ conferences. It's a great way to keep up to date and learn more. \$\endgroup\$
    – G. Sliepen
    Mar 31 at 20:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.