Observer pattern for temperature and pressure sensors

Question

I have tried to implement the observer pattern. The code should be able to handle a variable number of observables (set compile time) and notify any number of observers/registerables. Any observable is suitable because of the use of templates. The observers/registerers (?) must be derived from CRegisterableBase. Hence, polymorphism.

In a recent review I have learned about smart pointers and the Rule of Three... but using a std::shared_ptr turned out to be quite hard for this use case. I tried to be defensive a few times by implementing private operator= methods and private copy constructors. Good idea?

Compiling

g++ -Wall -g -std=c++11  -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/ObserverPattern/CAnotherPressureRegisterable.cpp -o obj/Debug/CAnotherPressureRegisterable.o
g++ -Wall -g -std=c++11  -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/ObserverPattern/CAnotherTemperatureRegisterable.cpp -o obj/Debug/CAnotherTemperatureRegisterable.o
g++ -Wall -g -std=c++11  -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/ObserverPattern/CCombiRegister.cpp -o obj/Debug/CCombiRegister.o
g++ -Wall -g -std=c++11  -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/ObserverPattern/CPressureRegisterable.cpp -o obj/Debug/CPressureRegisterable.o
g++ -Wall -g -std=c++11  -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/ObserverPattern/CTemperatureRegisterable.cpp -o obj/Debug/CTemperatureRegisterable.o
g++ -Wall -g -std=c++11  -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/ObserverPattern/main.cpp -o obj/Debug/main.o
g++  -o bin/Debug/ObserverPattern obj/Debug/CAnotherPressureRegisterable.o obj/Debug/CAnotherTemperatureRegisterable.o obj/Debug/CCombiRegister.o obj/Debug/CPressureRegisterable.o obj/Debug/CTemperatureRegisterable.o obj/Debug/main.o   

CAnotherPressureRegisterable.cpp

    #include <stdio.h>

    #include "CAnotherPressureRegisterable.h"

    void CAnotherPressureRegisterable::Notify(TPressure& aPressure)
    {
      printf("CAnotherPressureRegisterable::Notify(...): Received aPressure.iPressureData= %i\n",
             aPressure.iPressureData);
    }

    CAnotherPressureRegisterable::~CAnotherPressureRegisterable()
    {
      printf("CAnotherPressureRegisterable::~CAnotherPressureRegisterable(): Started.\n");
    }

    CAnotherPressureRegisterable CAnotherPressureRegisterable::operator=(const CAnotherPressureRegisterable& aAnotherPressureRegisterable)
    {
      printf("CAnotherPressureRegisterable::operator=(...): Do not copy an instance of this class.\n");
      CAnotherPressureRegisterable myAnotherPressureRegisterable;
      return myAnotherPressureRegisterable;
    }

CAnotherPressureRegisterable.h

    #ifndef CANOTHERPRESSUREREGISTERABLE_H
    #define CANOTHERPRESSUREREGISTERABLE_H

    #include "TPressure.h"
    #include "CSubject.h"

    class CAnotherPressureRegisterable : public CRegisterableBase<TPressure>
    {
    public:
      void Notify(TPressure& aPressure);

      ~CAnotherPressureRegisterable();

    private:
      CAnotherPressureRegisterable operator=(const CAnotherPressureRegisterable& aAnotherPressureRegisterable);
    };

    #endif // CANOTHERPRESSUREREGISTERABLE_H

CAnotherTemperatureRegisterable.cpp

    #include <stdio.h>

    #include "CAnotherTemperatureRegisterable.h"

    void CAnotherTemperatureRegisterable::Notify(TTemperature& aTemperature)
    {
      printf("CAnotherTemperatureRegisterable::Notify(...): Received aTemperature.iTemperatureData= %i\n",
             aTemperature.iTemperatureData);
    }

    CAnotherTemperatureRegisterable::~CAnotherTemperatureRegisterable()
    {
      printf("CAnotherTemperatureRegisterable::~CAnotherTemperatureRegisterable(): Started.\n");
    }

    CAnotherTemperatureRegisterable CAnotherTemperatureRegisterable::operator=(const CAnotherTemperatureRegisterable& aAnotherPressureRegisterable)
    {
      printf("CAnotherTemperatureRegisterable::operator=(...): Do not copy an instance of this class.\n");
      CAnotherTemperatureRegisterable myAnotherTemperatureRegisterable;
      return myAnotherTemperatureRegisterable;
    }

CAnotherTemperatureRegisterable.h

    #ifndef CANOTHERTEMPERATUREREGISTERABLE_H
    #define CANOTHERTEMPERATUREREGISTERABLE_H

    #include "TTemperature.h"
    #include "CSubject.h"

    class CAnotherTemperatureRegisterable : public CRegisterableBase<TTemperature>
    {
    public:
      void Notify(TTemperature& aTemperature);

      ~CAnotherTemperatureRegisterable();

    private:
      CAnotherTemperatureRegisterable operator=(const CAnotherTemperatureRegisterable& aAnotherPressureRegisterable);
    };


    #endif // CANOTHERTEMPERATUREREGISTERABLE_H

CCombiRegister.cpp

    #include <stdio.h>

    #include "CCombiRegister.h"

    CCombiRegister::CCombiRegister(CSubject<TTemperature>* aTemperatureSubjectPtr,
                                   CSubject<TPressure>* aPressureSubjectPtr)
      : iTemperatureSubjectPtr(aTemperatureSubjectPtr),
        iPressureSubjectPtr(aPressureSubjectPtr)
    {
      printf("CRegisterPressureAndTemperature::CRegisterPressureAndTemperature(): Started.\n");
      iTemperatureRegisterablePtr = new CAnotherTemperatureRegisterable();
      iPressureRegisterablePtr    = new CAnotherPressureRegisterable();
      printf("CRegisterPressureAndTemperature::CRegisterPressureAndTemperature(): Finished.\n");
    }

    CCombiRegister::CCombiRegister(CCombiRegister& aRegisterPressureAndTemperature)
    {
    }

    CCombiRegister::~CCombiRegister()
    {
      iTemperatureSubjectPtr->Unregister(iTemperatureRegisterablePtr);
      iPressureSubjectPtr->Unregister(iPressureRegisterablePtr);

      delete iTemperatureRegisterablePtr;
      delete iPressureRegisterablePtr;
    }

    void CCombiRegister::RegisterTemperature(CSubject<TTemperature>& aSubject)
    {
      printf("CRegisterPressureAndTemperature::RegisterTemperature(...): Started.\n");
      aSubject.Register(iTemperatureRegisterablePtr);
      printf("CRegisterPressureAndTemperature::RegisterTemperature(...): Finished.\n");
    }

    void CCombiRegister::RegisterPressure(CSubject<TPressure>& aSubject)
    {
      printf("CRegisterPressureAndTemperature::RegisterPressure(...): Started.\n");
      aSubject.Register(iPressureRegisterablePtr);
      printf("CRegisterPressureAndTemperature::RegisterPressure(...): Finished.\n");
    }

CCombiRegister.h

    #ifndef CCOMBIREGISTER_H
    #define CCOMBIREGISTER_H

    #include "TTemperature.h"
    #include "TPressure.h"

    #include "CSubject.h"

    #include "CAnotherTemperatureRegisterable.h"
    #include "CAnotherPressureRegisterable.h"

    class CCombiRegister
    {
    public:
      CCombiRegister(CSubject<TTemperature>* aTemperatureSubjectPtr, CSubject<TPressure>* aPressureSubjectPtr);
      ~CCombiRegister();

      void RegisterTemperature(CSubject<TTemperature>& aSubject);
      void RegisterPressure(CSubject<TPressure>& aSubject);

    private:
      CCombiRegister(CCombiRegister& aRegisterPressureAndTemperature);

      CAnotherTemperatureRegisterable* iTemperatureRegisterablePtr;
      CAnotherPressureRegisterable* iPressureRegisterablePtr;

      CSubject<TTemperature>* iTemperatureSubjectPtr;
      CSubject<TPressure>* iPressureSubjectPtr;
      // I tried to implement a smart pointer here. FAIL
    };

    #endif // CCOMBIREGISTER_H

CPressureRegisterable.cpp

    #include <stdio.h>

    #include "CPressureRegisterable.h"

    void CPressureRegisterable::Notify(TPressure& aPressure)
    {
      printf("CPressureRegisterable::Notify(...): Received aPressure.iPressureData= %i\n",
             aPressure.iPressureData);
    }
    CPressureRegisterable::~CPressureRegisterable()
    {
      printf("CPressureRegisterable::~CPressureRegisterable(): Started.\n");
    }

    CPressureRegisterable CPressureRegisterable::operator=(const CPressureRegisterable& aPressureRegisterable)
    {
      printf("CPressureRegisterable::operator=(...): Do not copy an instance of this class.\n");
      CPressureRegisterable myPressureRegisterable;
      return myPressureRegisterable;
    }

CPressureRegisterable.h

    #ifndef CPRESSUREREGISTERABLE_H
    #define CPRESSUREREGISTERABLE_H

    #include "TPressure.h"
    #include "CSubject.h"

    class CPressureRegisterable : public CRegisterableBase<TPressure>
    {
    public:
      void Notify(TPressure& aPressure);

      ~CPressureRegisterable();

    private:
      CPressureRegisterable operator=(const CPressureRegisterable& aPressureRegisterable);
    };

    #endif // CPRESSUREREGISTERABLE_H

CSubject.h

    #ifndef CSUBJECT_H
    #define CSUBJECT_H

    #include <algorithm>    // std::find
    #include <vector>       // std::vector

    template <typename TypenameObservable>
    class CRegisterableBase
    {
    public:
      virtual void Notify(TypenameObservable& aTypenameObservable) = 0;
      virtual ~CRegisterableBase() {};
    };

    template <typename TypenameObservable>
    class CSubject
    {
    public:
      void Register(CRegisterableBase<TypenameObservable>* aRegisterablePtr)
      {
        printf("CSubject::Register(): Started.\n");
        typename std::vector<CRegisterableBase<TypenameObservable>*>::iterator it;
        it = std::find(iRegisteredVector.begin(), iRegisteredVector.end(), aRegisterablePtr);

        if (it == iRegisteredVector.end())
        {
          iRegisteredVector.push_back(aRegisterablePtr);
        }
        else
        {
          printf("CSubject::Register(): This registerable was already added.\n");
        }

        printf("CSubject::Register(): Finished.\n");
      }

      /////////////////////////////////////////////////////////////////////////////////////////////

      void Unregister(CRegisterableBase<TypenameObservable>* aRegisterablePtr)
      {
        printf("CSubject::Unregister(): Started.\n");

        typename std::vector<CRegisterableBase<TypenameObservable>*>::iterator it;
        it = std::find(iRegisteredVector.begin(), iRegisteredVector.end(), aRegisterablePtr);

        if (it != iRegisteredVector.end())
        {
          iRegisteredVector.erase(it);
        }
        else
        {
          printf("CSubject::Unregister(): This registerable was not added.\n");
        }


        printf("CSubject::Unregister(): Finished.\n");
      }

      /////////////////////////////////////////////////////////////////////////////////////////////

      void NotifyObservers(TypenameObservable& aObservable)
      {
        printf("CSubject::NotifyObservers(): Started.\n");

        for(typename std::vector<CRegisterableBase<TypenameObservable>*>::iterator it = iRegisteredVector.begin(); it < iRegisteredVector.end(); it++)
        {
          (*it)->Notify(aObservable);
        }

        printf("CSubject::NotifyObservers(): Finished.\n");
      }

    private:
      std::vector<CRegisterableBase<TypenameObservable>*> iRegisteredVector;
    };

    #endif // CSUBJECT_H

CTemperatureRegisterable.cpp

    #include "TTemperature.h"
    #include "CSubject.h"
    #include "CTemperatureRegisterable.h"

    void CTemperatureRegisterable::Notify(TTemperature& aTemperature)
    {
      printf("CTemperatureRegisterable::Notify(...): Received aTemperature.iTemperatureData= %i\n",
             aTemperature.iTemperatureData);
    }

    CTemperatureRegisterable::~CTemperatureRegisterable()
    {
      printf("CTemperatureRegisterable::~CTemperatureRegisterable(): Started.\n");
    }

    CTemperatureRegisterable CTemperatureRegisterable::operator=(const CTemperatureRegisterable& aTemperatureRegisterable)
    {
      printf("CTemperatureRegisterable::operator=(...): Do not copy an instance of this class.\n");
      CTemperatureRegisterable myTemperatureRegisterable;
      return myTemperatureRegisterable;
    }

CTemperatureRegisterable.h

    #ifndef CTEMPERATUREREGISTERABLE_H
    #define CTEMPERATUREREGISTERABLE_H

    #include "TTemperature.h"
    #include "CSubject.h"

    class CTemperatureRegisterable : public CRegisterableBase<TTemperature>
    {
    public:
      void Notify(TTemperature& aTemperature);

      ~CTemperatureRegisterable();

    private:
      CTemperatureRegisterable operator=(const CTemperatureRegisterable& aTemperatureRegisterable);
    };

    #endif // CTEMPERATUREREGISTERABLE_H

TPressure.h

    #ifndef TPRESSURE_H
    #define TPRESSURE_H

    typedef struct
    {
      int iPressureData;
    } TPressure;

    #endif // TPRESSURE_H

TTemperature.h

    #ifndef TTEMPERATURE_H
    #define TTEMPERATURE_H

    typedef struct
    {
      int iTemperatureData;
    } TTemperature;

    #endif // TTEMPERATURE_H

main.cpp

    #include <stdio.h>

    #include "TTemperature.h"
    #include "TPressure.h"

    #include "CTemperatureRegisterable.h"
    #include "CPressureRegisterable.h"
    #include "CCombiRegister.h"

    #include "CSubject.h"

    int main()
    {
      TTemperature t1 = {101};
      TTemperature t2 = {102};
      TTemperature t3 = {103};

      TPressure p1 = {201};
      TPressure p2 = {202};
      TPressure p3 = {203};

      CSubject<TTemperature> temperatureSubject;
      CSubject<TPressure> pressureSubject;

      CTemperatureRegisterable temperatureClient1;
      CTemperatureRegisterable temperatureClient2;

      temperatureSubject.Register(static_cast<CRegisterableBase<TTemperature>*> (&temperatureClient1));
      temperatureSubject.Register(&temperatureClient2);

      temperatureSubject.NotifyObservers(t1);
      temperatureSubject.NotifyObservers(t2);

      temperatureSubject.Unregister(&temperatureClient1);

      temperatureSubject.NotifyObservers(t1);

      CPressureRegisterable pressureClient1;
      CPressureRegisterable pressureClient2;

      pressureSubject.Register(&pressureClient1);
      pressureSubject.Register(&pressureClient2);

      pressureSubject.NotifyObservers(p1);
      pressureSubject.NotifyObservers(p2);

      pressureSubject.Unregister(&pressureClient1);

      pressureSubject.NotifyObservers(p1);
      pressureSubject.NotifyObservers(p2);

      CCombiRegister myRegCombi(&temperatureSubject, &pressureSubject);

      myRegCombi.RegisterTemperature(temperatureSubject);
      myRegCombi.RegisterPressure(pressureSubject);

      temperatureSubject.NotifyObservers(t3);
      pressureSubject.NotifyObservers(p3);

      return 0;
    }

Answer 1

The way how you use pointers is quite dangerous and C++ has tools to make this totally safe. The biggest problem of your implementation is the usage of possibly dangling pointers in your subject classes. But also your pointers in interfaces are totally unguarded against nullptrs. Just don't pass pointers through functions if you do not have to: pass them as references.

There are multiple ways to tackle the lifetime issues which your code could potentially face:

• Subjects own their observers in some way (by value or shared pointer or whatever). That way observers never dangle, by definition.

• Observers cancel their submission to their subjects in their destructors. That's a really dirty way, since it beats the purpose of the pattern. In that scenario not only subjects handle observers, but you either have some global instance which knows of all connections between subjects or observers themself know which subjects they have registered to. Eh... no.

• Subjects hold only weak references in form of std::weak_ptr to their observers. A weak reference can tell whether some observer still lives and can be constructed from a std::shared_ptr.

I think that getting rid of this issue is currently most important. Maybe you are bound by your compiler, but you use really old C++ style which could be modernized. You also mix C-style with C++-style, which isn't really recommended.

I want to go with a solution like the first above because of simplicity and to show you std::shared_ptr semantics.

Let's go through your files and unify them for demonstration purpose (complete source code here). Lets also look at how all this can be handled. We start with the definitions of TPressure and TTemperature. You do not have to typedef structures in C++!

struct TPressure
{
    int iPressureData;
};

struct TTemperature
{
    int iTemperatureData;
};

No lets jump into CSubject.h! CRegisterableBase is good, but unless you do not intend to modify the observable value, don't pass by reference! (I do not see why you should modify it)

template <typename Observable>
class CRegisterableBase
{
public:
    virtual void Notify(const Observable&) = 0;
    virtual ~CRegisterableBase() = default;
};

A subject should hold a list (or vector, whatever) of std::shared_ptr to registrable objects. I remove all your output logic for demonstration and it boils down to

template <typename Observable>
class CSubject
{
private:
    using ObserverPointer = std::shared_ptr<CRegisterableBase<Observable>>;
    std::vector<ObserverPointer> observers;

public:
    bool Register(ObserverPointer observer) noexcept
    {
        if (!observer) return false; // check if it is empty
        auto it = std::find(observers.begin(), observers.end(), observer);
        if (it == observers.end()) {
            observers.emplace_back(std::move(observer));
            return true;
        }
        return false;
    }

    bool Unregister(const ObserverPointer& observer) noexcept
    {
        auto it = std::find(observers.begin(), observers.end(), observer);
        if (it != observers.end()) {
            observers.erase(it);
            return true;
        }
        return false;
    }

    void NotifyObservers(const Observable& observable) const
    {
        for (auto& observer : observers) {
            observer->Notify(observable);
        }
    }

    decltype(auto) Observers() const noexcept { return observers; }
};

Now the subject holds all the logic and the rest should be really simple.

class CPressureRegisterable : public CRegisterableBase<TPressure>
{
public:
    void Notify(const TPressure& aPressure) override
    {
        // do something
        std::cout << "Recieved pressure data: " << aPressure.iPressureData << '\n';
    }
};

class CTemperatureRegisterable : public CRegisterableBase<TTemperature>
{
public:
    void Notify(const TTemperature& aTemperature) override
    {
        // do something
        std::cout << "Recieved temperature data: " << aTemperature.iTemperatureData << '\n';
    }
};

And now an example main() function:

int main()
{
    auto p_observer = std::make_shared<CPressureRegisterable>();
    CSubject<TPressure> subject_1;
    subject_1.Register(p_observer);

    // go and print "Recieved temperature data: 201\n"
    TPressure p = {201};
    subject_1.NotifyObservers(p);
}

Your CCombiRegister class could take subjects by value and just forward pressure and temperature data. Just don't use pointers again if you don't have to. For example, you could use two (or variadic many) template parameters in CCombiRegister and combine arbitrary subjects. That way you can also pass a std::reference_wrapper (caution for dangling again) or a std::shared_ptr to subject.

There are still ways to improve this! If all subjects and observers are known to compile time, you can bind subjects and observers without any virtual interface and heap memory. But I feel like you should first learn more C++-ish programming.