Observer pattern implementation

Question

I'm reading Head First Design Patterns and I'm currently reading about the observer pattern. Is what I produced correct?

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
class ObserverInterface 
{
public:
 virtual void update(std::string message) = 0;
};
class concreteObserver : ObserverInterface 
{
 std::string name;
 std::string message;
public:
 concreteObserver(std::string name)
 :name(name)
 {}
 void update(std::string message)
 {
 this->message = message;
 std::cout << "Dear " << name << ", " << message << " from eBay" << std::endl;
 }
};
class SubjectInterface 
{
public:
 void subscribe(concreteObserver *subscriber)
 {
 subscribers.push_back(subscriber);
 }
 void unsubscribe(concreteObserver *subscriber)
 {
 subscribers.erase(std::remove(subscribers.begin(), subscribers.end(), subscriber), subscribers.end());
 }
 void notify(std::string message)
 {
 std::vector<concreteObserver*>::const_iterator itr = subscribers.begin();
 while (itr != subscribers.end())
 {
 (*itr)->update(message);
 ++itr;
 }
 }
private:
 std::vector<concreteObserver*> subscribers;
};
class concreteSubject : public SubjectInterface 
{
public:
 void newMessage(std::string msg)
 {
 notify(msg);
 }
};
int main(int argc, char* argv[])
{
 // EBAY
 concreteSubject ebay;
 // USERS
 concreteObserver user1("Steven");
 concreteObserver user2("John");
 // SUBSCRIBERS TO EBAY
 ebay.subscribe(&user1);
 ebay.subscribe(&user2);
 // EBAY SENDS NEW PROMOTIONS TO SUBSCRIBERS
 ebay.newMessage("enjoy free weekend listings");
 // UNSUBSCRIBE FROM EBAY
 ebay.unsubscribe(&user2);
 // EBAY SENDS NEW PROMOTION TO SUBSCRIBERS
 ebay.newMessage("free listings extended until 1st August");
 std::cout << std::endl << std::endl;
 system("pause");
 return 0;
}

Answer 1

Inheritance is by default private

class concreteObserver : ObserverInterface 
 ^^^^ Need public: here

Mark overriden methods:

void update(std::string message) // override ???

endl vs '\n'

Don't use endl unless you absolutely want to flush the buffer. Probably OK here. Just wanted to note it.

Don't pass pointers:

 void subscribe(concreteObserver *subscriber)

Is subscriber every going to be NULL?
Do you need to check if subscriber has been destroyed?

Personally. I would pass by reference and document that a subject must live longer than the observers. Not that just because you pass by reference does not mean you can hold a pointer.

 void subscribe(concreteObserver& subscriber) // pass by reference
 {
 subscribers.push_back(&subscriber); // but save pointer.
 }

But if you have a lot of dynamically allocated stuff this may not be suffecient. You could pass a std::weak_ptr then check of the subscriber is still available before sending the message.

Use the new foreach loop

 std::vector<concreteObserver*>::const_iterator itr = subscribers.begin();
 while (itr != subscribers.end())
 {
 (*itr)->update(message);
 ++itr;
 }

Easier to write as:

 for(auto& sub: subscribers) {
 sub->update(message);
 }

Answer 2

Yes, you have made an implementation of the observer pattern.

In you implementation of the notify function, you can use

for(auto subscriber : subscribers){
 (*subscriber)->update(message)
}

The name "SubjectInterface" is wrong, as you mix virtual declaration and code. It is an abstract class, so the name "AbstractSubject" should fit more.

You should use the "override" keyword, ie:

void subscribe override (concreteObserver *subscriber)
{
 ...
}