Circular Dependency with two depending Services

Question

I'm very new in C# and Dependency Injection. Currently I'm working on a new project and want to do a technology step forward.

In this Project, I've three situation causing circular dependency.

I've read a lot about this and found solutions like Lazy<T> and and IServiceProvider, but I want to learn a clean solution for this problem and want to follow the most common suggestion to refactor the code.

We have four services in this example:

AccountService -> Login, Logout and so on

HttpService -> Do the API-Stuff

LogService -> Do some logging

LogRepository -> CRUD for the logging table / wrapper for EF

The AccountService authenticate via API using HttpService. Later, I want use the HttpService to get more data via API. HttpService now need the AccountService to get the Token for authenticate the request. This is causing a circular dependency error.

AccountService

public interface IAccountService
{
    Identity Identity { get; }
    Task Login(Credentials Credentials);
    Task Logout();
}

public class AccountService : IAccountService
{
    public Identity Identity { get; private set; }
    
    private readonly IHttpService _httpService;
    private readonly ILogService _logService;
    
    public AccountService(
        IHttpService HttpService, ILogService LogService)
    {
        _httpService = HttpService;
        _logService = LogService;
    }

    public async Task Login(Credentials Credentials)
    {
        Identity = await _httpService.Post<Identity>(
            "api/rest/v1/user/authenticate", Credentials);
    }
}

HttpService

public interface IHttpService
{
    Task<T> Get<T>(string uri);
    Task Post(string uri, object value);
    Task<T> Post<T>(string uri, object value);
}

public class HttpService : IHttpService
{
    private readonly HttpClient _httpClient;
    private readonly IAccountService _accountService;
    private readonly ILogService _logService; 

    public HttpService(
        HttpClient HttpClient,
        IAccountService AccountService,
        ILogService ILogService)
    {
        _httpClient = HttpClient;
        _accountService = AccountService;
        _logService = LogService;
    }

    private async Task AddAuthentication(HttpRequestMessage Request)
    {
        Request.Headers.Authorization = new AuthenticationHeaderValue(
            "bearer", _accountService.Identity.SystemToken);
    }
}

How is the best practice to solve or proper redesign this?

I've more Circular Dependency, e.g. use the LogService in LogRepository or using LogService in HttpService (because the HttpService sends Log-Entrys to the Server).

Thank you so much for your help!

Answer 1

An object graph where there is a cycle can't be satisfied by your DI Container, just like you would be unable to express this in plain old C#:

new AccountService(
    new HttpService(
        new AccountService(
            new HttpService(
                new AccountService(
                    new HttpService(
                        new AccountService(
                            new HttpService(
                                ... this would go on for ever     

Although your object graph is cyclic (AccountService -> HttpService -> AccountService) your call graph is not. The call likely is something as follows:

AccountService.Login
    -> HttpService.Post
        -> HttpService.AddAuthentication
            -> AccountService.Identity

Cyclic object graphs with non-cyclic call graphs often happen on components that violate the Single Responsibly Principle. The more functionality (the more methods) classes get, the bigger the chance their object graphs become cyclic. Splitting classes up in smaller, more-focused pieces, not only fixes the cyclic-dependency problem, but often also improves the maintainability of the application.

I think your case is actually quite similar to the example that I discuss in section 6.3 of DIPP&P. That section specifically talks about fixing cyclic dependencies.

Long story short, I think your best bet is to split AccountService in (at least) two services:

• One service responsible of logging in and logging out
• A second service responsible of getting the user's identity.

Both services get their own interface and those new interfaces are now less wide compared to IAccountService. This improves your chances of adhering to the Interface Segregation Principle.

Here's an example of how that looks like:

Let's start with the new interface definitions:

// Contains Login and Logout methods of old IAccountService
public interface IAuthenticationService
{
    Task Login(Credentials Credentials);
    Task Logout();
}

// Contains Identity property of old IAccountService
public interface IIdentityProvider
{
    // For simplicity I added a setter to the interface, because that keeps
    // the example simple, but it is possible to keep Identity read-only if
    // required.
    Identity Identity { get; set; }
}

// This interface is kept unchanged.
public interface IHttpService
{
    Task<T> Get<T>(string uri);
    Task Post(string uri, object value);
    Task<T> Post<T>(string uri, object value);
}

Let's look at the implementations next, starting with the IAuthenticationService implementation:

// Old AccountService, now depending on IIdentityProvider
public class AuthenticationService : IAuthenticationService
{
    private readonly IHttpService _httpService;
    private readonly ILogService _logService;
    private readonly IIdentityProvider _identityProvider;
    
    public AccountService(
        IHttpService HttpService,
        ILogService LogService,
        IIdentityProvider IdentityProvider)
    {
        _httpService = HttpService;
        _logService = LogService;
        _identityProvider = IdentityProvider;
    }

    public async Task Login(Credentials Credentials)
    {
        _identityProvider.Identity = await _httpService.Post<Identity>(
            "api/rest/v1/user/authenticate", Credentials);
    }
}

This "new" AuthenticationService contains part of the code of the AccountService and the rest of the old AccountService logic is hidden behind the new IIdentityProvider abstraction, which is injected into AuthenticationService. This refactoring is very similar to the Facade Service refactoring (for an elaborate discussion on the Facade Service refactoring, see section 6.1 of DIPP&P).

IdentityProvider implements the new IIdentityProvider interface and contains the old logic from AccountService:

public class IdentityProvider : IIdentityProvider
{
    public Identity Identity { get; set; }
}

And finally, HttpService that now depends on IIdentityProvider instead of IAccountService:

// Now depends on IIdentityProvider instead of IAccountService
public class HttpService : IHttpService
{
    private readonly HttpClient _httpClient;
    private readonly IIdentityProvider _identityProvider;
    private readonly ILogService _logService; 

    public HttpService(
        HttpClient HttpClient,
        IIdentityProvider IdentityProvider,
        ILogService ILogService)
    {
        _httpClient = HttpClient;
        _identityProvider = IdentityProvider;
        _logService = LogService;
    }

    private async Task AddAuthentication(HttpRequestMessage Request)
    {
        // Now uses the new IIdentityProvider dependency instead
        // of the old IAccountService, which caused the cycle.
        Request.Headers.Authorization = new AuthenticationHeaderValue(
            "bearer", _identityProvider.Identity.SystemToken);
    }
}

Using this new design, the object graph is no longer cyclic and, when constructed in plain C#, can be constructed as follows:

var identity = new IdentityProvider();
var logger = new LogService();

new AccountService(
    new HttpService(
        new HttpClient(...),
        identity,
        logger),
    logger,
    identity);