Dependency Injection and initialization methods

c#, dependency-injection, design-patterns, simple-injector, unit-testing

Solution

Perhaps I misunderstood your question, but I don't think Simple Injector is a limiting factor here. Since constructors should do as little as possible, you shouldn't call the `Create` method from within your constructor. It's even an odd thing to do, since a factory is meant to delay the creation of a type, but since you call `Create` inside the constructor the creation is not delayed.

Your `Foo` constructor should simply depend on `IRedisWrapper` and you should extract the `redisFactory.Create()` call to your DI configuration like this:

var redisFactory = new RedisWrapperFactory();

container.Register<IRedisWrapper>(() => redisFactory.Create());

But since the factory's sole purpose is to prevent duplicate initialization logic throughout the application, it now lost its purpose, since the only place the factory is used is within the DI configuration. So you can throw the factory out and write the following registration:

container.Register<IRedisWrapper>(() =>
{
    IRedisWrapper redis = new RedisWrapper();
    redis.init("localhost", 1234);
    return redis;
});

You now placed the body of the `Create` method inside the anonymous delegate. Your `RedisWrapper` class currently has a default constructor, so this approach is fine. But if the `RedisWrapper` starts to get dependencies of its own, it's better to let the container create that instance. This can be done as follows:

container.Register<IRedisWrapper>(() =>
{
    var redis = container.GetInstance<RedisWrapper>();
    redis.init("localhost", 1234);
    return redis;
});

When you need your class to be initialized after creation, as the `RedisWrapper` clearly needs, the adviced approach is to use the `RegisterInitializer` method. The last code snippet can written as follows:

container.Register<IRedisWrapper, RedisWrapper>();

container.RegisterInitializer<RedisWrapper>(redis =>
{
    redis.init("localhost", 1234);
});

This registers the `RedisWrapper` to be returned when an `IRedisWrapper` is requested and that `RedisWrapper` is initialized with the registered initializer. This registration prevents the hidden call back to the container. This improves performance and improves the ability for the container to diagnose your configuration.

Problem

I read Miško Hevery's Guide: Writing Testable Code and it states a warning sign if an "Object not fully initialized after the constructor finishes (watch out for initialize methods)". Let's say I wrote a Redis wrapper class that has an init method that accepts hostname and port. This according to Miško, is a warning sign, since I need to call its init method. The solution I'm contemplating with is the following: For every class that need this kind of initialization, create a factory class that has a Create method which creates the class, and also call its init method. Now in code: Instead of using something like: ``` class Foo { private IRedisWrapper _redis; public Foo(IRedisWrapper redis) { _redis = redis; } } .... IRedisWrapper redis = new RedisWrapper(); redis.init("localhost", 1234); Foo foo = new Foo(redis); ``` I'd use something like: ``` class Foo { private IRedisWrapper _redis; public Foo(IRedisWrapper redis) { _redis = redis; } } .... RedisWrapperFactory redisFactory = new RedisWrapperFactory(); IRedisWrapper redisWrapper = redisFactory.Create(); Foo foo = new Foo(redisWrapper); ``` I'm using `Simple Injector` as an IOC framework, which makes this the above solution probelmatic - in this case I'd use something like: ``` class Foo { private RedisWrapper _redis; public Foo(IRedisWrapperFactory redisFactory) { _redis = redisFactory.Create(); } } ``` I'd really like to hear your input on the above solution. Thanks

Original source