The copy constructor and assignment operators of the Patterns::Observer class are currently the default ones provided by the compiler. However, this can lead to incomplete state in the copied object and it therefore inadequate for Observer functionality.
An implementation of such methods is proposed which fixes the problem and leads to increased security against Observer/Observable lifetime issues.
class Observer {
public:
virtual ~Observer() {}
virtual void update() = 0;
};
class Observable {
public:
virtual ~Observable() {}
// Observer management
virtual void registerObserver(Observer*);
virtual void unregisterObserver(Observer*);
void registerObservers(std::set<Observer*>&);
virtual void unregisterObserver(Observer*);
void unregisterObservers(std::set<Observer*>&);
virtual void unregisterAll();
std::set<Observer*> observers() const;
// Observer notification
virtual void notifyObservers();
private:
std::set<Observer*> theObservers;
};
It can be seen from a number of glitches in the above interfaces that I was young and not very wise when I implemented this classes. Such glitches will be outlined later together with proposed fixes.
However, the most important problem here is the lack of a copy constructor and an assignment operator for Observer. The problem is evident in the following code snippet:
class SampleObserver : public Observer {
public:
void update() {
std::cout << "I felt a disturbance in the Force"
<< std::endl;
}
};
SampleObserver obiwan;
theForce.registerObserver(&obiwan); // this expands to
// theForce.observers().insert(&obiwan);
SampleObserver clone = obiwan; // clone is not registered with theForce
// so it is not an exact copy of obiwan
obiwan = SampleObserver(); // obiwan is still registered so it
// is not a newly initialized instance
This problem did not present itself yet because of the fact that classes derived from Observer such as Instrument or TermStructure are usually contained into Handles. This implies that observers are passed by reference rather than by value, so that copy constructors and assignment operators are never invoked.
However, implementations of the latter should be written which provide the right behavior. Such implementations are proposed in the following section.
Public methods are provided which pass an Observable raw pointers to Observer for storing. This can lead to crashes should an Observer be deleted before the Observables it registered with.
The Observable::notifyObservers has public visibility. However, notification should be managed by the observables themselves. Therefore, protected visibility would be a better choice. The same is true for the Observable::observers method.
Registration and unregistration of observers must be both performed manually. It is left to the programmer to synchronize the two operations, most often by remembering to unregister an Observer before it gets destroyed. This also means that he must keep track of the Observables with which the registration was done. Automatic unregistration would be safer besides relieving the programmer from the outlined tasks.
A std::set is used for storing Observers. This was chosen so that an observer which registered twice with the same observable would receive only one notification. However, registering twice and unregistering once would result in being unregistered altogether. Often, this is not the desired behavior.
Registration and unregistration should be driven from the Observer class. This would allow Observer instances to keep track of the Observables they registered with, automatically unregister with them before being destroyed, and implement correct copy behavior.
The new implementation is outlined in the following pseudo-code:
class Observable {
friend class Observer; // this allows Observer to drive the process
public:
virtual ~Observable() {}
protected:
virtual void notifyObservers(); // used by derived classes only
private:
// Observer management...
virtual void registerObserver(Observer*);
virtual void unregisterObserver(Observer*);
// ...and storage.
std::list<Observer*> observers_;
};
class Observer {
public:
Observer() {} // default constructor does nothing
// copy constructor
Observer(const Observer& o)
: observables_(o.observables_) {
for (each observable in observables_)
observable->registerObserver(this);
}
// assignment operator
Observer& operator=(const Observer& o) {
for (each observable in own observables_)
observable->unregisterObserver(this);
observables_ = o.observables_;
for (each observable in observables_)
observable->registerObserver(this);
}
// destructor with automatic unregistration
virtual ~Observer() {
for (each observable in own observables_)
observable->unregisterObserver(this);
}
// users will call this instead of the private Observable methods
void registerWith(Handle<Observable> h) {
store h in observables_;
h->registerObserver(this);
}
// this is provided in case a user wants to manually unregister
// before the end of the Observer lifetime
void unregisterWith(Handle<Observable> h) {
h->unregisterObserver(this);
remove h from observables_;
}
// to be implemented by derived classes
virtual void update() = 0;
private:
// Observables are kept track of.
std::list<Handle<Observable> > observables_;
};
This implementation solves all the outlined problems, namely:
Feedback on the above proposal should be posted to the QuantLib-dev mailing list.