Search code examples
c++oopinheritancedependency-injectioncomposition

How to use composition instead of inheritance when dependency injection is involved?


I have a bunch of checkers in my program that I modelled as classes: check the RAM is OK, check the disk is OK, check the temperatures are OK, etc. These checkers have a lot in common, so I modelled them with inheritance: all that is in common goes into a base class CheckerBase that is derived from by specialised classes with checker-specific functionality and dependencies.

However I've often read that composition should be preferred over inheritance, so I'm wondering how this would be done in C++ with composition?

#include <chrono>
#include <iostream>
#include <thread>
#include <vector>
using namespace std;

/** Dependencies of various checkers that I pass in via dependency injection. */
struct ErrorReporter {
    void report_error(string myMsg) {
        cout << myMsg;
    }
};
struct TemperatureSensor {
    int get_cpu_temp() { return 42; }
    int get_disk_temp() { return 32; }
};
struct DiskStressor {
    void stress_disk() { }
};

/** Contains dependencies that are common to all checkers.. */
class CheckerBase {
public:
    CheckerBase(ErrorReporter* errReporter ) :
        mErrReporter(errReporter) { }

    virtual void runTest() = 0;
protected:
    ErrorReporter* mErrReporter;
};

/** Needs `TemperatureSensor` dependency. */
class TemperatureChecker : public CheckerBase {
public:
    TemperatureChecker(ErrorReporter* errReporter,
                       TemperatureSensor* tempSensor) :
        CheckerBase(errReporter), mTempSensor(tempSensor) { }

    void runTest() override {
        if (mTempSensor->get_cpu_temp() > 42) {
            mErrReporter->report_error("CPU too hot");
        }
     };
private:
    TemperatureSensor* mTempSensor;
};

/** Needs `TemperatureSensor` and `DiskStressor` dependencies. */
class DiskChecker : public CheckerBase {
public:
    DiskChecker(ErrorReporter* errReporter, TemperatureSensor* tempSensor,
                DiskStressor* diskStressor) :
        CheckerBase(errReporter), mTempSensor(tempSensor) { }

    void runTest() override {
        mDiskStressor->stress_disk();
        mTempSensor->get_disk_temp();
        if (mTempSensor->get_cpu_temp() > 32) {
            mErrReporter->report_error("HDD too hot after strees test");
        }
     };
private:
    TemperatureSensor* mTempSensor;
    DiskStressor* mDiskStressor;
};

/** Periodically runs each checker. */
class MasterChecker {
    public:
        MasterChecker() :
            mTempChecker { &mErrReporter, &mTempSensor },
            mDiskChecker { &mErrReporter, &mTempSensor, &mDiskStressor },
            mAllCheckers({&mTempChecker, &mDiskChecker}) {};

        void start() {
            // In reality I use a timer that continously runs each checker at
            // a certain interval.
            while (true) {
                for (CheckerBase *checker : mAllCheckers) {
                    checker->runTest();
                }
                this_thread::sleep_for(chrono::milliseconds(5000));
            }
        }
    private:
        ErrorReporter mErrReporter;
        TemperatureSensor mTempSensor;
        DiskStressor mDiskStressor;

        DiskChecker mDiskChecker;
        TemperatureChecker mTempChecker;

        vector<CheckerBase*> mAllCheckers;
};

int main() {
    MasterChecker master;
    master.start();
}

EDIT: Updated to include an approximation of how the checkers are used. A MasterChecker runs all the individual checkers periodically. It has a list of the checkers and calls their runTest() member function--which all checkers override from their base class.


Solution

  • ... composition should be preferred over inheritance

    That means, where you could choose either, prefer composition. In this case, MasterChecker (correctly) composes the various concrete checkers, as your advice recommended.

    The fact that the individual checkers inherit/implement an abstract base class isn't a problem, because you can't compose an interface. There's no choice here, and the advice didn't say you should never use inheritance even when composition isn't an alternative.

    The case your advice actually warns against is doing something like:

    class MasterChecker: public DiskChecker, public TemperatureChecker
    

    where inheritance is abused to aggregate the base class subobjects.

    In your case this probably wouldn't work well anyway, at least without changes, due to initialization order and diamond-shaped inheritance reasons.