Is there a good/proper way of solving the dependency injection loop problem in the ASP.NET MVC ContactsManager tutorial?

If you don't know what I'm talking about either go through the tutorial and try to add dependency Injection yourself or try your luck with my explanation of the problem.

Note: This problem isn't within the scope of the original tutorial on ASP.NET. The tutorial only suggests that the patterns used are dependency injection friendly.

The problem is basically that there is a dependency loop between the Controller, the ModelStateWrapper and the ContactManagerService.

1. The ContactController constuctor takes an IContactManagerService.
2. The ContactManagerService constructor takes an IContactManagerRepository (not important) and an IValidationDictionary (which ModelStateWrapper implements).
3. The ModelStateWrapper constructor takes a ModelStateDictionary (which is a property called "ModelState" on the controller).

So the dependency cycle goes like this: Controller > Service > ModelStateWrapper > Controller

If you try to add dependency injection to this, it will fail. So my question is; what should I do about it? Others have posted this question, but the answers are few, different, and all seem kinda "hack-ish".

My current solution is to remove the IModelStateWrapper from the IService constructor and add an Initialize method instead like this:

public class ContactController : Controller
{
    private readonly IContactService _contactService;

    public ContactController(IContactService contactService)
    {
        _contactService = contactService;
        contactService.Initialize(new ModelStateWrapper(ModelState));
    }

    //Class implementation...
}

public class ContactService : IContactService
{
    private IValidationDictionary _validationDictionary;
    private readonly IContactRepository _contactRepository;

    public ContactService(IContactRepository contactRepository)
    {
        _contactRepository = contactRepository;
    }

    private void Initialize(IValidationDictionary validationDictionary)
    {
        if(validationDictionary == null)
            throw new ArgumentNullException("validationDictionary");

        _validationDictionary = validationDictionary;
    }

    //Class implementation...
}

public class ModelStateWrapper : IValidationDictionary
{
    private readonly ModelStateDictionary _modelState;

    public ModelStateWrapper(ModelStateDictionary modelState)
    {
        _modelState = modelState;
    }

    //Class implementation...
}

With this construct I can configure my unity container like this:

public static void ConfigureUnityContainer()
{
    IUnityContainer container = new UnityContainer();

    // Registrations
    container.RegisterTypeInHttpRequestLifetime<IContactRepository, EntityContactRepository>();
    container.RegisterTypeInHttpRequestLifetime<IContactService, ContactService>();

    ControllerBuilder.Current.SetControllerFactory(new UnityControllerFactory(container));
}

Unfortunately this means that the "Initialize" method on the service has to be called manually by the controller constructor. Is there a better way? Maybe where I somehow include the IValidationDictionary in my unity configuration? Should I switch to another DI container? Am I missing something?

Solution 1:

As a general consideration, circular dependencies indicate a design flaw - I think I can safely say this since you are not the original author of the code :)

I wouldn't consider an Initialize method a good solution. Unless you are dealing with an add-in scenario (which you aren't), Method Injection is not the right solution. You have almost already figured that out, since you find it unsatisfactory that you need to manually invoke it because your DI Container can't.

Unless I am entirely mistaken, the ContactController doesn't need the IValidationDictionary instance before its Action methods are being invoked?

If this is true, the easiest solution would probably be to define an IValidationDictionaryFactory interface and make the ContactController constructor take an instance of this interface.

This interface could be defined like this:

public interface IValidationDictionaryFactory
{
    IValidationDictionary Create(Controller controller);
}

Any Action method on the controller that needs an IValidationDictionary instance can then invoke the Create method to get the instance.

The default implementation would look something like this:

public class DefaultValidationDictionaryFactory : IValidationDictionaryFactory
{
    public IValidationDictionary Create(Controller controller)
    {
        return controller.ModelState;
    }
}

Solution 2:

How about slightly changing/improving the design to something like this: http://forums.asp.net/t/1486130.aspx