1
\$\begingroup\$

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;
}
\$\endgroup\$
4
\$\begingroup\$

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);
        }
\$\endgroup\$
3
\$\begingroup\$

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)
{
   ...
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.