TheRocinante
TheRocinante

Reputation: 4201

C++ Unable to call template class function after passing it to another function

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

Answers (1)

Guillaume Racicot
Guillaume Racicot

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

Related Questions