Search code examples
c#unit-testingdesign-patternsdependency-injectionsimple-injector

Dependency Injection and initialization methods


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


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.