Observer pattern implementation in C++11

Question

Can this observer pattern implementation be improved?

Problem:

User will drive a car if pressed l --> car move left c--> move middle ,r--> move right

#include<iostream>
#include<vector>
#include<algorithm>

// -1 0 1
class Car // subject
{
        int position;
        std::vector<class observer*>observerList;
        public:
        int getPosition()
        {
                return position;
        }

        void setPosition(int newpos)
        {
                position =newpos;
                notify();
        }

        void attach(observer *obs)
        {
                observerList.push_back(obs);
        }

        void detach(observer *obs)
        {
                observerList.erase(std::remove(observerList.begin(),observerList.end(),obs),observerList.end());

        }
        void notify();
};

class observer
{
        Car *_car;
        public:
        observer(Car *car)
        {
                _car=car;
                _car->attach(this);
        }
        virtual void update()=0;
        protected :
        Car* getCar()
        {
                return _car;
        }
};

void Car::notify()
{
        if(observerList.empty())
                return;
        for(int i=0;i< observerList.size(); ++i)
        {
                observerList[i]-> update();
        }
}

class LeftObserver:public observer
{
        public: LeftObserver(Car *car):observer(car){}
                void update()
                {
                        int pos = getCar()->getPosition();
                        if(pos<0)
                        {
                                std::cout<<"Left side"<<"\n";
                        }

                }

};

class RightObserver : public  observer
{
        public: RightObserver(Car *car):observer(car) {}
                void update()
                {
                        int pos=getCar()->getPosition();
                        if(pos > 0)
                        {
                                std::cout<<"Right side"<<"\n";
                        }
                }
};

class Middleobserver : public observer
{
        public: Middleobserver(Car * car):observer(car){}

                void update()
                {               int pos=getCar()->getPosition();
                        if(pos==0)
                        {
                                std::cout<<"Middle side"<<"\n";
                        }
                }
};

int main()
{
        Car *car = new Car(); //subject
        LeftObserver letObserver(car);   // one to many relationship
        RightObserver rigthObserver(car);
        Middleobserver middleObsever(car);
        std::cout<<"hit left,right,middle to drive a car,press Esc to close"<<"\n";
        char pressedButton;
        bool breakLoop= false;
        while(breakLoop==false)
        {
                std::cin>>pressedButton;
                switch(pressedButton)
                {
                        case 108: // l ->pressed for left
                                {
                                        car->setPosition(-1);
                                        break;
                                }

                        case 99: // c ->pressed for middle 
                                {
                                        car->setPosition(0);
                                        break;
                                }

                        case 114: //  r ->pressed for right
                                {
                                        car->setPosition(1);
                                        break;
                                }

                        case 98: // b ->pressed for left
                                {

                                        breakLoop= true;
                                        break;
                                }
                        default :
                                {
                                std::cout<<"please drive carefully"<<std::endl;
                                break;
                                }
                }

        }
        std::cout<<"Bye"<<"\n";
}

Answer 1

Don't use push_back on vectors, basically ever

emplace_back() is almost always preferable.

Slow removal

Ideally, you'd want to store the observer list in an intrusive doubly-linked list, so that you can have O(1) removal. Unfortunately, this type of pattern is a bit of a blind spot in the STL, as there is no ideal container for it. (AFAIK, someone please correct me on this if I overlooked something).

if you don't have access to something like boost::intrusive::list, you can get around this by storing a std::list<Observer*>::iterator in the Observer itself. You "could" manually set up the list, but it's rarely worth the trouble.

Naming: Do not call a vector somethingList.

"List" is loaded with meaning, and implies guarantees that your code does not provide, so it's misleading

I would personally just use observers for the vector.

RAII

Since you attach the observer to the car in Observer's constructor, you should detach it in the destructor.

Consider using std::function instead of polymorphism for callbacks

Consider how much more flexible your code would be if observers was a: std::list<std::function<void(Car const&)>>.

Specifically, once you start scaling usage of this pattern up, you will find yourself with classes inheriting from 4-5 different listener interfaces, which will lead you to realize that update() is an awkward name, and a bunch of different headaches.

Here's a rough example to get you started: #include #include

class Car {
  std::vector<std::function<void(Car const&)>> observers_;
public:
  void attach(std::function<void(Car const&)> cb) {
    observers_.emplace_back(std::move(cb));
  }

  void update() {
    for(auto const& obs : observers_) {
      obs(*this);
    }
  }
};

class Whatever {
public:
  Whatever(Car* car) {
    car->attach([this](Car const& c) {onCarUpdate(c);});
  }
private:
  void onCarUpdate(Car const&) {
  }
};