How to fix a Circular Dependency

Question

So I've just recently started using the Microsoft.Extensions.DependencyInjection nuget package in my WPF project because I wanted to start learning more about DI.

Issue

I keep getting a Circular Dependency Exception whenever I try to access a dependency from any other ViewModel besides the MainViewModel

This is what I have done so far.

I've installed these two Nuget packages into my project

Microsoft.Extensions.Hosting --version 7.0.0

Microsoft.Extensions.DependencyInjection --version 7.0.0

And then I went ahead and created my container inside my App.xaml.cs

public partial class App : Application
{
    private readonly ServiceProvider _serviceProvider;
    public App()
    {
        IServiceCollection _services = new ServiceCollection();
        
        _services.AddSingleton<MainViewModel>();
        _services.AddSingleton<HomeViewModel>();
        _services.AddSingleton<SettingsViewModel>();
        
        _services.AddSingleton<DataService>();
        _services.AddSingleton<NavService>();
        
        _services.AddSingleton<MainWindow>(o => new MainWindow
        {
            DataContext = o.GetRequiredService<MainViewModel>()
        });
        _serviceProvider = _services.BuildServiceProvider();
    }
    protected override void OnStartup(StartupEventArgs e)
    {
        var MainWindow = _serviceProvider.GetRequiredService<MainWindow>();
        MainWindow.Show();
        base.OnStartup(e);
    }
}

In my App.xaml I also defined a few DataTemplates which will allow me to display different views based in their DataType

<Application.Resources>
    <DataTemplate DataType="{x:Type viewModel:HomeViewModel}">
        <view:HomeView/>
    </DataTemplate>
    
    <DataTemplate DataType="{x:Type viewModel:SettingsViewModel}">
        <view:SettingsView/>
    </DataTemplate>
</Application.Resources>

Then I went ahead and created my MainWindow.xaml

<Window x:Class="Navs.MainWindow"
        ...>

    <Grid>
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="100" />
            <ColumnDefinition />
        </Grid.ColumnDefinitions>

        <Border>
            <StackPanel>
                <Button Height="25" Content="Home" Command="{Binding HomeViewCommand}"/>
                <Button Height="25" Content="Settings" Command="{Binding SettingsViewCommand}"/>
            </StackPanel>
        </Border>

        <ContentControl Grid.Column="1" Content="{Binding NavService.CurrentView}">
            
        </ContentControl>
    </Grid>
</Window>

And a corresponding ViewModel

public class MainViewModel : ObservableObject
{
    private NavService _navService;
    public NavService NavService
    {
        get => _navService;
        set
        {
            _navService = value;
            OnPropertyChanged();
        }
    }
    /* Commands */
    public RelayCommand SettingsViewCommand { get; set; }
    public RelayCommand HomeViewCommand { get; set; }
    public MainViewModel(NavService navService, HomeViewModel homeViewModel, SettingsViewModel settingsViewModel)
    {
        NavService = navService;
        
        HomeViewCommand = new RelayCommand(o => true, o => { NavService.CurrentView = homeViewModel; });
        SettingsViewCommand = new RelayCommand(o => true, o => { NavService.CurrentView = settingsViewModel; });
    }
}

As you can see, with the help of Dependency Injection, I can now access the objects I've registered in my container through the constructor.

I've also created two UserControls

UserControl1

<Grid>
    <StackPanel VerticalAlignment="Center">
        <Button Height="25" Content="Click me" Command="{Binding OpenWindowCommand}" />
        <Button Content="Settings View" Command="{Binding SettingsViewCommand}" Height="25" />
    </StackPanel>
</Grid>

And it's corresponding ViewModel

public class HomeViewModel
{
    public RelayCommand SettingsViewCommand { get; set; }
    public HomeViewModel()
    {
        
    }
}

And then we have UserControl2

<Grid>
    <StackPanel VerticalAlignment="Center">
        
    <TextBox Text="{Binding Message}"
             Height="25"/>
    
    <Button Height="25" Content="Home View" Command="{Binding HomeViewCommand}"/>
    <Button Height="25" Content="Fetch" Command="{Binding FetchDataCommand}"/>
    </StackPanel>
</Grid>

With it's corresponding ViewModel

public class SettingsViewModel : ObservableObject
{
    public string Message { get; set; }
    public RelayCommand HomeViewCommand { get; set; }
    public RelayCommand FetchDataCommand { get; set; }
    public SettingsViewModel()
    {
        
    }
}

The NavService.cs

public class NavService : ObservableObject
{
    private object _currentView;

    public object CurrentView
    {
        get => _currentView;
        set
        {
            _currentView = value;
            OnPropertyChanged();
        }
    }

    private HomeViewModel HomeViewModel { get; set; }
    private SettingsViewModel SettingsViewModel { get; set; }

    public NavService(HomeViewModel homeViewModel, SettingsViewModel settingsViewModel)
    {
        HomeViewModel = homeViewModel;
        SettingsViewModel = settingsViewModel;

        CurrentView = HomeViewModel;
    }

    public void NavigateTo(string viewName)
    {
        switch (viewName)
        {
            case "Settings":
                CurrentView = SettingsViewModel;
                break;
            case "Home":
                CurrentView = HomeViewModel;
                break;
        }
    }
}

This all works just fine, the issue occurs when I take the HomeViewModel and try to pass in the NavService as a constructor.

public HomeViewModel(NavService navService)
{
    
}

At that point, it throws the exception.

I want to be able to access the NavService from various Views so that I can change the NavService.CurrentView from multiple places.

Answer 1

It's rarely a good idea to resolve mainwindow out your DI container.

You want that rendered ASAP and what are you going to inject into it's constructor?

Probably nothing.

What are you going to switch it out for?

Probably nothing.

The main practical reason you want interfaces is so you can mock that class in your unit tests.

Everything you build should have a single responsibility.

The mainwindow's job is to contain what you initially show the user. Maybe pretty much everything.

Unit testing UI is quite a challenge so it's common to reduce what goes into it to re-usable components and not write tests.

You should not pass in the concrete versions of viewmodels to mainviewmodel. If it needs to know about them, what do you do when you have 50 views?

public MainViewModel(NavService navService)
{

Navservice should resolve concrete instances on demand. More like.

  NavService.CurrentView = 
 
 (IInitiatedViewModel)serviceProvider.GetService(typeofregisteredinterface);

The serviceprovider is an instance class. You need some sort of reference to it unless you instantiate absolutely everything in app.xaml.cs resolving everything at start up.

In a real world application, you will need transients. You will not be able to rely on everything being a singleton.

You will want to navigate to a NEW instance of a FooViewModel because you want a new Foo which isn't the same as the last Foo and will not be the same as a future Foo. You will want a new Customer or repository instance.

You will have more complicated things in any real app than two viewmodels.

Note that serviceprovider is:

https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.serviceprovider?view=dotnet-plat-ext-7.0

Which already has an interface defined IServiceProvider. So you could easily inject a mock for testing purposes.

The serviceprovider you use in app.xaml will need to be referenced somehow.

You usually want the viewmodel interface:

    interface IInitiatedViewModel
    {
        Task Initiate();
    }

So you can get any data for your viewmodel after it's instantiated.

    public async Task Initiate()
    {
        Suppliers = await _repository.GetAddressesByTypeAsync((int)AddressType.Supplier);
        if(Suppliers.Count == 1)
        {
            ChosenSupplier = Suppliers[0];
        }
    }

I suggest you should also have a list somewhere which has Type and Description of your viewmodel and view.

You can then abstract away navigation from specific viewmodel types. They're choosing whatever the [3] view is and that is called whatever is in description, it's viewmodel interface is whatever is in Type.

If necessary you can then extend this principle to have an optional factory method in there.

And logic can be in your navigation service or another injected class.

Parent viewmodels such as homeviewmodel may have dependencies but they use things rather than get used by something else. There is no real advantage to defining an interface for them because you never really replace them with a moq for tests. That means there's no need to register an interface as being associated with them.

I often find there are a lot of these.

You can define some custom attributes for these and compose your navigation list dynamically. This saves editing separate things. You don't want to compose this list before you've shown the user the first view so it's usual to resolve HomeViewModel and MainWindowViewModel immediately. You can register these close to your entry point for di if necessary.

Others can be added using reflection.

Hence you could have viewmodels decorated:

 [ParentViewModel("Foo View Name Here")]
 ....
 public class FooViewModel()
 {

You can find code samples which allow such reflection:

How enumerate all classes with custom class attribute?

As your app grows you might have many views. Iterating all those classes can be done in a console app generates xml or a source generator. You can also have attributes define aspects such as menu location. Because attributes go on classes, they're associated with the code you're writing/maintaining rather than completely disconnected. That means you're less likely to make mistakes like mis spell an enum or fail to remove an obsolete enum entry when you remove an unwanted view.

Attribute driven generation is very powerful.

You could use code generation to generate interfaces and registration for repeated patterns such as views and viewmodels.

You might not find any value in explicitly registering these with the dependency injection container if you do not want a singleton for each. You can instantiate an instance of an UnregisteredClass using ActivatorUtilities.CreateInstance

    ActivatorUtilities.CreateInstance<UnregisteredClass>(serviceProvider);

That will provide any dependencies to your UnregisteredClass.

https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.activatorutilities.createinstance?view=dotnet-plat-ext-7.0&viewFallbackFrom=aspnetcore-2.1#Microsoft_Extensions_DependencyInjection_ActivatorUtilities_CreateInstance__1_System_IServiceProvider_System_Object___

There's an optional parameter which can be useful to provide variable parameters. Like say you want Invoice nnn.

Answer 2

Don't configure the IoC container in the constructor! Move related code to your OnStartup override or the Application.Startup event handler. The constructor is only meant to initialize/configure the instance. The constructor must always return fast.

Your are implementing Dependency Injection wrong. As you have currently implemented it, it defies its purpose. First step is always to follow the Dependency Inversion principle (the D in SOLID): don't depend on concrete types. Rather depend on their abstractions.
This means you have to introduce abstract classes and interfaces. Then only inject those abstractions into your concrete types.

Circular dependencies are usually introduced by designing wrong responsibilities (bad class design). IoC reveals this design flaw because dependencies are now publicly exposed, usually via the constructor.

Circular dependencies exist on constructor level and on class level. A constructor level and class level circular dependency exist independent of each other and must be fixed separately. Fixing one doesn't fix the other. It's the constructor level circular dependency that throws the InvalidOperationException or StackOverflowException.

From a class level design perspective,
when type A depends on B and B on A (A ⟷ B)
then you have a circular dependency and the following options to fix it:
a) A and Bshould be merged into a single class (A)
b) B has too much responsibility or too much knowledge of A or other classes in general. Move related responsibility back to A. Now B would have to use A to fulfill its responsibility

A ⟷ B ➽ A ⟶ B

c) the shared logic must be moved/extracted to a third type C:

A ⟷ B ➽ A ⟶ C ⟵ B  

d) introduce interfaces to invert the dependency (Dependency Inversion principle):

            IA    
           ⬀ ⬉        
A ⟷ B ➽ A   B  
           ⬊ ⬃  
            IB 

This means:
a) A knows how to navigate. This could lead to A having too much responsibility. The proof: every type that needs to navigate also has to implement the complete logic (duplicate code)
b) every type knows where it can/is allowed to navigate to. While the navigation logic is encapsulated by a dedicated type (NavigationService), the actual valid destination is only known to the client. This adds robustness to the code. In you case, this would mean A will have to provide B with arguments to allow B to fulfill its responsibility. B is now unaware of the existence of A (A ➞ B).
c) because the dependency is not introduced to make particular class members (API) available, c) can't be applied in your case. In your case your B depends on A as a type alone (instance and not instance members).
d) because the circular dependency is manifested in the constructor, introducing interfaces alone won't resolve the circular dependency exception thrown by the IoC container (or creator in general).

Three Solutions

To fix your original problem you have three options:

1. hide the (constructor) dependencies behind a factory (not recommended)
2. fix your design. The NavigationService knows too much. Following your pattern, NavigationService will have to know every view model class (or every navigation destination) explicitly.
3. use property injection instead of constructor injection (not recommended)

The following examples will use a Func<TProduct> instead of an abstract factory to simplify the examples.
The examples also use an enum as destination identifier to eliminate the use of magic strings
and assume that you have introduced an interface for each dependency.

It's important to understand that there are basically two circular dependencies: class level and constructor. Both can be resolved individually.
The class level circular dependency is usually resolved by introducing interfaces (applying the Dependency Inversion principle). The constructor circular dependency is fixed using one of the three below suggestions.
For the sake of completeness, all three suggestions also fix the class level circular dependency (although it is not responsible for the circular dependency exception thrown by the IoC container).

NavigationId.cs
enum which is used by all examples to replace the magic string parameter to identify the navigation destination.

public enum NavigationId
{
  None = 0,
  HomeScreen,
  SettingsScreen
}

Solution 1): Hide dependencies (not recommended)

Instead of depending on explicit types, let your classes depend on (abstract) factories by implementing the Abstract Factory pattern.

Note, there still will be an implicit circular dependency. It is just hidden behind factories. The dependency is just removed from the constructor (constructor dependency - constructing a NavigationService no longer requires the construction of HomeViewModel and vice versa).
As already mentioned, you would have to introduce interfaces (for example a IHomeViewModel) to completely remove the circular dependency.

You will also see that in order to add more destinations you would have to modify the NavigationService too. This is a good indicator that you have implemented a bad design. In fact, you have violated the Open-Closed principle (the O in SOLID).

NavigationService.cs

class NavigationService : INavigationService, INotifyPropertyChanged
{
  // Constructor.
  // Because of the factory the circular dependency of the constructor
  // is broken. On class level the dependency still exists,
  // but could be removed by introducing a 'IHomeViewModel'  interface.
  public NavigationService(Func<IHomeViewModel> homeViewModelFactory)
  {
    // This reveals that the class knows too much.
    // To introduce more destinations, 
    // you will always have to modify this code.
    // Same applies to your switch-statement.
    // A switch-statement is another good indicator 
    // for breaking the Open-Closed principle
    this.NavigationDestinationFactoryTable = new Dictionary<NavigationId, Func<object>>()
    {
      { NavigationId.HomeScreen, homeViewModelFactory.Invoke()}
    };
  }

  public void Navigate(NavigationId navigationId)
    => this.CurrentSource = this.NavigationDestinationTable.TryGetValue(navigationId, out Func<object> factory) ? factory.Invoke() : default;

  public object CurrentSource { get; private set; }
  private Dictionary<NavigationId, Func<object>> NavigationDestinationFactoryTable { get; }
}

HomeViewModel.cs

class HomeViewModel : IHomeViewModel, INotifyPropertyChanged
{
  private INavigationService NavigationService { get; }

  // Constructor
  public HomeViewModel(Func<INavigationService> navigationServiceFactory)
    => this.NavigationService = navigationServiceFactory.Invoke();
}

App.xaml.cs
Configure the IoC container to inject the factories. In this example the factories are simple Func<T> delegates. For more complex scenarios you probably want to implement abstract factories instead.

protected override void OnStartup(StartupEventArgs e)
{  
  IServiceCollection _services = new ServiceCollection();
 
  // Because ServiceCollection registration members return the current ServiceCollection instance
  // you can chain registrations       
  _services.AddSingleton<IHomeViewModel, HomeViewModel>()
    .AddSingleton<INavigationService, NavigationService>()

    /* Register the factory delegates */
    .AddSingleton<Func<IHomeViewModel>>(serviceProvider => serviceProvider.GetRequiredService<HomeViewModel>)
    .AddSingleton<Func<INavigationService>>(serviceProvider => serviceProvider.GetRequiredService<NavigationService>);
}

Solution 2): Fix the class design/responsibilities (recommended)

Every class should know the navigation destinations it is allowed to navigate to. No class should know about an other class where it can navigate to or if it can navigate at all.

Opposed to solution 1), the circular dependency is completely lifted.

NavigationService.cs

public class NavigationService : INavigationService, INotifyPropertyChanged
{
  // The critical knowledge of particular types is now removed
  public NavigationService()
  {}

  // Every class that wants to navigate to a destination 
  // must know/provide his destination explicitly
  public void Navigate(object navigationDestination) 
    => this.CurrentSource = navigationDestination;

  public object CurrentSource { get; private set; }
}

HomeViewModel.cs

class HomeViewModel : IHomeViewModel, INotifyPropertyChanged
{
  private INavigationService NavigationService { get; }

  // Constructor
  public HomeViewModel(INavigationService navigationService)
    => this.NavigationService = navigationService;
}

Solution 3): Property injection

Property injection is not supported by the .NET dependency injection framework. However, property injection is generally not recommended. Aside from hiding dependencies, it imposes the danger of accidentally making a bad class design work instead of fixing what really needed to be fixed (as it would be the case with this example).

---

While 2) is the recommended solution, you can combine both solutions 1) and 2) and decide how much the particular navigation source needs to know about destinations.

public class NavigationService : INavigationService, INotifyPropertyChanged
{
  public Navigator(Func<IHomeViewModel> homeViewModelFactory)
  {
    this.HomeViewModelFactory = homeViewModelFactory;

    // This reveals that the class knows too much.
    // To introduce more destinations, 
    // you will always have to modify this code.
    // Same applies to your switch-statement.
    // A switch-statement is another good indicator 
    // for breaking the Open-Closed principle
    this.NavigationDestinationFactoryTable = new Dictionary<NavigationId, Func<object>>()
    {
      { NavigationId.HomeScreen, homeViewModelFactory }
    };
  }

  public void Navigate(NavigationId navigationId)
    => this.CurrentSource = this.NavigationDestinationTable.TryGetValue(navigationId, out Func<object> factory) ? factory.Invoke() : default;

  public void Navigate(object navigationDestination)
    => this.CurrentSource = navigationDestination;

  public object CurrentSource { get; private set; }
  public Func<IHomeViewModel> HomeViewModelFactory { get; }
  private Dictionary<NavigationId, Func<object>> NavigationDestinationFactoryTable { get; }
}

Then improve the MainViewModel initialization and clean up its dependencies.
It should use the NavigationService instead of explicit assignment.
That's why it got an NavigationService injected:

MainViewModel.cs

public class MainViewModel : INotifyPropertyChanged
{
  // Must be read-only
  public INavigationService NavigationService { get; }
 
  /* Commands. Must be read-only too */
  public RelayCommand SettingsViewCommand { get; }
  public RelayCommand HomeViewCommand { get; }

  public MainViewModel(INavigationService navigationService)
  {
    this.NavigationService = navigationService;
        
    this.HomeViewCommand = new RelayCommand(
      o => true, 
      o => this.NavigationService.Navigate(NavigationId.HomeScreen));
    this.SettingsViewCommand = new RelayCommand(
      o => true, 
      o => this.NavigationService.Navigate(NavigationId.SettingsScreen));
  }
}

Answer 3

This is a design issue. The main view model tightly coupled, is acting as a pass through and violating SRP (Single responsibility principle). The navigation service and other view models both explicitly depend on each other, which is the direct cause of the circular dependency issue.

For simplicity, note the following refactor for the NavService

public abstract class ViewModel : ObservableObject {

}

public interface INavigationService {
    object CurrentView { get; }
    void NavigateTo<T>() where T : ViewModel;
}

public class NavService : INavigationService, ObservableObject {
    private readonly Func<Type, object> factory;
    private object _currentView;
    
    public NavService(Func<Type, object> factory) {
        this.factory = factory;
    }
    
    public object CurrentView {
        get => _currentView;
        private set {
            _currentView = value;
            OnPropertyChanged();
        }
    }
    
    public void NavigateTo<T>() where T: ViewModel {
        object viewModel = factory.Invoke(typeof(T)) 
            ?? throw new InvalidOperationException("Error message here");
        CurrentView = viewModel;
    }
}

This service when registered should configure the factory used to get the view models.

public partial class App : Application {
    private readonly IServiceProvider _serviceProvider;
    public App() {
        IServiceCollection _services = new ServiceCollection();
        
        _services.AddSingleton<MainViewModel>();
        _services.AddSingleton<HomeViewModel>();
        _services.AddSingleton<SettingsViewModel>();
        
        _services.AddSingleton<DataService>();
        _services.AddSingleton<INavigationService, NavService>()(sp => {
            return new NavService(type => sp.GetRequiredService(type));
        });
        
        _services.AddSingleton<MainWindow>(o => new MainWindow {
            DataContext = o.GetRequiredService<MainViewModel>()
        });
        _serviceProvider = _services.BuildServiceProvider();
    }
    protected override void OnStartup(StartupEventArgs e) {
        var mainWindow = _serviceProvider.GetRequiredService<MainWindow>();
        mainWindow.Show();
        base.OnStartup(e);
    }
}

This completely decouples the main and other view models but allows for strongly typed navigation and removes the circular dependency since in this particular scenario the models only need know about the navigation service

public class MainViewModel : ViewModel {
    private INavigationService _navService;
    
    /* Ctor */
    public MainViewModel(INavigationService navService) {
        NavService = navService;
        HomeViewCommand = new RelayCommand(o => true, o => { NavService.NavigateTo<HomeViewModel>(); });
        SettingsViewCommand = new RelayCommand(o => true, o => { NavService.NavigateTo<SettingsViewModel(); });
    }
    
    public INavigationService NavService {
        get => _navService;
        set {
            _navService = value;
            OnPropertyChanged();
        }
    }
    /* Commands */
    public RelayCommand SettingsViewCommand { get; set; }
    public RelayCommand HomeViewCommand { get; set; }

}

Note that no changes were needed in the views. The navigation service is also now flexible enough to allow any number of view models to be introduced into the system with no changes needed to be made to it.