Is property injection considered to be bad?

Question

Example solution to demonstrate the problem:

class World
{
    public override string ToString()
    {
        return "Hello World";
    }
}
class Hello
{
    [Inject]
    public World theWorld {  get;  set; }
    public Hello(IKernel kernel)
    {
        kernel.Inject(this);
    }
    public override string ToString()
    {
       return theWorld.ToString();
    }
}
class Program
{
    static IKernel kernel = new StandardKernel();
    static void RegisterServices()
    {
        kernel.Bind<World>().ToSelf();
    }
    static void Main(string[] args)
    {
        RegisterServices();
        Hello hello = new Hello(kernel);
        Console.WriteLine(hello.ToString());
        Console.ReadLine();
    }
}

This is the way I got property injection actually working.

It won`t work if:

1. Property is not public (or its setter).
2. The class that asks for injection doesnt get IKernel instance, to call kernel.Inject(this);.

For me its seems very excess and wrong to do this only to get instance of a property. Are there simpler ways or I haven't considered something?

Answer 1

Constructor Injection is typically a more favorable technique than Property injection, because Property Injection causes the Temporal Coupling code smell. Property Injection should, therefore, only be used for dependencies that are truly optional (which it isn't in your case). Dependencies, however, should hardly ever be optional. Even in case there is no implementation for a dependency, it's generally better to create and inject a Null Object implementation compared to injecting a null reference. Instead of using Property Injection, prefer injecting all required dependencies through the constructor.

Another practice that leads to considerable downsides is when you let your application code take a dependency on the DI Container itself (or an abstraction that represents the resolve-API of the container). This is an anti-pattern called Service Locator. The only place you should reference the container is inside your Composition Root. The Program class in your example represents the Composition Root.

Instead your Hello class should simply accept World as required constructor argument:

class Hello
{
    private readonly World theWorld;

    public Hello(World world)
    {
        this.theWorld = world ?? throw new ArgumentNullException("world");
    }

    public override string ToString()
    {
        return this.theWorld.ToString();
    }
}

Notice how any reference to the container was completely removed from this class. This makes the class simpler, more maintainable, more testable, and even makes it possible to compose this class without using a DI Container; a practice commonly known as Pure DI. When your application is small, Pure DI can even be a better option than using a container.

Here's how your Program class can look like using Ninject:

class Program
{
    static void Main(string[] args)
    {
        // Configure
        var kernel = new StandardKernel();
        kernel.Bind<Hello>().ToSelf();
        kernel.Bind<World>().ToSelf();

        // Resolve
        var hello = kernel.Get<Hello>();

        // Use
        Console.WriteLine(hello.ToString());
        Console.ReadLine();
    }
}

Without a container, it would be as follows:

class Program
{
    static void Main(string[] args)
    {
        // Resolve
        var hello = new Hello(new World());

        // Use
        Console.WriteLine(hello.ToString());
        Console.ReadLine();
    }
}