Type object code smells · Brian Hartsock's Blog

I have been playing around with the Enyim Memcached client for .NET over the past few days. It has a lot of really cool functionality, but one big smell that has been bugging me. Extensibility is used through the Type class, not through dependency injection. Here is the interface for the configuration object.

public class MemcachedClientConfiguration : IMemcachedClientConfiguration
{
   public MemcachedClientConfiguration();

   public Type NodeLocator { get; set; }
   public Type KeyTransformer { get; set; }
   public IList<ipendpoint> Servers { get; }
   public ISocketPoolConfiguration SocketPool { get; }
   public Type Transcoder { get; set; }
}

Note that KeyTransformer, NodeLocator, and Transcoder are extension points for the library (really cool extension points at that). What sucks is they are Type objects, not the real implementation classes. Here is an example of how you configure the NodeLocator.

config.NodeLocator = typeof(MyNodeLocator);

Try to implement the decorator pattern in a scenario like this. You can’t, because the library is in complete control of the life-cycle of the object. There are two solutions.

public class MemcachedClientConfiguration : IMemcachedClientConfiguration
{
   public MemcachedClientConfiguration();

   //Notice the change to an interface
   public IMemcachedNodeLocator NodeLocator { get; set; }
   public Type KeyTransformer { get; set; }
   public IList<ipendpoint> Servers { get; }
   public ISocketPoolConfiguration SocketPool { get; }
   public Type Transcoder { get; set; }
}

//And then configure like follows.
config.NodeLocator = new MyNodeLocator();

But what if controlling the life-cycle is important to the library? Well, that’s why the factory pattern exists.

public class MyNodeLocatorFactory : IMemcachedNodeLocatorFactory
{
    IMemcachedNodeLocator Create();
}

So try to avoid referencing Type objects. As you can see, it smells of an inflexible design. Not to mention leaves the possibility open to runtime errors for no good reason.

Menu