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.22k stars 152 forks source link

RegisterPackages extension for collection of IPackage interface #870

Closed mariohines closed 3 years ago

mariohines commented 4 years ago

The current extension uses a lot of reflection but there isn't one for a collection of IPackage. I think having an extension for just a collection of IPackage would add greater versatility. The reason I request this is due using a parameterized constructor which isn't currently allowed.

dotnetjunkie commented 4 years ago

Can you give an example that illustrates what you want to achieve?

mariohines commented 4 years ago

Use case: Class implementation of IPackage has a parameter in the constructor. Common scenario would be connectionStrings for a database or a caching provider like Redis in the appSettings.json file. This also allows for greater observance of separation of concerns.

public sealed class RegistrationPackage : IPackage
{
     private readonly SystemSettings _settings;
     public RegistrationPackage (SystemSettings settings)
     {
         _settings = settings;
     }

     // Implementation for RegisterServices not included for simplicity.
}

Solution: Method to accommodate one or more explicit IPackage implementations without reflection.

public static void RegisterPackages(this Container container, params IPackage[] packages)
{
     // ... checks similar to current implementation not included for simplicity.
     foreach(var package in packages)
     {
          package.RegisterServices(container);
     }
}

Solution Usage:

var settings = Configuration.GetSection(nameof(SystemSettings)).Get<SystemSettings>();
_container.RegisterPackages(new RegistrationPackage(settings));

This would allow versatility in having dependencies for registration using Package Registration passed in via constructor instead of registering the dependency in the container only to pull it out and not really need it throughout the application.

dotnetjunkie commented 4 years ago

The current extension uses a lot of reflection but there isn't one for a collection of IPackage. I think having an extension for just a collection of IPackage would add greater versatility

This use case is not something that the Packages library was designed for. As stated in the documentation:

SimpleInjector.Packaging exists to accommodate applications that require plug-in like modularization, where parts of the application, packed with their own container registrations, can be independently compiled into a dll and ‘dropped’ into a folder, where the main application can pick them up, without the need for the main application to be recompiled and redeployed.

Your use case does not fit this use case as your packages (e.g. RegistrationPackage) are known at compile time:

// Compiler knows at RegistrationPackage at this point
_container.RegisterPackages(new RegistrationPackage(settings));

The reason for SimpleInjector.Packaging to focus on plug-in scenario where the packages are unknown to the root application, is because you don't need SimpleInjector.Packaging in case the packages are known to the root application, as you will be able to simply do this in the Composition Root:

// Make RegistrationPackage static
RegistrationPackage.RegisterServices(container);

And in the case where you need some extra settings, just add it as a parameter:

RegistrationPackage.RegisterServices(container, settings);

For providing values to plug-in packages, there is currently the possibility to use the Container.ContainerScope Items dictionary to provide items to a package. Example:

// Add the settings to the container's items dictionary before adding packages
container.ContainerScope.SetItem("settings", settings);

container.RegisterPackages(pluginAssemblies);

public sealed class RegistrationPackage : IPackage
{
    public void RegisterServices(Container container)
    {
        // Retrieve the settings from the container's items dictionary
        var settings = (SystemSettings)container.ContainerScope.GetItem("settings");
    }
}
mariohines commented 4 years ago

This use case is not something that the Packages library was designed for. As stated in the documentation

That's fair. Plug-in architecture is a good use case for it. What I was asking for is that it be extended. Why can't it handle plug-in architecture and knowing which assemblies to use. That shouldn't cause any harm to it to be honest. Also part of your documentation:

Simple Injector has the notion of ‘packages’. A package is a group of container registrations packed into a class that implements the IPackage interface. This feature is similar to what other containers call Installers, Modules or Registries.

Other containers don't explicitly restrict it to plug-in like architecture and have what I requested as an optional override. Also, while the workaround you provided would work....it's pretty similar to what I was against. I would still not be injecting the dependency into the constructor of the package.

Thanks for the help though. I'd still think it would be a good addition to the SimpleInjector.Packaging library.

dotnetjunkie commented 4 years ago

Your use case is certainly something that can be added to Packaging, and something that other DI Containers might support OOTB. It's, however, not the intention of Packaging to replicate what other DI Containers do, as this is the case for the whole of Simple Injector. Packaging focuses on plug-in architecture, because it is our point of view that such feature shouldn't be used it outside of that context, because in that case the simpler solution is simply to not use Packaging and simply use static method calls (as I shown above).

Because of this, your request doesn't match Simple Injector's point of view, and I will not be adding this to Packaging. Fortunately, as you already demonstrated, implementing this feature takes 3 lines of code. So you will have no difficulty to add this to your code base.