Search code examples
c#simple-injector

SimpleInjector ctor injection mix registered types and simple values

How do I register types which take another registered type as a parameter and also simple types (like an integer)?

public interface IDeviceManager
{
   // implementation omitted.
}

public class DeviceManager : IDeviceManager
{
    public DeviceManager(IDeviceConfigRepository configRepo, int cacheTimeout)
    {
        // implementation omitted
    }
}

I do have a container registration for the IDeviceConfigRepository. That's ok. But how do I create an instance of DeviceManager with the configured dependency and passing along an integer of my choice in composition root?

I thought about creating a factory.

public class DeviceManagerFactory : IDeviceManagerFactory
{
    private readonly Container _container;

    public DeviceManagerFactory(Container container)
    {
        _container = container;
    }

    public DeviceManager Create(int minutes)
    {
        var configRepo = _container.GetInstance<IDeviceConfigurationRepository>();
        return new DeviceManager(configRepo, minutes);
    }
}

This is pretty simple.
However now I do not have a registration for DeviceManager which is the type I ultimately need. Should I change these dependencies to the factory instead?

public class ExampleClassUsingDeviceManager
{
    private readonly DeviceManager _deviceManager;

    public ExampleClassUsingDeviceManager(DeviceManager deviceManager, ...)
    {
        _deviceManage = deviceManager;
    }

    // actions...
}

For this to work and to avoid circular dependencies I would probably have to move the factory from the "application" project (as opposed to class libraries) where the composition root is to the project where the DeviceManager is implemented.

Is that OK? It would of course mean passing around the container.

Any other solutions to this?

EDIT In the same project for other types I am using parameter objects to inject configuration into my object graph. This works OK since I only have one class instance per parameter object type. If I had to inject different parameter object instances (for example MongoDbRepositoryOptions) into different class instances (for example MongoDbRepository) I would have to use some kind of named registration - which SimpleInjector doesn't support. Even though I only have one integer the parameter object pattern would solve my problem. But I'm not too happy about this pattern knowing it will break as soon as I have multiple instances of the consuming class (i.e. MongoDbRepository).

Example:

MongoDbRepositoryOptions options = new MongoDbRepositoryOptions();
MongoDbRepositoryOptions.CollectionName = "config";
MongoDbRepositoryOptions.ConnectionString = "mongodb://localhost:27017";
MongoDbRepositoryOptions.DatabaseName = "dev";

container.RegisterSingleton<MongoDbRepositoryOptions>(options);
container.RegisterSingleton<IDeviceConfigurationRepository, MongoDbRepository>();

I am excited to hear how you deal best with configurations done at composition root.

Solution

  • Letting your DeviceManagerFactory depend on Container is okay, as long as that factory implementation is part of your Composition Root.

    Another option is to inject the IDeviceConfigRepository into the DeviceManagerFactory, this way you can construct a DeviceManager without the need to access the container:

    public class DeviceManagerFactory : IDeviceManagerFactory {
    private readonly IDeviceConfigurationRepository _repository;

    public DeviceManagerFactory(IDeviceConfigurationRepository repository) {
        _repository = repository;
    }

    public DeviceManager Create(int minutes) {
        return new DeviceManager(_repository, minutes);
    }
}

    However now I do not have a registration for DeviceManager which is the type I ultimately need. Should I change these dependencies to the factory instead?

    In general I would say that factories are usually the wrong abstraction, since they complicate the consumer instead of simplifying them. So you should typically depend on the service abstraction itself (instead of depending on a factory abstraction that can produces service abstraction implementations), or you should inject some sort of proxy or mediator that completely hides the existence of the service abstraction from point of view of the consumer.

    @DavidL points at my blog post about runtime data. I'm unsure though whether the cacheTimeout is runtime data, although you seem to be using it as such, since you are passing it in into the Create method of the factory. But we're missing some context here, to determine what's going on. My blog post still stands though, if it is runtime data, it's an anti-pattern and in that case you should

    1. pass runtime data through method calls of the API

    or

    1. retrieve runtime data from specific abstractions that allow resolving runtime data.

    UPDATE

    In case the value you are using is an application constant, that is read through the configuration file, and doesn't change during lifetime of the application, it is perfectly fine to inject it through the constructor. In that case it is not a runtime value. There is also no need for a factory.

    There are multiple ways to register this in Simple Injector, for instance you can use a delegate to register the DeviceManager class:

    container.Register<DeviceManager>(() => new DeviceManager(
    container.GetInstance<IDeviceConfigRepository>(),
    cacheTimeout: 15));

    Downside of this approach is that you lose the ability of Simple Injector to auto-wire the type for you, and you disable Simple Injector's ability to verify, diagnose and visualize the object graph for you. Sometimes this is fine, while other times it is not.

    The problem here is that Simple Injector blocks the registration of primitive types (because they cause ambiguity) while not presenting you with a clean way to make the registration. We are considering (finally) adding such feature in v4, but that doesn't really address your current needs.

    Simple Injector doesn't easily allow you to specify a primitive dependency, while letting the container auto-wire the rest. Simple Injector's IDependencyInjectionBehavior abstraction allows you to override the default behavior (which is to disallow doing this). This is described here, but I usually advice against doing this, because it is usually requires quite a lot of code.

    There are basically two options here:

    1. Abstract the specific logic that deals with this caching out of the class and wrap it in a new class. This class will have just the cacheTimeout as its dependency. This is of course only useful when there actually is logical to abstract and is usually only logical when you are injecting that primitive value into multiple consumers. For instance, instead of injecting a connectionstring into multiple classes, you're probably better of injecting an IConnectionFactory into those classes instead.
    2. Wrap the cacheTimeout value into a complex data container specific for the consuming class. This enables you to register that type, since it resolves the ambiguity issue. In fact, this is what you yourself are already suggesting and I think this is a really good thing to do. Since those values are constant at runtime, it is fine to register that DTO as singleton, but make sure to make it immutable. When you give each consumer its own data object, you won't have to register multiple instances of those, since they are unique. Btw, although named registations aren't supported, you can make conditional or contextual registrations using RegisterConditional and there are other ways to achieve named registrations with Simple Injector, but again, I don't think you really need that here.