Dependency Injection – Acceptable Number of Injections in One Class

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.