Simple Injector: Cyclic Graph Error

Question

Registry:

container.Register<IAuthenticationHandler, AuthenticationHandler>(Lifestyle.Transient);
container.Register<IUserHandler, UserHandler>(Lifestyle.Transient);    

Class 1:

public UserHandler(IAuthenticationHandler authenticationHandler)
{
    _authenticationHandler = authenticationHandler;
} 

---

Class 2:

public AuthenticationHandler(IUserHandler userHandler)
{
    _userHandler = userHandler;
}

---

I understand what the cyclic issue is. When the UserHandler is initialized, it is injecting the AuthenticationHandler implementation and then that is trying to create the UserHandler instance and the cycle begins...

My question is how do I solve this in (SIMPLE INJECTOR) this situation and others where I need to inject like this?

Thank you!

UPDATE:

function AddUser(User user){ // User Handler
    _authenticationHandler.GenerateRandomSalt();
    string hashedPassword = _authenticationHandler.HashPassword(user.Password.HashedPassword, salt);
}

function Authenticate(string username, string password){ // Authentication Handler
    _userHandler.GetUserByUsername(username?.Trim());
}

Bascially I need to call the UserHandler in the AuthenticationHandler to get the user and verify there is a user.

I need to call the AuthenticationHandler in the UserHandler to get functions to salt and hash the password.

I guess I could call the Repository to get the user but shouldn't I go through the service handler in case more stuff is done in the user service

Answer 1

Cyclic dependencies are often caused by SOLID principle violations, since classes with too broad interfaces and too much functionality have a much higher chance of needing each other's functionality.

I believe this is the case in your situation as well, since the UserHandler.AddUser functionality depends on the AuthenticationHandler.GenerateRandomSalt and HashPassword functionality, while it is different functionality from AuthenticationHandler (i.e. Authenticate) that depends on yet another function from UserHandler. This is a strong indication that IAuthenticationHandler abstraction actually violates the Interface Segregation Principle and its implementation violates the Single Responsibility Principle.

The solution is to split the IAuthenticationHandler and its implementation into multiple independent parts. For instance

interface IPasswordUtilities {
    // NOTE: I believe GenerateRandomSalt is an implementation detail;
    // it should not be part of the interface
    string HashPassword(string plainPassword);
}

interface IAuthenticationHandler {
    void Authenticate(string username, string password);
}

class PasswordUtilities : IPasswordUtilities {
    // implementation
}

class AuthenticationHandler : IAuthenticationHandler {
    public AuthenticationHandler(IUserHandler userHandler, IPasswordUtilities utilities) { 
        ... 
    }
}

class UserHandler : IUserHandler {
    public UserHandler(IPasswordUtilities utilities) { ... }

    public void AddUser(User user) {
        string hashedPassword = _utilities.HashPassword(user.Password.HashedPassword);
    }
}

This will solve your problem elegantly, because you:

• Remove the cyclic dependency by extracting part of the logic into a smaller more focused class
• You make your code base more maintainable by fixing both the SRP and ISP violations.

The end graph will look like this:

new AuthenticationHandler(
    new UserHandler(
        new PasswordUtilities()),
    new PasswordUtilities());

Answer 2

One way to handle this is to implement a way for one or the other to create an instance of its dependency.

An abstract factory for authentication handler can you be used here,

public interface IAuthenticationHandlerFactory
{
    IAuthenticationHandler Create(IUserHandler userHandler);
}

public class AuthenticationHandlerFactory : IAuthenticationHandlerFactory
{
    public IAuthenticationHandler Create(IUserHandler userHandler)
    {
        return new AuthenticationHandler(userHandler);
    }
}

and change the UserHandler to depend on the factory,

public class UserHandler : IUserHandler
{
    private IAuthenticationHandler _authenticationHandler;

    public UserHandler(IAuthenticationHandlerFactory authenticationHandler)
    {
        _authenticationHandler = authenticationHandler.Create(this);
    }
}

then register like usual in the container,

container.Register<IAuthenticationHandlerFactory, AuthenticationHandlerFactory>(Lifestyle.Singleton);

This absolutely does couple the concrete implementation of the factory to authentication handler. I'd keep this relatively simple so it doesn't go overboard on complication.

---

Another way to do this is with a delegate type. This way, you can keep the references to concrete implementations in the composition root.

The UserHandler class would be

public class UserHandler : IUserHandler
{
    private IAuthenticationHandler _authenticationHandler;

    public UserHandler(Func<IUserHandler, IAuthenticationHandler> authenticationHandler)
    {
        _authenticationHandler = authenticationHandler(this);
    }
}

and the registration for the Func<,> is

container.Register<Func<IUserHandler, IAuthenticationHandler>>(() => u => new AuthenticationHandler(u), Lifestyle.Singleton);