Question

I often use the "dependency injection" pattern in my projects. In C++ it is easiest to implement by passing around raw pointers, but now with C++11, everything in high-level code should be doable with smart pointers. But what is the best practice for this case? Performance is not critical, a clean and understandable code matters more to me now.

Let me show a simplified example. We have an algorithm that uses distance calculations inside. We want to be able to replace this calculation with different distance metrics (Euclidean, Manhattan, etc.). Our goal is to be able to say something like:

SomeAlgorithm algorithmWithEuclidean(new EuclideanDistanceCalculator());
SomeAlgorithm algorithmWithManhattan(new ManhattanDistanceCalculator());

but with smart pointers to avoid manual new and delete. This is a possible implementation with raw pointers:

class DistanceCalculator {
public:
    virtual double distance(Point p1, Point p2) = 0;
};

class EuclideanDistanceCalculator {
public:
    virtual double distance(Point p1, Point p2) {
        return sqrt(...);
    }
};

class ManhattanDistanceCalculator {
public:
    virtual double distance(Point p1, Point p2) {
        return ...;
    }
};

class SomeAlgorithm {
    DistanceCalculator* distanceCalculator;

public:
    SomeAlgorithm(DistanceCalculator* distanceCalculator_)
        : distanceCalculator(distanceCalculator_) {}

    double calculateComplicated() {
        ...
        double dist = distanceCalculator->distance(p1, p2);
        ...
    }

    ~SomeAlgorithm(){
        delete distanceCalculator;
    }
};

Let's assume that copying is not really an issue, and if we didn't need polymorphism we would just pass the DistanceCalculator to the constructor of SomeAlgorithm by value (copying). But since we need to be able to pass in different derived instances (without slicing), the parameter must be either a raw pointer, a reference or a smart pointer.

One solution that comes to mind is to pass it in by reference-to-const and encapsulate it in a std::unique_ptr<DistanceCalculator> member variable. Then the call would be:

SomeAlgorithm algorithmWithEuclidean(EuclideanDistance());

But this stack-allocated temporary object (rvalue-reference?) will be destructed after this line. So we'd need some copying to make it more like a pass-by-value. But since we don't know the runtime type, we cannot construct our copy easily.

We could also use a smart pointer as the constructor parameter. Since there is no issue with ownership (the DistanceCalculator will be owned by SomeAlgorithm) we should use std::unique_ptr. Should I really replace all of such constructor parameters with unique_ptr? it seems to reduce readability. Also the user of SomeAlgorithm must construct it in an awkward way:

SomeAlgorithm algorithmWithEuclidean(std::unique_ptr<DistanceCalculator>(new EuclideanDistance()));

Or should I use the new move semantics (&&, std::move) in some way?

It seems to be a pretty standard problem, there must be some succinct way to implement it.

Was it helpful?

Solution

If I wanted to do this, the first thing I'd do is kill your interface, and instead use this:

SomeAlgorithm(std::function<double(Point,Point)> distanceCalculator_)

type erased invocation object.

I could do a drop-in replacement using your EuclideanDistanceCalculator like this:

std::function<double(Point,Point)> UseEuclidean() {
  auto obj = std::make_shared<EuclideanDistance>();
  return [obj](Point a, Point b)->double {
    return obj->distance( a, b );
  };
}
SomeAlgorithm foo( UseEuclidean() );

but as distance calculators rarely require state, we could do away with the object.

With C++1y support, this shortens to:

std::function<double(Point,Point>> UseEuclidean() {
  return [obj = std::make_shared<EuclideanDistance>()](Point a, Point b)->double {
    return obj->distance( a, b );
  };
}

which as it no longer requires a local variable, can be used inline:

SomeAlgorithm foo( [obj = std::make_shared<EuclideanDistance>()](Point a, Point b)->double {
    return obj->distance( a, b );
  } );

but again, the EuclideanDistance doesn't have any real state, so instead we can just

std::function<double(Point,Point>> EuclideanDistance() {
  return [](Point a, Point b)->double {
    return sqrt( (b.x-a.x)*(b.x-a.x) + (b.y-a.y)*(b.y*a.y) );
  };
}

If we really don't need movement but we do need state, we can write a unique_function< R(Args...) > type that does not support non-move based assignment, and store one of those instead.

The core of this is that the interface DistanceCalculator is noise. The name of the variable is usually enough. std::function< double(Point,Point) > m_DistanceCalculator is clear in what it does. The creator of the type-erasure object std::function handles any lifetime management issues, we just store the function object by value.

If your actual dependency injection is more complicated (say multiple different related callbacks), using an interface isn't bad. If you want to avoid copy requirements, I'd go with this:

struct InterfaceForDependencyStuff {
  virtual void method1() = 0;
  virtual void method2() = 0;
  virtual int method3( double, char ) = 0;
  virtual ~InterfaceForDependencyStuff() {}; // optional if you want to do more work later, but probably worth it
};

then, write up your own make_unique<T>(Args&&...) (a std one is coming in C++1y), and use it like this:

Interface:

SomeAlgorithm(std::unique_ptr<InterfaceForDependencyStuff> pDependencyStuff)

Use:

SomeAlgorithm foo(std::make_unique<ImplementationForDependencyStuff>( blah blah blah ));

If you don't want virtual ~InterfaceForDependencyStuff() and want to use unique_ptr, you have to use a unique_ptr that stores its deleter (by passing in a stateful deleter).

On the other hand, if std::shared_ptr already comes with a make_shared, and it stores its deleter statefully by default. So if you go with shared_ptr storage of your interface, you get:

SomeAlgorithm(std::shared_ptr<InterfaceForDependencyStuff> pDependencyStuff)

and

SomeAlgorithm foo(std::make_shared<ImplementationForDependencyStuff>( blah blah blah ));

and make_shared will store a pointer-to-function that deletes ImplementationForDependencyStuff that will not be lost when you convert it to a std::shared_ptr<InterfaceForDependencyStuff>, so you can safely lack a virtual destructor in InterfaceForDependencyStuff. I personally would not bother, and leave virtual ~InterfaceForDependencyStuff there.

OTHER TIPS

In most cases you don't want or need ownership transfer, it makes code harder to understand and less flexible (moved-from objects can't be reused). The typical case would be to keep ownership with the caller:

class SomeAlgorithm {
    DistanceCalculator* distanceCalculator;

public:
    explicit SomeAlgorithm(DistanceCalculator* distanceCalculator_)
        : distanceCalculator(distanceCalculator_) {
        if (distanceCalculator == nullptr) { abort(); }
    }

    double calculateComplicated() {
        ...
        double dist = distanceCalculator->distance(p1, p2);
        ...
    }

    // Default special members are fine.
};

int main() {
    EuclideanDistanceCalculator distanceCalculator;
    SomeAlgorithm algorithm(&distanceCalculator);
    algorithm.calculateComplicated();
}

Raw pointers are fine to express non-ownership. If you prefer you can use a reference in the constructor argument, it makes no real difference. However, don't use a reference as data member, it makes the class unnecessarily unassignable.

The down side of just using any pointer (smart or raw), or even an ordinary C++ reference, is that they allow calling non-const methods from a const context.

For stateless classes with a single method that is a non-issue, and std::function is a good alternative, but for the general case of classes with state or multiple methods I propose a wrapper similar but not identical to std::reference_wrapper (which lacks the const safe accessor).

  template<typename T>
  struct NonOwningRef{
    NonOwningRef() = delete;
    NonOwningRef(T& other) noexcept : ptr(std::addressof(other)) { };
    NonOwningRef(const NonOwningRef& other) noexcept = default;

    const T& value() const noexcept{ return *ptr; };
    T& value() noexcept{ return *ptr; };

  private:
    T* ptr;
  };

usage:

   class SomeAlgorithm {
      NonOwningRef<DistanceCalculator> distanceCalculator;

   public:
      SomeAlgorithm(DistanceCalculator& distanceCalculator_)
        : distanceCalculator(distanceCalculator_) {}

      double calculateComplicated() {

         double dist = distanceCalculator.value().distance(p1, p2);
         return dist;
    }
};

Replace T* with unique_ptr or shared_ptr to get owning versions. In this case, also add move construction, and construction from any unique_ptr<T2> or shared_ptr<T2> ).

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top