Using reference as class members for dependencies

Question

I am going back to C++ after spending some time in memory-managed languages, and I'm suddently kinda lost as to what is the best way to implement dependency injection. (I am completely sold to DI because I found it to be the simplest way to make test-driven design very easy).

Now, browsing SO and google got me quite a number of opinions on the matter, and I'm a bit confused.

As an answer to this question, Dependency injection in C++ , someone suggested that you should not pass raw pointers around, even for dependency injection. I understand it is related to ownership of the objects.

Now, ownership of objects is also tackled (although not into enough details to my state ;) ) in the infamous google style guide : http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Smart_Pointers

So what I understand is that in order to make it clearer which object has ownership of which other objects, you should avoid passing raw pointers around. In particular, it seems to be against this kind of coding :

class Addict {
   // Something I depend on (hence, the Addict name. sorry.)
   Dependency * dependency_;
public:
   Addict(Dependency * dependency) : dependency_(dependency) {
   }
   ~Addict() {
     // Do NOT release dependency_, since it was injected and you don't own it !
   }
   void some_method() {
     dependency_->do_something();
   }
   // ... whatever ... 
};    

If Dependency is a pure virtual class (aka poor-man's-Interface ), then this code makes it easy to inject a mock version of the Dependency (using something like google mock).

The problem is, I don't really see the troubles I can get in with this kind of code, and why I should want to use anything else than raw pointers ! Is it that it is not clear where the dependency comes from?

Also, I read quite a few posts hinting that one should really be using references in this situation, so is this kind of code better?

class Addict {
   // Something I depend on (hence, the Addict name. sorry.)
   const Dependency & dependency_;
  public:
   Addict(const Dependency & dependency) : dependency_(dependency) {
   }
   ~Addict() {
     // Do NOT release dependency_, since it was injected and you don't own it !
   }
   void some_method() {
     dependency_.do_something();
   }
   // ... whatever ... 
};

But then I get other, equally authoritive advices against using references as member: http://billharlan.com/pub/papers/Managing_Cpp_Objects.html

As you can see I am not exactly sure about the relative pros and cons of the various approaches, so I am a bit confused. I am sorry if this has been discussed to death, or if it is only a matter of personnal choice and consistency inside a given project ... but any idea is welcome.

---

Answers summary

(I don't know if it is good SO-tiquette to do this, but I'll add code example for what I gathered from answers...)

From the various responses, here's what I'll probably end up doing in my case:

• pass dependencies as reference (at least to make sure NULL is not possible)
• in the general case where copying is not possible, explicitly disallow it, and store dependencies as reference
• in the rarer case where copying is possible, store dependencies as RAW pointers
• let the creator of the dependencies (factory of some kind) decide between stack allocation or dynamic allocation (and in the latter case, management through a smart pointer)
• establish a convention to separate dependencies from own resources

So I would end up with something like:

class NonCopyableAddict {
    Dependency & dep_dependency_;

    // Prevent copying
    NonCopyableAddict & operator = (const NonCopyableAddict & other) {}
    NonCopyableAddict(const NonCopyableAddict & other) {}

public:
    NonCopyableAddict(Dependency & dependency) : dep_dependency_(dep_dependency) {
    }
    ~NonCopyableAddict() {
      // No risk to try and delete the reference to dep_dependency_ ;)
    }
    //...
    void do_some_stuff() {
      dep_dependency_.some_function();
    }
};

And for a copyable class:

class CopyableAddict {
    Dependency * dep_dependency_;

public: 
    // Prevent copying
    CopyableAddict & operator = (const CopyableAddict & other) {
       // Do whatever makes sense ... or let the default operator work ? 
    }
    CopyableAddict(const CopyableAddict & other) {
       // Do whatever makes sense ...
    }


    CopyableAddict(Dependency & dependency) : dep_dependency_(&dep_dependency) {
    }
    ~CopyableAddict() {
      // You might be tempted to delete the pointer, but its name starts with dep_, 
      // so by convention you know it is not your job
    }
    //...
    void do_some_stuff() {
      dep_dependency_->some_function();
    }
};

From what I understood, there is no way to express the intent of "I have a pointer to some stuff, but I don't own it" that the compiler can enforce. So I'll have to resort to naming convention here...

---

Kept for reference

As pointed out by Martin, the following example does not solve the problem.

Or, assuming I have a copy constructor, something like:

class Addict {
   Dependency dependency_;
  public:
   Addict(const Dependency & dependency) : dependency_(dependency) {
   }
   ~Addict() {
     // Do NOT release dependency_, since it was injected and you don't own it !
   }
   void some_method() {
     dependency_.do_something();
   }
   // ... whatever ... 
};

Answer 1

Summary: If you need to store a reference, store a pointer as a private variable and access it through a method which dereferences it. You can stick a check that the pointer isn't null in the object's invariant.

In depth:

Firstly, storing references in classes makes it impossible to implement a sensible and legal copy constructor or assignment operator, so they should be avoided. It is usually a mistake to use one.

Secondly, the type of pointer/reference passed in to functions and constructors should indicate who has responsibility for freeing the object and how it should be freed:

• std::auto_ptr - the called function is responsible for freeing, and will do so automatically when it's done. If you need copy semantics, the interface must provide a clone method which should return an auto_ptr.

• std::shared_ptr - the called function is responsible for freeing, and will do so automatically when it's done and when all other references to the object are gone. If you need shallow copy semantics the compiler generated functions will be fine, if you need deep copying the interface must provide a clone method which should return a shared_ptr.

• A reference - the caller has responsibility. You don't care - the object may be stack allocated for all you know. In this case you should pass by reference but store by pointer. If you need shallow copy semantics the compiler generated functions will be fine, if you need deep copying you're in trouble.

• A raw pointer. Who knows? Could be allocated anywhere. Could be null. You might be responsible for freeing it, you might not.

• Any other smart pointer - it should manage the lifetime for you, but you'll need to look at the documentation to see what the requirements are for copying.

Note that the methods which give you responsibility for freeing the object don't break DI - freeing the object is simply a part of the contract you have with the interface (as you don't need to know anything about the concrete type to free it).

Answer 2

I can here my downmoderation coming already, but I will say that there should be no reference members in a class FOR ANY REASO, EVER. Except if they are a simple constant value. The reasons for this are many, the second you start this you open up all the bad things in C++. See my blog if you really care.

Answer 3

There is no hard and fast rule:
As people have mentioned using references inside objects can cause copy problems (and it does) so it is not a panacea, but for certain situation it can be useful (that is why C++ gives us the option to do it all these different ways). But using RAW pointers is really not an option. If you are dynamically allocating objects then you should always be maintaining them with smart pointers and your object should also be using smart pointers.

For people who demand examples: Streams are always passed and stored as references (as they can't be copied).

Some Comments on your code examples:

Example one and two

Your first example with pointers. Is basically the same as the second example using references. The difference being that a reference can not be NULL. When you pass a reference the object is already alive and thus should have a lifespan greater than the object you are testing already (If it was created on the stack) so it should be safe to keep a reference. If you are dynamically creating pointers as dependencies I would consider using boost::shared_pointer or std::auto_ptr depending if ownership of the dependency is shared or not.

Example Three:

I don't see any great use for your third example. This is because you can not use polymorphic types (If you pass an object derived from Dependency it will be sliced during the copy operation). Thus the code may as well be inside Addict rather than a separate class.

Bill Harlen: (http://billharlan.com/pub/papers/Managing%5FCpp%5FObjects.html)

Not to take anything away from Bill But:

1. I have never heard of him.
   • He is a Geo-Physists not a computer programmer
   • He recomends programming in Java to improve your C++
   • The languages are now so different in usage that is utterly false).
   • If you want to use references of What to-do/not to-do.
     Then I would pick one of the Big names in the C++ field:
     Stroustrup/Sutter/Alexandrescu/Meyers

Summary:

1. Don't use RAW pointers (when ownership is required)
2. Do use smart pointers.
3. Don't copy objects into your object (it will slice).
4. You can use references (but know the limitations).

My example of Dependency injection using references:

class Lexer
{
    public: Lexer(std::istream& input,std::ostream& errors);
    ... STUFF
    private:
       std::istream&  m_input;
       std::ostream&  m_errors;
};
class Parser
{
    public: Parser(Lexer& lexer);
    ..... STUFF
    private:
        Lexer&        m_lexer;
};

int main()
{
     CLexer  lexer(std::cin,std::cout);  // CLexer derived from Lexer
     CParser parser(lexer);              // CParser derived from Parser

     parser.parse();
}

// In test.cpp
int main()
{
     std::stringstream  testData("XXXXXX");
     std::stringstream  output;
     XLexer  lexer(testData,output);
     XParser parser(lexer);

     parser.parse();
}

Answer 4

[update 1]
If you can always guarantee the dependency outlives the addict, you can use a raw pointer/reference, of course. between these two, I'd make a very simple decision: pointer if NULL is allowed, reference otherwise.

(The point of my original post was that neither pointer nor reference solve the lifetime problem)

---

I'd follow the infamous google style guideline here, and use smart pointers.

Both a pointer and a reference have the same problem: you need to make sure the dependency outlives the addict. That pushes a quite nasty responsibility onto the client.

With a (reference-counted) smart pointer, the policy becomes dependency is destroyed when noone uses it anymore. Sounds perfect to me.

Even better: with boost::shared_ptr (or a similar smart pointer that allows a type-neutral destruction policy) the policy is attached to the object at construction - which usually means everything affecting the dependency ends up in a single place.

The typical problems of smart pointers - overhead and circular references - rarely come into play here. Dependency instances usually aren't tiny and numerous, and a dependency that has a strong reference back to its addicts is at least a code smell. (still, you need to keep these things in mind. Welcome back to C++)

Warning: I am not "totally sold" to DI, but I'm totally sold on smart pointers ;)

[update 2]
Note that you can always create a shared_ptr to a stack/global instance using a null deleter.
This requires both sides to support this, though: addict must make guarantees that it will not transfer a reference to the dependency to someone else who might live longer, and caller is back with the responsibility ensuring lifetime. I am not to happy with this solution, but have used this on occasion.

Answer 5

It has been asked before, but my SO search skills are not up to finding it. To summarise my position - you should very rarely, if ever, use references as class members. Doing so causes all sorts of initialisation, assignment and copying problems. Instead, use a pointer or a value.

Edit: Found one - this is a question with a variety of opinions as answers: Should I prefer pointers or references in member data?

Answer 6

I would steer clear of references as members since they tend to cause no end of headaches if you end up sticking one of your objects in an STL container. I would look into using a combination of boost::shared_ptr for ownership and boost::weak_ptr for dependents.

Answer 7

But then I get other, equally authoritive advices against using references as member : http://billharlan.com/pub/papers/Managing%5FCpp%5FObjects.html

In this case I think you only want to set the object once, in the constructor, and never change it so no problem. But if you want to change it later on, use an init function, have a copy constructor, in short everything that would have to change the reference you will have to use pointers.