Reputation: 4201
I have an Observable template.
template<typename T>
class Observable {
public:
~Observable() {
observers.clear();
}
void registerObserver(const std::shared_ptr<Observer<T>> & observer) {
observers.push_back(observer);
}
void update(const T & record) {
for (auto &observer: observers) {
observer->update(record);
}
}
private:
std::list<std::shared_ptr<Observer<T>>> observers;
};
My observers are also created from a template:
class Observer {};
template<class T>
class Observer : public Observer {
public:
virtual void update(const T record) = 0;
};
I run this like this:
auto cat = Cat()
auto catObservable = Observable<Cat>();
auto catObserver = std::shared_ptr<CatObserver>(); // CatObserver is a derived class from Observer<Cat>
observable.registerObserver(catObserver);
observable.update(cat);
This code has a seg fault at runtime in the Observable
template where it calls observer->update(record)
. The only way to fix it is to do a dynamic cast like this:
void update(const T & record) {
for (auto &observer: observers) {
std::shared_ptr<CatObserver> catObserver = std::dynamic_pointer_cast<CatObserver>(observer);
catObserver->update(record);
}
}
I want to have different types of observers, and the only way I can think to make this work is have a large if statement or switch statement that tries to perform dynamic_pointer_cast to all the observers. This feels like a code smell.
I'm new to C++ and not even sure if this type of a problem is well known and what I"m trying to do here is just an anti-pattern.
Any thoughts?
Upvotes: 0
Views: 44
Reputation: 41770
You are inserting null pointers and you push those in the list:
// catObserver is nullptr
auto catObserver = std::shared_ptr<CatObserver>();
Indeed, just like creating a raw pointer with no parameter, it will be null:
CatObserver* rawCatObserver{}; // null
To actually create a CatObserver
, you have to call std::make_shared
, which will do the dynamic allocation:
// returns an allocated shared pointer
auto catObserver = std::make_shared<CatObserver>();
I'm new to C++ and not even sure if this type of a problem is well known and what I"m trying to do here is just an anti-pattern.
This code is okay, but performance may suffer when using std::list
to iterate on those. For your use case, std::vector
may be better. On a more personal note, I try to get away with std::unique_ptr
as the ownership is more clear. And if I can get away with replacing all those classes with a std::function
and a couple lambdas, I will usually leans towards that.
Upvotes: 1