0

I have two classes Observer and Subject and ObserverA, ObserverB which are extending Observer (for implementing Observer DP).

In Subject I have a vector of pointers to Observer class:

class Subject {
    std::vector<Observer*> obs;
    int state;
public:
    Subject(): state(0) {}

    void notifyAll() {
      for(std::vector<Observer*>::iterator it = obs.begin(); it != obs.end(); it++)
      {
         (*it)->update();
      }
    }
}

...
};

In Observer, I also have a Subject:

class Observer {
protected:
   Subject subj;   
public:
    virtual void update() {};
};

ObserverA, ObserverB alike:

class ObserverA: public Observer {
public:    
    ObserverA(Subject s) {
        subj = s;
        s.attach(this);
    }

    virtual void update() {
        std::cout << "Update A, state subj: " << subj.getState() << std::endl;    
    }
};

Before declaring class Subject, I added forward declaration for class Observer. However I am getting compiling errors:

Observer.h: In member function ‘void Subject::notifyAll()’:
Observer.h:27:17: error: invalid use of incomplete type ‘class Observer’
            (*it)->update();
                 ^
Observer.h:7:7: error: forward declaration of ‘class Observer’
 class Observer;
   ^

I have a vector of pointers to Observer, in Subject class, I forward declared Obsever class and Observer class knows about Subect, why do I still have the error?

georgiana_e
  • 1,809
  • 10
  • 35
  • 54
  • If you want to actually be able to *use* the functions of a class, you need the full definition of it. If you only have a forward declaration how would the compiler actually know what member functions it have? – Some programmer dude Feb 01 '17 at 15:16
  • IMO, it is different "a duplicated question" and two questions about the same issue. A programmer who know the solution will immediately know the answer is similar, but a new programmer will not, and probably that is why he is asking and not already reading the answer. – Adrian Maire Feb 01 '17 at 15:33

1 Answers1

3

Move the implementation of Subject::notifyAll into a .cpp file (in fact, do so for all member function implementations) - say the subject.cpp file. Then keep the forward declaration for Observer in the header file that contains the Subject class definition, and #include "observer.hpp" in the subject.cpp.

The forward declaration is sufficient in the header file since the class definition only ever refers to Observer*. On the other hand, the .cpp file needs to see the full Observer class definition, since it calls a member function on an Observer instance.

So :

// subject.hpp
#include <vector>

class Observer;

class Subject {
  std::vector<Observer*> obs;
  int state;
public:
  Subject(): state(0) {}

  void notifyAll();
};

and :

// subject.cpp
#include "subject.hpp"

#include "observer.hpp"

#include <vector>

void Subject::notifyAll() {
  for(std::vector<Observer*>::iterator it = obs.begin(); it != obs.end(); it++)
  {
    (*it)->update();
  }
}

Furthermore, your observer implementation can be improved quite a bit. Eg. :

  • don't keep a Subject in the Observer. You will likely have multiple subjects for the same observer, but more importantly : the observer doesn't need to keep track of the subjects.
  • don't link a Subject to an Observer in the Observer constructor. That would only allow you to have one subject per observer. Instead, attach an Observer to a Subject in the Subject constructor (or another member function).
  • don't pass by value when you don't want to make a copy. Instead, pass by reference (eg. const Subject&).
Sander De Dycker
  • 16,053
  • 1
  • 35
  • 40