Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a simple question, and I'm not even sure it has an answer but let's try. I'm coding in C++, and using dependancy injection to avoid global state. This works quite well, and I don't run in unexpected/undefined behaviours very often.

However I realise that, as my project grows I'm writing a lot of code which I consider boilerplate. Worse : the fact there is more boilerplate code, than actual code makes it sometimes hard to understand.

Nothing beats a good example so let's go :

I have a class called TimeFactory which creates Time objects.

For more details (not sure it's relevant) : Time objects are quite complex because the Time can have different formats, and conversion between them is neither linear, nor straightforward. Each "Time" contains a Synchronizer to handle conversions, and to make sure they have the same, properly initialized, synchronizer, I use a TimeFactory. The TimeFactory has only one instance and is application wide, so it would qualify for singleton but, because it's mutable, I don't want to make it a singleton

In my app, a lot of classes need to create Time objects. Sometimes those classes are deeply nested.

Let's say I have a class A which contains instances of class B, and so on up to class D. Class D need to create Time objects.

In my naive implementation, I pass the TimeFactory to the constructor of class A, which passes it to the constructor of class B and so on until class D.

Now, imagine I have a couple of classes like TimeFactory and a couple of class hierarchies like the one above : I loose all the flexibility and readability I'm suppose to get using dependancy injection .

I'm starting to wonder if there isn't a major design flaw in my app ... Or is this a necessary evil of using dependancy injection ?

What do you think ?

share|improve this question
4  
I posted this question to programmers instead of stackoverflow because, as I understood, programmers is more for design related questons and stackoverflow is for "when you're in front of your compiler"-questions . Sorry in advance if I'm wrong. –  Dinaiz Aug 5 '12 at 3:48
2  
aspect oriented programming, is also good for this tasks - en.wikipedia.org/wiki/Aspect-oriented_programming –  Yusubov Aug 5 '12 at 5:32
    
Is class A factory for classes B,C and D? If not why does it create instances of them? If only class D needs Time-objects, why would you inject any other class with TimeFactory? Does class D really need TimeFactory, or could just an instance of Time be injected to it? –  simoraman Aug 5 '12 at 9:59
    
D needs to creates Time objects, with information only D is supposed to have. I have to think about the rest of your comment. There might be a problem in "who's creating who" in my code –  Dinaiz Aug 6 '12 at 0:12
    
"who's creating who" is solved by IoC containers, see my answer for detail.. "who's creating who" is one of the best question you can ask when using Dependency Injection. –  DarioOO Jul 21 '13 at 11:40
add comment

5 Answers 5

up vote 7 down vote accepted

In my app, a lot of classes need to create Time objects

Seems that your Time class is a very basic data type which should belong to the "general infrastructure" of your application. DI does not work well for such classes. Think about what it means if a class like string had to be injected into every part of the code which uses strings, and you would need to use a stringFactory as the only possibilty of creating new strings - the readability of your program would decrease by an order of magnitude.

So my suggestion: don't use DI for general datatypes like Time. Write unit tests for the Time class itself, and when its done, use it everywhere in your program and your unit tests just like the string class, or a vector class or any other class of the standard lib. Use DI for components which should be really decoupled one from each other.

share|improve this answer
    
You got it completely right. "the readability of your program would decrease by an order of magnitude." : that's exactly what happened yes. Provided I still want to use a factory, it's not THAT bad to make it a singleton then ? –  Dinaiz Aug 6 '12 at 0:03
1  
@Dinaiz: the idea is to accept tight coupling between Time and the other parts of your program. So you could accept also accept tight coupling to TimeFactory. What I would avoid, however, is having a single global TimeFactory object with a state (for example, a locale information or something like that) - that could cause nasty side-effects to your program and make general testing and re-use very hard. Either make it stateless, or don't use it as a singleton. –  Doc Brown Aug 6 '12 at 9:32
    
Actually it has to have a state, so when you say "Don't use it as a singleton", you mean "keep using dependancy injection, i.e passing the instance to every object which needs it", right ? –  Dinaiz Aug 6 '12 at 16:37
1  
@Dinaiz: honestly, that depends - on the kind of state, on the kind and number of parts of your program using Timeand TimeFactory, on the degree of "evolvability" you need for future extensions related to TimeFactory, and so on. –  Doc Brown Aug 6 '12 at 18:26
    
OK I'll try to keep dependancy inject but have a more clever design which doesn't require so much boilerplate then ! thanks a lot doc' –  Dinaiz Aug 7 '12 at 0:12
add comment

What do you mean by "I loose all the flexibility and readability I'm suppose to get using dependency injection" - DI isn't about readability. It's about decoupling the dependency between objects.

It sounds like you have Class A creating Class B, Class B creating Class C and Class C creating Class D.

What you should have is Class B injected in to Class A. Class C injected in to Class B. Class D injected in to Class C.

share|improve this answer
    
DI can be about readbility. If you have a global variable "magically" appearing in the middle of a method, it doesn't help to understand the code (from a maintenance point of view). Where is this variable from ? Who owns it ? Who initializes it ? And so on ... For your second point, you're probably right but I don't think it applies in my case. Thank you for taking the time to reply –  Dinaiz Aug 6 '12 at 0:09
    
DI increase readibility mostly because "new/delete" will be magically stripped out of your code. If you don't have to warry about dynamic allocation code is easier to read. –  DarioOO Jul 21 '13 at 10:15
    
Also forget to say that this make code a bit stronger because cannot longer make assumptions about "how" a dependency is created, it "just need it". And that makes the class easier to maintain.. see my answer –  DarioOO Jul 21 '13 at 12:08
add comment

I'm not sure why you don't want to make your time factory a singleton. If there's only one instance of it in your whole app, it is de facto a singleton.

That being said, it is very dangerous to share a mutable object, except if it's properly guarded by synchronize blocks, in which case, there's no reason for it not to be a singleton.

If you want to do dependency injection, you might want to look at spring or other dependency injection frameworks, which would allow you to auto assign parameters using an annotation

share|improve this answer
    
Spring is for java and I use C++. But why did 2 people downvote you ? There are points in your post I disagree with but still ... –  Dinaiz Aug 6 '12 at 0:05
    
I'm going to assume the downvote was due not answering the question per se, perhaps as well as the mention of Spring. However, the suggestion of using a Singleton was a valid one in my mind, and may not answer the question - but suggests a valid alternative and brings up an interesting design issue. For this reason I've +1. –  Fergus Morrow Jul 21 '13 at 20:16
add comment

Do your A, B and C classes also need to create Time instances, or only class D? If it's only class D, then A and B should know nothing about TimeFactory. Create an instance of TimeFactory inside C class, and pass it to class D. Note that by "create an instance" I don't necessarily mean that the C class has to be responsible for instantiating the TimeFactory. It can receive DClassFactory from class B, and DClassFactory knows how to create Time instance.

A technique that I also often use when I don't have any DI framework is providing two constructors, one which accepts a factory, and another one which creates a default factory. The second one has usually a protected/package access, and is used mainly for unit tests.

share|improve this answer
add comment

This aims to be a complementary answer to Doc Brown, and also to respond to unanswered comments of Dinaiz that are still related to the Question.

What you probably need is a framework for doing DI. Having complex hierarchies does not necessarily means bad design, but if you have to inject a TimeFactory bottom-up (from A to D) instead of injecting directly to D then probably there's something wrong with the way you are doing Dependency Injection.

A singleton? No thanks. If you need only one istance make it shared across your application context (Using a IoC container for DI like Infector++ requires just to bind TimeFactory as single istance), here's the example (C++11 by the way, but so C++. Maybe move to C++11 already? You get Leak-Free application for free):

Infector::Container ioc; //your app's context

ioc.bindSingleAsNothing<TimeFactory>(); //declare TimeFactory to be shared
ioc.wire<TimeFactory>(); //wire its constructor 

// if you want to be sure TimeFactory is created at startup just request it
// (else it will be created lazily only when needed)
auto myTimeFactory = ioc.buildSingle<TimeFactory>();

Now the good point of a IoC container is that you don't need to pass time factory up to D. if your class "D" need time factory, just put time factory as constructor parameter for class D.

ioc.bindAsNothing<A>(); //declare class A
ioc.bindAsNothing<B>(); //declare class B
ioc.bindAsNothing<D>(); //declare class D

//constructors setup
ioc.wire<D, TimeFactory>(); //time factory injected to class D
ioc.wire<B, D>(); //class D injected to class B
ioc.wire<A, B>(); //class B injected to class A

as you see you inject TimeFactory only once. How to use "A"? Very simple, every class is injected, builded in the main or istantiated with a factory.

auto myA1 = ioc.build<A>(); //A is not "single" so many different istances
auto myA2 = ioc.build<A>(); //can live at same time

every time you create class A it will be automatically (lazy istantiation) injected with all dependencies up to D and D will be injected with TimeFactory, so by calling only 1 method you have your complete hierarchy ready (and even complex hierachies are solved this way removing A LOT of boiler plate code): You don't have to call "new/delete" and that's very important because you can separate application logic from glue code.

D can create Time objects with informations that only D can have

That's easy, your TimeFactory have a "create" method, then just use a different signature "create(params)" and you are done. Parameters that are no dependencies are often resolved this way. This also remove the duty of injecting things like "strings" or "integers" because that just add extra boiler plate.

Who creates who? IoC container creates istances and factories, the factories creates the rest (factories can create different objects with arbitrary parameters, so you don't really need a state for factories). You can still use the factories as wrappers for the IoC Container: generally speaking Injectin the IoC Container is very bad and is the same of using a service locator. Some people solved the problem by wrapping the IoC Container with a factory (this is not strictly necessary, but has the advantage that the hierarchy is solved by the Container and all your factories becomes even easier to maintain).

//factory method
std::unique_ptr<myType> create(params){
    auto istance = ioc->build<myType>(); //this code's agnostic to "myType" hierarchy
    istance->setParams(params); //the customization you needed
    return std::move(istance); 
}

Also don't abuse dependency injection, simple types can just be class' members or local scoped variables. This seem obvious but I saw people injecting "std::vector" just because there was a DI framework that allowed that. Always remember Demeter's law: "Inject only what you really need to inject"

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.