Dependency Injection and initialization methods

Question

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

Answer 1

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.

Answer 2

Having RedisWrapperFactory as the dependency doesn't seem quite right, since it's not really the factory you want. Unless of course there were specific parameters that you needed to pass to Create().

I don't know Simple Injector, but I would suggest that if it doesn't allow you to customise the creation of you objects to use your factory, you might want to look at some other DI frameworks. I use StructureMap, but there are others to choose from.

Edit: Having said that, if the contract for IRedisWrapper is that it must be initialised in some specific way after the constructor is called, it will look a little odd if you use it in Foo without calling init(). Especially to someone who is familiar with IRedisWrapper (many people), and not with the IOC setup of that particular application (not many people). Certainly, if you are going to use a factory, as Arghya C has said, use that through an interface too, otherwise you haven't actually achieved anything because you don't get to choose which IRedisWrapper you are injecting.

Answer 3

If your RedisWrapperFactory is defined in some other layer (where it gets data from DB/File/some service), the the code will kill the purpose of dependncy injection. Your layer becomes directly dependent on the other. Also, that is not testable anymore as you cannot create a mock/fake object of that for testing. Obviously you don't want to do real DB operations or I/O read-write or service call in testing.

You might want to do something like...

class Foo
{
    private IRedisWrapper _redis;

    public Foo(IRedisWrapperFactory redisFactory)
    {
        _redis = redisFactory.Create();
    }
}

In the other layer

public class RedisWrapperFactory : IRedisWrapperFactory
{
    public IRedisWrapper Create()
    {
        var r = RedisWrapper();
        r.Init("localhost", 1234); //values coming from elsewhere
        return r;
    }                                   
}

In your Bootstrapper() or application_Start() method, inject the factory, something like

container.Register<IRedisWrapperFactory, RedisWrapperFactory>();