Implementing the observer pattern for a stock exchange scenario I am trying to implement the observer pattern for a stock exchange scenario. Multiple stocks can be registered at a stock exchange. There will be stock observers who are observing some specific stocks. Please advise how this implementation can be improved.

stock_exchange.hpp

#ifndef STOCK_EXCHANGE_HPP
#define STOCK_EXCHANGE_HPP

#include "stock_observer.hpp"
#include "stock.hpp"
#include&lt;unordered_map&gt;
#include&lt;string&gt;
#include&lt;list&gt;

class Stock_Observer;
class Stock;

class Stock_Exchange
{
    std::unordered_map&lt;std::string, Stock*&gt; registered_stocks;
    std::unordered_map&lt;std::string, std::list&lt;Stock_Observer*&gt;&gt; stock_observers;

    public:
        void notify_observers(std::string stock_name);
        void unregister_observer(Stock_Observer&amp; so, std::string stock_name);
        void register_observer(Stock_Observer&amp; so, std::string stock_name);
        void add_stock(Stock&amp; new_stock);
        void remove_stock(Stock&amp; s);
        Stock&amp; get_stock(std::string stock_name);
};

#endif // STOCK_EXCHANGE_HPP


stock_exchange.cpp

#include "stock_exchange.hpp"
#include "stock_observer.hpp"
#include&lt;algorithm&gt;

void Stock_Exchange::add_stock(Stock&amp; new_stock)
{
    registered_stocks[new_stock.get_name()] = &amp;new_stock;
}

void Stock_Exchange::remove_stock(Stock&amp; s)
{
    registered_stocks.erase(s.get_name());
    stock_observers.erase(s.get_name()); //Memory leak!!!
}

Stock&amp; Stock_Exchange::get_stock(std::string stock_name)
{
    return *registered_stocks[stock_name];
}

void Stock_Exchange::register_observer(Stock_Observer&amp; observer, std::string stock_name)
{
    stock_observers[stock_name].push_back(&amp;observer);
}

void Stock_Exchange::unregister_observer(Stock_Observer&amp; observer, std::string stock_name)
{
    auto l = stock_observers[stock_name];
    l.erase(std::find(l.begin(), l.end(),&amp;observer));
}

void Stock_Exchange::notify_observers(std::string stock_name)
{
    for(auto o : stock_observers[stock_name])
    {
        o-&gt;notify(stock_name);
    }
}


stock_observer.hpp

#ifndef STOCK_OBSERVER_HPP
#define STOCK_OBSERVER_HPP

#include "stock.hpp"
#include "stock_exchange.hpp"
#include&lt;string&gt;

class Stock_Exchange;

class Stock_Observer
{
    static int id_sequence;
    int id;
    Stock_Exchange* se;
    public:
        Stock_Observer(Stock_Exchange* s)
        {
            se = s;
            id = id_sequence++;
        }
        void notify(std::string stock_name);
};

#endif // STOCK_OBSERVER_HPP


stock_observer.cpp

#include "stock_observer.hpp"
#include&lt;iostream&gt;

int Stock_Observer::id_sequence = 1;

void Stock_Observer::notify(std::string stock_name)
{
    std::cout &lt;&lt; "Object " &lt;&lt; id &lt;&lt; " notified about " &lt;&lt; stock_name &lt;&lt; std::endl;
}


stock.hpp

#ifndef STOCK_HPP
#define STOCK_HPP
#include "stock_exchange.hpp"
#include&lt;string&gt;

class Stock_Exchange;

class Stock
{
    std::string name;
    double price;
    Stock_Exchange* se;

    public:
        Stock(Stock_Exchange* s, std::string stock_name, double stock_price)
        {
            se = s;
            name = stock_name;
            price = stock_price;
        }
        std::string get_name();
        void set_price(double p);

};
#endif // STOCK_HPP


stock.cpp

#include "stock.hpp"

std::string Stock::get_name()
{
    return name;
}

void Stock::set_price(double p)
{
    price = p;
    se-&gt;notify_observers(this-&gt;name);
}


main.cpp

#include&lt;iostream&gt;
#include "stock_exchange.hpp"
#include "stock_observer.hpp"
#include "stock.hpp"

int main()
{
    Stock_Exchange se;

    Stock_Observer so1(&amp;se), so2(&amp;se);

    Stock infy(&amp;se,"INFY",1234.33);
    Stock tcs(&amp;se,"TCS",1100);
    Stock sbi(&amp;se,"SBI",2312.33);

    se.register_observer(so1,"INFY");
    se.register_observer(so2,"INFY");
    se.register_observer(so1,"TCS");

    infy.set_price(1250.33);
    tcs.set_price(999.32);
    sbi.set_price(2300.00);

}
Make immutable class members const

If you have a member of a class that is set once at construction time, but will never change, then make it const. For example, in Stock:

class Stock {
  const std::string name;
  ...
  public:
  Stock(..., std::string stock_name, ...): name(stock_name) {
    ...
  }


And once it is const, you can safely make name public and remove get_name().

Avoid circular references

Your stock exchanges contain stocks, but stocks also refer to stock exchanges. This circular reference is unnecessary and might give you problems; for example you had to add forward references to class Stock_Observer and class Stock in stock_exchange.hpp. Conceptually, a stock is not tied to a specific stock exchange, so just remove the member variable Stock::se.

Of course, the problem is now how to notify observers when the stock price changes. One way to handle that is to allow one to set a callback function in a Stock, for example:

#include &lt;functional&gt;

class Stock {
  std::function&lt;void(Stock &amp;)&gt; price_changed_callback;
  double price;
  ...
  public:
  void set_price_changed_callback(std::function&lt;void(Stock &amp;)&gt; callback) {
    price_changed_callback = callback;
  }

  void set_price(double p) {
    price = p;
    if (price_changed_callback)
      price_changed_callback(*this);
  }
  ...
}


Then the stock exchange calls set_price_changed_callback() when it registers the stock, and sets it to something like its notify_observers() function.
If you want to have a Stock be part of multiple exchanges, then you'd better look for a library that handles more complex callback registration for you, like libsigc++.

Another option would be to move set_price() to the stock exchange itself.

Think carefully about ownership

In your code, stock exchanges have pointers to stocks, and stocks have a pointer to a stock exchange. But what if a stock goes out of scope before a stock exchange, or vice versa? It's not a problem in your simple main(), but once your code grows the chance you will make a mistake will increase. It's best to use the right tools to control ownership.

One possibility is to use shared_ptr&lt;&gt;. Make it so you can only create a shared_ptr to a Stock:

class Stock {
  private:
  Stock() { ... }

  public:
  static std::shared_ptr&lt;Stock&gt; make_stock(args) {
    return std::make_shared&lt;Stock&gt;(args);
  }
}


And have Stock_Exchange keep a map of shared_ptrs:

class Stock_Exchange {
  std::unordered_map&lt;std::string, std::shared_ptr&lt;Stock&gt;&gt; registered_stocks;
  ...
}


Alternatively, if you'd only ever have stocks associated with a single exchange, you could have Stock_Exchange take direct ownership:

class Stock_Exchange {
  std::unordered_map&lt;std::string, Stock&gt; registered_stocks;
  ...
  public:
  register_stock(std::string name, double price) {
    register_stocks.emplace(std::make_pair(name, Stock(name, price));
  }
}


Of course, if you go that route there is little point in having a class Stock anymore.

Use references when iterating over a container where possible

In this part of your code:

for(auto o : stock_observers[stock_name])
{
    o-&gt;notify(stock_name);
}


You are actually making a copy of each observer. It is almost always better to use a reference when iterating through a container:

for(auto &amp;o : stock_observers[stock_name])
{
    o-&gt;notify(stock_name);
}