How to avoid Dependency Injection constructor madness?

Question

I find that my constructors are starting to look like this:

public MyClass(Container con, SomeClass1 obj1, SomeClass2, obj2.... )

with ever increasing parameter list. Since "Container" is my dependency injection container, why can't I just do this:

public MyClass(Container con)

for every class? What are the downsides? If I do this, it feels like I'm using a glorified static. Please share your thoughts on IoC and Dependency Injection madness.

Answer 1

Too many parameters for a method is a hint (not necessarily) that your method is too big / has too many responsibilites.

Furthermore, parameters that are used in the same method (may) have a high cohesion -> it is worth considering putting them into one class -> only one parameter must be passed.

Answer 2

You are right that if you use the container as a Service Locator, it's more or less a glorified static factory. For lots of reasons I consider this an anti-pattern (also see this excerpt from my book).

One of the wonderful benefits of Constructor Injection is that it makes violations of the Single Responsibility Principle glaringly obvious.

When that happens, it's time to refactor to Facade Services. In short, create a new, more coarse-grained interface that hides the interaction between some or all of the fine-grained dependencies you currently require.

Answer 3

Injecting the container is a shortcut that you will ultimately regret.

Over injection is not the problem, it is usually a symptom of other structural flaws, most notably separation of concerns. This is not one problem but can have many sources and what makes this so difficult to fix is that you are going to have to deal with all of them, sometimes at the same time (think of untangling spaghetti).

Here is an incomplete list of the things to look out for

Poor Domain Design (Aggregate root’s …. etc)

Poor separation of concerns (Service composition, Commands, queries) See CQRS and Event Sourcing.

OR Mappers (be careful, these things can lead you into trouble)

View Models and other DTO’s (Never reuse one, and try to keep them to a minimal !!!!)

Answer 4

I read this whole thread, twice, and I think people are responding by what they know, not by what is asked.

JP's original question looks like he's constructing objects by sending a resolver, and then a bunch of classes, but we're assuming that those classes/objects are themselves services, ripe for injection. What if they are not?

JP, if you're looking to leverage DI and desire the glory of mixing injection with contextual data, none of these patterns (or supposed "anti-patterns") specifically address that. It actually boils down to using a package which will support you in such an endeavor.

Container.GetSevice<MyClass>(someObject1, someObject2)

... this format is rarely supported. I believe the difficulty of programming such support, added to the miserable performance that would be associated with the implementation, makes it unattractive for opensource developers.

But it should be done, because I should be able to create and register a factory for MyClass'es, and that factory should be able to receive data/input that aren't pushed into being a "service" just for the sake of passing data. If "anti-pattern" is about negative consequences, then forcing the existence of artificial service types for passing data/models is certainly negative (on par with your feeling about wrapping up your classes into a container. Same instinct applies).

There are framework that may help, though, even if they look a bit ugly. For example, Ninject:

Creating an instance using Ninject with additional parameters in the constructor

That's for .NET, is popular, and is still nowhere as clean as it should be, but I'm sure there's something in whatever language you choose to employ.

Answer 5

This is the approach I use

public class Hero
{

    [Inject]
    private IInventory Inventory { get; set; }

    [Inject]
    private IArmour Armour { get; set; }

    [Inject]
    protected IWeapon Weapon { get; set; }

    [Inject]
    private IAction Jump { get; set; }

    [Inject]
    private IInstanceProvider InstanceProvider { get; set; }


}

Here is a crude approach how to perform the injections and run constructor after injecting values. This is fully functional program.

public class InjectAttribute : Attribute
{

}


public class TestClass
{
    [Inject]
    private SomeDependency sd { get; set; }

    public TestClass()
    {
        Console.WriteLine("ctor");
        Console.WriteLine(sd);
    }
}

public class SomeDependency
{

}


class Program
{
    static void Main(string[] args)
    {
        object tc = FormatterServices.GetUninitializedObject(typeof(TestClass));

        // Get all properties with inject tag
        List<PropertyInfo> pi = typeof(TestClass)
            .GetProperties(BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Public)
            .Where(info => info.GetCustomAttributes(typeof(InjectAttribute), false).Length > 0).ToList();

        // We now happen to know there's only one dependency so we take a shortcut just for the sake of this example and just set value to it without inspecting it
        pi[0].SetValue(tc, new SomeDependency(), null);


        // Find the right constructor and Invoke it. 
        ConstructorInfo ci = typeof(TestClass).GetConstructors()[0];
        ci.Invoke(tc, null);

    }
}

I am currently working on a hobby project which works like this https://github.com/Jokine/ToolProject/tree/Core

Answer 6

Problem :

1) Constructor with ever increasing parameter list.

2) If class is inherited (Ex: RepositoryBase) then changing constructor signature causes change in the derived classes.

Solution 1

Pass IoC Container to constructor

Why

• No more ever increasing parameter list
• Constructor's signature becomes simple

Why not

• Makes you class tightly coupled to IoC container. (That causes problems when 1. you want to use that class in other projects where you use different IoC container. 2. you decide to change IoC container)
• Makes you class less descriptive. (You can't really look at class constructor and say what it needs for functioning.)
• Class can access potentially all service.

Solution 2

Create a class which groups all service and pass it to constructor

 public abstract class EFRepositoryBase 
 {
    public class Dependency
    {
        public DbContext DbContext { get; }
        public IAuditFactory AuditFactory { get; }

         public Dependency(
            DbContext dbContext,
            IAuditFactory auditFactory)
        {
            DbContext = dbContext;
            AuditFactory = auditFactory;
        }
    }

    protected readonly DbContext DbContext;        
    protected readonly IJobariaAuditFactory auditFactory;

    protected EFRepositoryBase(Dependency dependency)
    {
        DbContext = dependency.DbContext;
        auditFactory= dependency.JobariaAuditFactory;
    }
  }

Derived class

  public class ApplicationEfRepository : EFRepositoryBase      
  {
     public new class Dependency : EFRepositoryBase.Dependency
     {
         public IConcreteDependency ConcreteDependency { get; }

         public Dependency(
            DbContext dbContext,
            IAuditFactory auditFactory,
            IConcreteDependency concreteDependency)
        {
            DbContext = dbContext;
            AuditFactory = auditFactory;
            ConcreteDependency = concreteDependency;
        }
     }

      IConcreteDependency _concreteDependency;

      public ApplicationEfRepository(
          Dependency dependency)
          : base(dependency)
      { 
        _concreteDependency = dependency.ConcreteDependency;
      }
   }

Why

• Adding new dependency to class does not affect derived classes
• Class is agnostic of IoC Container
• Class is descriptive (in aspect of its dependencies). By convention, if you want to know what class A Depends on, that information is accumulated in A.Dependency
• Constructor signature becomes simple

Why not

• need to create additional class
• service registration becomes complex (You need to register every X.Dependency separately)
• Conceptually same as passing IoC Container
• ..

Solution 2 is just a raw though, if there is solid argument against it, then descriptive comment would be appreciated

Answer 7

I came across a similar question about constructor based dependency Injection and how complex it was getting to pass in all the dependencies.

One of the approach, I have used in past is to use the application facade pattern using a service layer. This would have a coarse API. If this service depends on repositories, It would use a setter injection of the private properties. This requires creating an abstract factory and moving the logic of creating the repositories into a factory.

Detailed code with explanation can be found here

Best practices for IoC in complex service layer

Answer 8

What dependency injection framework are you using? Have you tried using setter based injection instead?

The benefit for constructor based injection is that it looks natural for Java programmers who don't use DI frameworks. You need 5 things to initialize a class then you have 5 arguments for your constructor. The downside is what you have noticed, it gets unwieldy when you have lots of dependencies.

With Spring you could pass the required values with setters instead and you could use @required annotations to enforce that they are injected. The downside is that you need to move the initialization code from the constructor to another method and have Spring call that after all the dependencies are injected by marking it with @PostConstruct. I'm not sure about other frameworks but I assume they do something similar.

Both ways work, its a matter of preference.

Answer 9

The difficulty of passing in the parameters is not the problem. The problem is that your class is doing too much, and should be broken down more.

Dependency Injection can act as an early warning for classes getting too big, specifically because of the increasing pain of passing in all of the dependencies.

Answer 10

I don't think your class constructors should have a reference to your IOC container period. This represents an unnecessary dependency between your class and the container (the type of dependency IOC is trying to avoid!).