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;
 }

• Maikel: Are you allowed to use the heap (shared_ptr/weak_ptr)?
• TradingDerivatives.eu: Yes, I am allowed to use smart pointers. I had great troubles in using std::shared_ptr and I gave up; too many templates confusing me. I wonder if creating some smart typedef synonyms would have helped.
• Loki Astari: What's this printf() function I see all through your code. Is this from another language or a private function you have written?

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.

• Enigma Maitreya: One reason one may want to modify the original/raw data is to filter/remove noise. Another but less valid, is if the information is being displayed real time and calibration values (polynomial, slope/offset etc.) are known then there may be a desire to see the raw data calibrated/corrected. I am going to reread the initial request and make sure I am taking your point in context. If I have replied to something out of context I am going to delete this message.
• Maikel: Good point about the noise. But extra caution must be taken because the order of observers matters.
• TradingDerivatives.eu: Processing the temperature and pressure data is something that the observers should do. In general, it is a bad idea to overwrite data received over an API.
• Maikel: Even then I would prefer to pass const references toNotify and return the modified value instead. Let the Subject handle the logic for passing modified values to successors. This scales better with possibly asynchronous NotifyObservers algorithms.
• TradingDerivatives.eu: Thanks a lot for your input! The CombiRegister class (and main.cpp) is intended just for testing the CSubject class. I studied how to get the instances of the CSubject class to a client class. I tried to work with smart pointers but I failed miserably. I decided to put this code on the site hoping for a push in the right direction. Thanks for this. The main idea is that the CSubject class offers a notification scheme to any data (here only temperature and pressure data) independently. I will have a good look at these comments, thank you. More to follow.