simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 155 forks source link

C# 11 required property support #969

Closed mikernet closed 8 months ago

mikernet commented 1 year ago

Now that C# 11 has required properties, are there plans for SimpleInjector to support auto-injecting those?

dotnetjunkie commented 1 year ago

Can you provide me a link to some C# docs about this topic?

mikernet commented 1 year ago

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/required

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-11.0/required-members

mikernet commented 1 year ago

The TLDR is that a property marked with required must be initialized in the object initializer:

public class SomeService
{
    public required IEmailService EmailService { get; init; }
}

var s = new SomeService(); // error

var s = new SomeService () {
    EmailService = new EmaiService(); // ok
};
dotnetjunkie commented 1 year ago

Ah, okay. I see the use case, and it seems to me more useful for instantiating data-centric objects, rather than DI components. What would be the advantage of using required over constructor injection?

mikernet commented 1 year ago

Changes to a base class's dependencies don't require you to update all the constructors of all subclasses. Would be very useful for some types of desktop app scenarios that use DI to build view models with large inheritance structures.

mikernet commented 1 year ago

You no longer need to chain all the dependencies into base classes via constructor, basically, so changes to a base class's dependencies don't require you to update all the sub classes accordingly.

mikernet commented 1 year ago

FWIW here's an Autofac discussion around the feature: https://github.com/autofac/Autofac/issues/1355

GeraldLx commented 1 year ago

The feature is already in SimpleInjector:

When you have this:

public interface IEmailService
{
  void Send();
}

public sealed class EmailService : IEmailService
{
  void IEmailService.Send()
  {
    Console.WriteLine("Sending mail.");
  }
}

public class SomeService
{
  public required IEmailService EmailService { get; init; }

  public void Foo()
  {
    EmailService.Send();
  }
}

You can write code like this:

public static class Program
{
  public static void Main()
  {
    using var container = new Container();
    container.Options.PropertySelectionBehavior = new RequiredPropertySelectionBehavior();

    container.Register<IEmailService, EmailService>();
    container.Register<SomeService>();

    container.Verify();

    var service = container.GetInstance<SomeService>();
    service.Foo();
  }
}

public sealed class RequiredPropertySelectionBehavior : IPropertySelectionBehavior
{
  bool IPropertySelectionBehavior.SelectProperty(Type implementationType, PropertyInfo propertyInfo)
  {
    var attribute = propertyInfo.GetCustomAttribute<RequiredMemberAttribute>();

    return attribute is not null;
  }
}

So basically the question is more or less, should this be a fixed part of SimpleInjector.

mikernet commented 1 year ago

Yeah, that's what I meant when I said "auto-injecting", as in, "out of the box" :)

dotnetjunkie commented 1 year ago

I don't feel this is something I'd like to promote for Simple Injector, as I feel that constructor injection is a better pick for injecting dependencies. But as @GeraldLx showed, this behavior is easily plugged in into Simple Injector.

mikernet commented 1 year ago

IMO returning "invalidly constructed" types is worse than just allowing devs to massively reduce boilerplate using a built-in language construct that enforces initialization of properties if they so choose:

Before:

public class ServiceBase : IServiceBase
{
    IBaseDepA _baseDepA;
    IBaseDepB _baseDepB;
    IBaseDepC _baseDepC;
    IBaseDepD _baseDepD;

    public ServiceBase(IBaseDepA baseDepA, IBaseDepB baseDepB, IBaseDepC baseDepC, IBaseDepD baseDepD)
    {
        _baseDepA = baseDepA;
        _baseDepB = baseDepB;
        _baseDepC = baseDepC;
        _baseDepD = baseDepD;
    }
}

public class ServiceA : ServiceBase
{
    IDepA _depA;

    public ServiceA(IDepA _depA, IBaseDepA baseDepA, IBaseDepB baseDepB, IBaseDepC baseDepC, IBaseDepD baseDepD)
        : base(baseDepA, baseDepB, baseDepC, baseDepD)
    {
        _depA = _depA;
    }
}

public class ServiceB : ServiceBase
{
    IDepB _depB;

    public ServiceB(IDepB _depB, IBaseDepA baseDepA, IBaseDepB baseDepB, IBaseDepC baseDepC, IBaseDepD baseDepD)
        : base(baseDepA, baseDepB, baseDepC, baseDepD)
    {
        _depB = _depB;
    }
}

After:

public class ServiceBase : IServiceBase
{
    public required IBaseDepA BaseDepA { get; init; }
    public required IBaseDepB BaseDepB { get; init; }
    public required IBaseDepC BaseDepC { get; init; }
    public required IBaseDepD BaseDepD { get; init; }
}

public class ServiceA : ServiceBase
{
    public required IDepA DepA { get; init; }
}

public class ServiceB : ServiceBase
{
    public required IDepB DepB { get; init; }
}

It seems like an error to me for any tooling that can dynamically construct types with constructors to not respect required properties going forward, given that they are effectively just as required as constructor parameters outside of a reflection-based construction context.

What is it about the pattern above that differs so significantly from constructor injection that you consider it an anti-pattern?

dotnetjunkie commented 1 year ago

Consider using record types instead and, where possible, prevent using base classes (you know, composition over inheritance). In that case, your code would be the following:

public record ServiceB(IDepB _depB, IBaseDepA baseDepA, IBaseDepB baseDepB, IBaseDepC baseDepC, IBaseDepD baseDepD)
{
}

But either way, whatever behavior you like, it is easily plugged in to Simple Injector.

mikernet commented 1 year ago

composition over inheritance

Generally speaking yes of course, but not always possible or desirable when working with view models in MVVM desktop scenarios.

GeraldLx commented 1 year ago

SimpleInjector has always been about writing clean code. I always feel like a journey back in time, when I have to use the "new" Microsoft Dependency Injection.

There are sometimes reasons to use property injection. I used basically the same code I posted before with my own DependencyAttribute (the required keyword did not exist at that time) in project with a lot of legacy code heavily using inheritance.

Perhaps a compromise would be to have a fitting PropertySelectionBehavior included and a nice extension method or boolean property in the options to activate it?