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 ...
};
See Question&Answers more detail:os