Search code examples
c++dependency-injectiondependenciespolymorphismsmart-pointers

At what inheritance level to store a dependency pointer when both dependency and dependent are polymorphic?


I'm currently dealing with a situation where a polymorphic object has an injected dependency which is also polymorphic. My question is about the best way for the first family of classes to have behavior common to the family, requiring a call to a virtual method first defined in the top base class of the second family. Specifically, about where to store the owning smart pointer to the dependency - in the base, in derived classes, or in both places. (This question is about using smart pointers for the task, but a similar problem exists when using references or raw pointers for DI.)

I'll use some example base classes Worker and Job to illustrate. Every Worker owns one Job, constructor-injected. DerivedWorkers might require the user to inject a specific DerivedJob, and call methods specific to that DerivedJob. Every Worker must have public method get_location(), and the logic for this function is the same for all Workers, and it requires a call to a virtual method get_hours() defined in Job and overrode by its children. These are the three strategies I've come up with, "pointer in base and derived," "pointer in derived only," and "pointer in base only":

class Job
{
public:
    virtual ~Job();
    virtual Hours_t get_hours();
};

class DerivedJob : public Job
{
public:
    virtual Hours_t get_hours();
    void derived_specific_method();
};

Strategy 1: Pointer in base and derived

class Worker
{
public:
    Worker(std::shared_ptr<Job> job) : my_job(job) {}
    virtual ~Worker();
    Location_t get_location()
    {
        return some_logic(my_job->get_hours());
    }
private:
    std::shared_ptr<Job> my_job; //cannot be unique_ptr
};

class DerivedWorker : public Worker
{
public:
    DerivedWorker(std::shared_ptr<DerivedJob> derivedJob) : Worker(derivedJob), my_derived_job(derivedJob) {}
    void derived_specific_duty()
    {
        my_derived_job->derived_specific_method();
    }
private:
    std::shared_ptr<DerivedJob> my_derived_job;
};

Strategy 2: pointer in derived only

class Worker //abstract
{
public:
    virtual ~Worker();
    virtual Location_t get_location() = 0;
};

class DerivedWorker : public Worker
{
public:
    DerivedWorker(std::unique_ptr<DerivedJob> derivedJob) : my_derived_job(derivedJob) {}
    virtual Location_t get_location()
    {
        return some_logic(my_derived_job->get_hours());
    }
    void derived_specific_duty()
    {
        my_derived_job->derived_specific_method();
    }
private:
    std::unique_ptr<DerivedJob> my_derived_job;
};

Strategy 3: pointer in base only

class Worker
{
public:
    Worker(std::unique_ptr<Job> job) : my_job(job) {}
    virtual ~Worker();
    Location_t get_location()
    {
        return some_logic(my_job->get_hours());
    }
protected:
    std::unique_ptr<Job> my_job;
};

class DerivedWorker : public Worker
{
public:
    DerivedWorker(std::unique_ptr<DerivedJob> derivedJob) : Worker(derivedJob) {}
    void derived_specific_duty()
    {
        dynamic_cast<DerivedJob*>(my_job.get())->derived_specific_method();
    }
};

Each comes with downsides and I'm trying to figure out if there's a fourth method I'm missing, if there's an idiomatic or "best" way of doing this, or if I'm missing some refactoring trick that makes this type of dependency pattern obsolete.

For 1, "pointers in base and derived," the drawback is that you can't use unique_ptr even if each Job is owned by only one Worker, because each Worker technically can own several smart pointers to the same Job. This might be a problem if Workers are frequently moved, or Jobs are swapped between Workers, due to the cache cohesion slowdowns introduced by moving shared_ptrs. This is the strategy I'm currently leaning towards.

For 2, "pointer in derived only," the drawback is a ton of code duplication. get_location() has to be virtual despite being almost exactly the same code for all Workers. In addition, now Worker may have to be abstract. (In this particular example, you could avoid this by making a null value for Location_t, but that's not always feasible in real applications of this problem.)

For 3, "pointer in base only," the drawback is having to use dynamic_cast, which is a huge code smell for a reason. Huge runtime costs, having to add in checks for the failed cast case, etc.


Solution

  • I would go for a variant of 2. :

    Instead of virtual Location_t get_location() = 0; better to have virtual Job& get_job() = 0;, so you can use covariant return type in derived class, and implementation of get_location() is not duplicated.

    class Worker // abstract
    {
    public:
        virtual ~Worker() = default;
        virtual Job& get_job() = 0;
    
        Location_t get_location() { return some_logic(get_job().get_hours()); }
    };
    
    class DerivedWorker : public Worker
    {
    public:
        explicit DerivedWorker(std::unique_ptr<DerivedJob> derivedJob) : my_job(std::move(derivedJob)) {}
    
        DerivedJob& get_job() override { return *my_job;}
    
        void derived_specific_duty() { my_job->derived_specific_method(); }
    private:
        std::unique_ptr<DerivedJob> my_job;
    };