Circular dependency in dependency injection

Question

we're quite new to DI and we're refactoring our application to use Prism/Unity. We're almost there but got stuck on a circular dependency. I've been reading a lot, there are a lot of similar questions but I'm in doubt as to what would be a 'correct' solution in our situation.

The application is similar to some IDEs. We have a project explorer, search, documents, ... The documents are actually treeviews with nodes of INodeViewModel. Note that the code samples are minimized versions of the real code.

We have a IDockingService that manages the tool and document items.

public interface IDockingService
{
 INodeViewModelNavigationService NodeViewModelNavigationService { get; }
 ExplorerViewModel ExplorerViewModel { get; }
 SearchViewModel SearchViewModel { get; }
 ReadOnlyCollection<ToolItemViewModel> ToolItemViewModels { get; }
 ReadOnlyCollection<DocumentItemViewModel> DocumentItemViewModels { get; }
}

And we have a ISpecificationViewModelService that manages all the INodeViewModels and has the connection with the model.

public interface ISpecificationViewModelService
{
 INodeViewModel RootX { get; }
 INodeViewModel RootY { get; }
 INodeViewModel RootZ { get; }
}

We have two requirements that conflict.

• When a new node is created somewhere we want to navigate to it. Currently we pass IDockingService via constructor injection to the concrete SpecificationViewModelService implementation and further down to each NodeViewModel.
• Certain tools in the DockingService need to know about the ISpecificationViewModelService. For example the Explorer needs to display all the roots.

Since DockingItemService creates and manages the toolitems such as the Explorer it needs the ISpecificationViewModelService. But the ISpecificationViewModelService needs the IDockingService to be able to navigate to a node. Circular troubles.

Our first attempt didn't have this issue because we let the the tool items be created in the composition root but that didn't seem correct. Instances we not managed together nicely.

From reading around I understand that a factory (or another third class) could help out here. I'm not seeing how yet. I think I understand that the problem is that DockingService only needs SpecificationViewModelService to create the toolitems and vice versa. A factory could take this circular trouble away but I'm not sure it is a correct solution. The container can only be used in the composition root but wouldn't a facory (which needs the container?) then just be hiding the container and take over its work under a different name?

What would be a correct way to handle this problem?

• Timothy Truckle: It looks like you're not correctly separate the model from the view. Move the "navigation" related methods/data to a new class located in the view.
• Jef Patat: You'll have to elaborate on that. Nowhere in my question i see a connection between model & view. This is a WPF application. Navigation can perfectly be triggered from the VM layer. Things like navigation history, manipulation, enabling, navigating from a search result, ... clearly don't belong in the view.
• Timothy Truckle: Cyclic dependencies are an indication that some behavior should be extracted to a separate unit. And yes: I was wrong, the model does not have to do anything with it. Sorry.
• Jef Patat: I know that, I stated it in other words in the question. My problem is how I can solve it in a correct way taking care of the concerns I stated.
• Timothy Truckle: IMHO there is no "general advice" possible. It all depends on your concrete (current) implementation...

Answer 1

I'm going to significantly abstract from your situation. I'm not sure if this will be the best solution to your problem. It will be a solution, but it may be just enabling a poor design. It's hard to tell, but certainly the situation comes up in reasonable designs.

It's quite common that you get in a situation that to build X you need Y, and to build Y you need X. Of course, it can't be that conceptually to build X you need to have completely built a Y and vice versa as then it would be logically impossible to construct the system. Instead, what is really meant is to build one you merely need a way of referring to the other. There are many ways of referring to something that hasn't yet been created. For example, in C you might pass a pointer to allocated but uninitialized memory, or you might pass a string that will be used to look up the created object in a registry. In a language like Haskell or Scala, one way to solve this is with lazy evaluation. Implementation-wise, this is more or less equivalent to passing around a higher order function with some internal mutable state, or, equivalently, a "factory" object that will memoize the dependency resolution. This is the approach I'm going to suggest here. I'll use Autofac's terminology and API below, though the idea should be easily be adaptable to other dependency injection frameworks.

class Lazy<T> where T : class {
 private T _cached = null;
 private IContainer _container;
 public Lazy(Func<IContainer> container) {
 _container = container();
 }
 public T Value {
 get {
 if(_cached != null) return _cached;
 _cached = _container.Resolve<T>();
 _container = null;
 return _cached;
 }
 }
}

Here IContainer is intended to be the dependency injection container, and Resolve does the dependency look-up. I strongly recommend against passing the dependency injection container to objects, but in this case I'm viewing Lazy as part of the dependency injection framework. In fact, you should make sure that your dependency injection framework doesn't already provide such functionality. The alternative would be to make a factory for each class you want to lazily inject. The code would be basically identical except that IContainer would be replaced by any dependencies and instead of using Resolve to create the object you'd use new. Alternatively, you could generalize Lazy to take an Action<T> and just memoize it and then these factory objects would just be singletons that you register in the composition root like: container.Register(new Lazy<Foo>(() => new Foo())); The ideal situation would be to register the open generic type Lazy<>, so that with one registration you could handle all cases. (I have verified that you can make this work with Autofac.)

The upshot of this is that you would depend on Lazy<IFoo> instead of IFoo and you would access via the Value property (but not in the constructor!)

• Jef Patat: I think you are hitting the nail right on here. I indeed think I don't need the circularity to construct. I'll reflect on this further, investigate it further and come back on it.
• Timo: A small improvement: Currently if _cached is populated with null, we get a NulReferenceException on the next call to Value. This is avoided by replacing if (_cached != null) by if (_container == null).

Answer 2

If you really have to, another solution would be to drop your constructor injection, add IServiceProvider constructor injection, and use ActivatorUtilities when you need the docking service like this :

var dockingService = (IDockingService)ActivatorUtilities.CreateInstance(this.serviceProvider, typeof(DockingService));

Of course this is not the preferred way, and you can't test it properly (as you are instantiating a concrete IDockingService), but it's a solution.