Register a dependency, based on static property of other dependency

Question

I'm sure I'm using the right terms, so it's probably best to resort to pseudo code.

A dll has a type, without an interface and exposes the stuff I need through static properties. The code is pretty ugly.

public class ExternalType : IDisposable
{
   public static IOtherType PropertyINeed { get; set; } // also disposable

   public ExternalType(IOtherTypeFactory otherTypeFactory)
   {
        PropertyINeed = otherTypeFactory.Create();
   }
}

// in the registration I'd like to accomplish something along these lines
public class Service
{
   //...

   public void RegisterStuff()
   {
       Container.Register<IOtherTypeFactory>(() => new OtherTypeFactory());
       Container.Register<ExternalType>(() => new ExternalType());

       // Offending part: I need to have IOtherType registered with this 
       // implementation.

       // throws: Additional information: The registered delegate for type 
       // IOtherType returned null.
       Container.Register<IOtherType>(() =>  ExternalType.PropetyINeed)
   }
}

Since registrations and getting instances cannot be mixed, I'm wondering how to approach this scenario. I'm not at liberty to change the implementation of 'ExternalType', otherwise I would've done that :)

Answer 1

Your code is exibiting several code smells and anti-patterns:

• You use an Abstract Factory IOtherTypeFactory: this is a code smell, because a type should use the IOtherType directly instead of having to depend on two interfaces.
• The public static IOtherType PropertyINeed property is an implementation of the Ambient Context anti-pattern. The problems of this anti-pattern are very much related to the problems of the Service Locator anti-pattern, such as:
  • The Dependency is hidden.
  • Testing becomes more difficult.
  • It becomes very hard to change the Dependency based on its context.
• The PropertyINeed property has to be initialized before use, and in your case the initialization is done by the construction of ExternalType. This causes Temporal Coupling.
• The PropertyINeed is static, which means IOtherType must be thread-safe as well, which is very unclear from this design. There are no safety nets in your code, nor can DI Containers help you to detect such issue because of the existance of this static property.
• The PropertyINeed is initialized over and over again, which can cause concurrency problems as well, especially when IOtherType changes under certain conditions.

The solution to these problems is not to store IOtherType in any static field and not to expose IOtherType from any class. Instead rely solely on Constructor Injection to inject IOtherType itself into any consumer that requires its use. Do not revert to Abstract Factories, Ambient Context or Service Locator.

If refactoring this at once is too hard, I would suggest doing the following as intermediate step:

var otherTypeInstance = new OtherTypeFactory().Create();

Container.RegisterSingleton<IOtherType>(otherTypeInstance);
Container.Register<ExternalType>();

// Set PropetyINeed Ambient Context with IOtherType singleton.
ExternalType.PropertyINeed = otherTypeInstance;

What changed:

• ExternalType doesn't depend on IOtherTypeFactory anymore.
• IOtherTypeFactory isn't registered anymore in the container: nobody needs to depend on it.
• The singleton IOtherType is created directly in the Composition Root using OtherTypeFactory and registered as 'single instance' in the container.
• The PropertyINeed Ambient Context is initialized at the end of the Composition Root using the earlier created instance.