patterncppMinor
Still an observer pattern?
Viewed 0 times
stillobserverpattern
Problem
Usually when I see tutorials about Observer Pattern I see an unique method called
notify, but I'm wondering. What if I have different methods that can be called in different moments but needs to notify the others when this happen? Like events, am I doing this wrong? or still begin the observer pattern?#include
#include
#include
class Observer
{
public:
virtual void notifyBefore() = 0;
virtual void notifyAfter() = 0;
};
class Subject
{
public:
void attachObserver(Observer * observer)
{
observers.push_back(observer);
}
void detachObserver(Observer * observer)
{
auto index = std::find(observers.begin(), observers.end(), observer);
if (index != observers.end())
{
observers.erase(index);
}
}
virtual void notifyBefore()
{
for (auto current : observers)
{
current->notifyBefore();
}
}
virtual void notifyAfter()
{
for (auto current : observers)
{
current->notifyAfter();
}
}
private:
std::vector observers;
};
class ConcreteObserver : public Observer
{
public:
void notifyBefore()
{
std::cout attachObserver(new ConcreteObserver);
subject->notifyBefore();
for (int i = 0; i notifyAfter();
}Solution
Yes its still the observer pattern (details in my comment on the question).
But there are a few implementation details that are bad for a C++ context.
Good interface for your observer:
But you should also make the destructor virtual. It is quite probable that these objects will be destroyed via a pointer to the base class. Without a virtual destructor this is undefined behavior.
You have the basics of subject correct.
My one problem is here. You are passing a pointer to an observer. Pointers do not express ownership. So it is unclear for future users of your object if you should pass a dynamically allocated object or a a pointer to an automatic object. It becomes clear by reading your class that you are not taking ownership of the pointer and your class always assumes that it is not NULL. So in this case it is probably better to pass it by reference.
If there are cases were ownership can be passed then you should use smart pointers. A quick search on google for smart pointers or ownership semantics should give you lots of references.
Implementation:
In C++11 we added the
This is not Java
You should prefer to use automatic objects (that is perfect here). If you are going to use
But there are a few implementation details that are bad for a C++ context.
Good interface for your observer:
class Observer
{
public:
virtual void notifyBefore() = 0;
virtual void notifyAfter() = 0;
};But you should also make the destructor virtual. It is quite probable that these objects will be destroyed via a pointer to the base class. Without a virtual destructor this is undefined behavior.
You have the basics of subject correct.
class Subject
{
public:
void attachObserver(Observer * observer)
{
observers.push_back(observer);
}My one problem is here. You are passing a pointer to an observer. Pointers do not express ownership. So it is unclear for future users of your object if you should pass a dynamically allocated object or a a pointer to an automatic object. It becomes clear by reading your class that you are not taking ownership of the pointer and your class always assumes that it is not NULL. So in this case it is probably better to pass it by reference.
If there are cases were ownership can be passed then you should use smart pointers. A quick search on google for smart pointers or ownership semantics should give you lots of references.
void attachObserver(Observer& observer)
{
observers.push_back(&observer); // Save the address of the observer
// It is the responsibility of the observer
// to make sure it lives long enough or
// detaches itself on destruction.
}Implementation:
class ConcreteObserver : public Observer
{
public:
void notifyBefore()
{
std::cout << "You called me before..." << std::endl;
}
void notifyAfter()
{
std::cout << "You called me after..." << std::endl;
}
};In C++11 we added the
override keyword. This should be used in case you refactor your class. This will help in finding locations were you have misnamed the functions:void notifyAfter() override ....This is not Java
auto subject = new ConcreteSubject;
subject->attachObserver(new ConcreteObserver);You should prefer to use automatic objects (that is perfect here). If you are going to use
new then you should wrap the result in a smart pointer to make sure the allocation is exception safe.Code Snippets
class Observer
{
public:
virtual void notifyBefore() = 0;
virtual void notifyAfter() = 0;
};class Subject
{
public:
void attachObserver(Observer * observer)
{
observers.push_back(observer);
}void attachObserver(Observer& observer)
{
observers.push_back(&observer); // Save the address of the observer
// It is the responsibility of the observer
// to make sure it lives long enough or
// detaches itself on destruction.
}class ConcreteObserver : public Observer
{
public:
void notifyBefore()
{
std::cout << "You called me before..." << std::endl;
}
void notifyAfter()
{
std::cout << "You called me after..." << std::endl;
}
};void notifyAfter() override ....Context
StackExchange Code Review Q#64172, answer score: 4
Revisions (0)
No revisions yet.