How many injections is acceptable in one class when using dependency injection

Question

I'm using Unity in C# for dependency injection, but the question should be applicable for any language and framework that is using dependency injection.

I'm trying to follow the SOLID-principle and therefore I got a lot of abstractions. But now I'm wondering if there's a best practice for how many injections one class should inject?

For example, I have a repository with 9 injections. Will this be hard to read for another developer?

The injections have the following responsibilities:

• IDbContextFactory - creating the context for the database.
• IMapper - Mapping from entities to domain models.
• IClock - Abstracts DateTime.Now to help with unit tests.
• IPerformanceFactory - measures the execution time for specific methods.
• ILog - Log4net for logging.
• ICollectionWrapperFactory - Creates collections (that extends IEnumerable).
• IQueryFilterFactory - Generates queries based on input that will query db.
• IIdentityHelper - Retrieves the logged in user.
• IFaultFactory - Create different FaultExceptions (I use WCF).

I'm not really dissapointed with how I delegated the responsibilities, but I'm starting to get worried for the readability.

So, my questions:

Is there a limit for how many injections a class should have? And if so, how to avoid it?

Does many injections limit readability, or does it actually improve it?

Answer 1

Too much dependencies may indicate that the class itself is doing too much. In order to determine if it's doing too much:

• Look at the class itself. Would it make sense to split it in two, three, four? Does it make sense as a whole?

• Look at the types of dependencies. Which ones are domain-specific, and which ones are “global”? For instance, I wouldn't consider ILog at the same level as IQueryFilterFactory: the first one would be available in most business classes anyway if they are using logging. On the other hand, if you find a lot of domain-specific dependencies, this may indicate that the class is doing too much.

• Look at the dependencies which can be replaced by values.

  IClock - Abstracts DateTime.Now to help with unit tests.

This could easily be replaced by DateTime.Now being passed directly to the methods which require to know the current time.

By looking at the actual dependencies, I don't see anything which would be indicative of bad things happening:

• IDbContextFactory - creating the context for the database.

  OK, we are probably inside a business layer where classes interact with data access layer. Looks fine.

• IMapper - Mapping from entities to domain models.

  It's difficult to tell anything without the overall picture. It might be that the architecture is wrong and the mapping should be done directly by the data access layer, or it may be that the architecture is perfectly fine. In all cases, it makes sense to have this dependency here.

  Another choice would be to split the class in two: one dealing with mapping, the other one dealing with the actual business logic. This would create a de facto layer which will separate further the BL from the DAL. If mappings are complex, it could be a good idea. In most cases, though, it would just add useless complexity.

• IClock - Abstracts DateTime.Now to help with unit tests.

  It is probably not very useful to have a separate interface (and class) just to get the current time. I would simply pass the DateTime.Now to the methods which require the current time.

  A separate class may make sense if there is some other information, like the timezones, or the date ranges, etc.

• IPerformanceFactory - measures the execution time for specific methods.

  See the next point.

• ILog - Log4net for logging.

  Such transcendent functionality should belong to the framework, and the actual libraries should be interchangeable and configurable at runtime (for instance through app.config in .NET).

  Unfortunately, this is not (yet) the case, which let you either pick a library and stick with it, or create an abstraction layer to be able to swap the libraries later if needed. If your intention is specifically to be independent of the choice of the library, go for it. If you're pretty sure that you'll continue using the library for years, don't add an abstraction.

  If the library is too complex to use, a facade pattern makes sense.

• ICollectionWrapperFactory - Creates collections (that extends IEnumerable).

  I would assume that this creates very specific data structures which are used by the domain logic. It looks like a utility class. Instead, use one class per data structure with relevant constructors. If the initialization logic is slightly complicated to fit in a constructor, use static factory methods. If the logic is even more complex, use factory or a builder pattern.

• IQueryFilterFactory - Generates queries based on input that will query db.

  Why isn't that in the data access layer? Why is there a Filter in the name?

• IIdentityHelper - Retrieves the logged in user.

  I'm not sure why is there a Helper suffix. In all cases, other suffixes won't be particularly explicit either (IIdentityManager?)

  Anyway, it makes perfect sense to have this dependency here.

• IFaultFactory - Create different FaultExceptions (I use WCF).

  Is the logic so complex that it requires to use a factory pattern? Why is Dependency Injection used for that? Would you swap the creation of exceptions between production code and tests? Why?

  I would try to refactor that into simple throw new FaultException(...). If some global information should be added to all the exceptions before propagating them to the client, WCF probably has a mechanism where you catch an unhandled exception and can change it and re-throw it to the client.

Is there a limit for how many injections a class should have? And if so, how to avoid it?

Measuring the quality by numbers is usually as bad as being paid by lines of code you write per month. You may have a high number of dependencies in a well-designed class, as you can have a crappy class using few dependencies.

Does many injections limit readability, or does it actually improve it?

Lots of dependencies make the logic more difficult to follow. If the logic is difficult to follow, the class is probably doing too much and should be split.

Answer 2

This is a classic example of DI telling you that your class is probably becoming too big to be a single class. This is often interpreted as "wow, DI makes my constructors larger than jupiter, this technique is horrible", but what it ACTUALLY tells you is that "your class has a boat-load of dependencies". Knowing this, we can either

• Sweep the issue under the rug by starting to new up dependencies instead
• Reconsider our design. Maybe some dependencies always come together and should be hidden behind some other abstraction. Maybe your class should be split into 2. Maybe it should be composed by several classes that each require a small subset of the dependencies.

There are countless ways of managing dependencies, and it's impossible to say what works best in your case without knowing your code and your application.

To answer your final questions:

• Is there an upper limit to how many dependencies a class should have?

Yes, the upper limit is "too many". How many is "too many"? "Too many" is when the cohesion of the class gets "too low". It all depends. Normally if your reaction to a class is "wow, this thing has a lot of dependencies", that's too much.

• Does injecting dependencies improve or hurt readability?

I think this question is misleading. The answer can be yes or no. But it's not the most interesting part. The point of injecting dependencies is to make them VISIBLE. It's about making an api that doesn't lie. It's about preventing global state. It's about making code testable. It's about reducing coupling.

Well designed classes with reasonably designed and named methods are more readable than badly designed ones. DI doesn't really improve nor hurt readability per se, it just makes your design choices stand out, and if they are bad it will sting your eyes. This doesn't mean DI made your code less readable, it just showed you that your code was already a mess, you had just hidden it.

Answer 3

According to Steve McConnel, author of the ubiquitous "Code Complete", the rule of thumb is that more than 7 is a code smell that hurts maintainability. I personally think the number is lower in most cases, but doing DI properly will result in having many dependencies to inject when you're very close to the Composition Root. This is normal and expected. It's one of the reasons that IoC containers are a useful thing and worth the complexity they add to a project.

So, if you're very close to the entry point of your program, this is normal and acceptable. If you're deeper into the logic of the program, it's likely a smell that should be addressed.