Home > Mobile >  How to fix a Circular Dependency
How to fix a Circular Dependency

Time:01-10

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.

CodePudding user response:

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.

I would advise against that because you will likely 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 passing in though.

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.

CodePudding user response:

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 types.

Circular dependencies are usually introduced by designing wrong responsibilities. IoC reveals this design flaw as dependencies are publicly exposed, usually in the constructor.

From a class level (module) design perspective,
when type A depends on B and B on A (AB)
then you have 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 (AB).
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 dependency is manifested in the constructor, introducing interfaces won't resolve the circular dependency

To fix your original problem you have three options:

  1. hide the 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:

NavigationId.cs

public enum NavigationId
{
  None = 0,
  HomeViewModel
}

1) Hide dependencies

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

Note, there is still an implicit circular dependency. It is just hidden behind factories. The dependency is just removed from the constructor (constructing a NavigationService no longer requires the construction of HomeViewModel and vice versa).
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
{
  private HomeViewModel HomeViewModel { get; }

  // 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<HomeViewModel> homeViewModelFactory)
    => this.HomeViewModel = homeViewModelFactory.Invoke();
}

HomeViewModel.cs

class HomeViewModel
{
  private NavigationService NavigationService { get; }

  // Constructor
  public HomeViewModel(Func<NavigationService> 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<HomeViewModel>()
    .AddSingleton<NavigationService>();
    .AddSingleton<Func<HomeViewModel>>(serviceProvider => serviceProvider.GetRequiredService<HomeViewModel>)
    .AddSingleton<Func<NavigationService>>(serviceProvider => serviceProvider.GetRequiredService<NavigationService>);
}

2) Fix the class design (responsibilities)

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.

public class Navigator : INavigator
{
  // The critical knowledge of particular types is now removed
  public Navigator()
  {}

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

  public object CurrentSource { get; set; }
}

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 making a bad class design work (as it would be the case in 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 Navigator : INavigator
{
  public Navigator(Func<HomeViewModel> 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.NavigationDestinationTable = new Dictionary<NavigationId, Func<object>>()
    {
      { NavigationId.HomeViewModel, 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; set; }
  public Func<HomeViewModel> HomeViewModelFactory { get; }
  internal Dictionary<NavigationId, Func<object>> NavigationDestinationTable { get; }
}

CodePudding user response:

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.

  • Related