How to deal with too many interfaces

Question

I am looking at updating some of my companies existing code to allow unit tests to be implemented. To be able to do this, all repositories are being interfaced to allow DI. However, the existing code has a habit of creating 1 repository class per table in the database, meaning that one section of business logic may need to query 5 different repositories classes.

Passing in 5+ interfaces using DI seems messy and I am looking for help on how to redesign this.

This is all for an MVC 5 C# site, and an example of it is below (I have reduced the code size so that it is easier to read).

We have our controller for Account, which in this scenario manages the user sessions:

public class AccountController : Controller
{
    private AccountLogic _accountLogic { get; set; }

    public AccountController(
        ISessionRepository sessionRepository, 
        ILoginRepository loginRepository)
    {
        _accountLogic = new AccountLogic(sessionRepository, loginRepository);
    }

    public IActionResult Index()
    {
        return View();
    }

    public IActionResult Login()
    {
        return View();
    }

    public IActionResult Login(LoginModel loginModel)
    {
        if (ModelState.IsValid)
        {
            if (_accountLogic.DoesUserExist(loginModel.Username))
            {
                var existingHash= _accountLogic.GetPassword(loginModel.Username);

                if (Hash.VerifyPassword(loginModel.Password, existingHash))
                {
                    // Set Auth Cookies

                    return RedirectToAction("Index", "Home");
                }
                else
                {
                    ModelState.AddModelError(
                        "Username", 
                        "Sorry, we did not recognize " +
                            "either your username or password");
                }
            }
        }
        return View(loginModel);
    }
}

The Team tries to reduce the size of the controller by having a business class perform most of the actions. What this actually means is that the business class is very large and has many responsibilities as it has to be passed an interface for all of the repositories it has to query. In the example shown, we pass in both the Session and Login repositories, each of this is solely responsible for their corresponding tables in the database.

public class AccountLogic
{
    private ISessionRepository _sessionRepository { get; set; }
    private ILoginRepository _loginRepository { get; set; }

    public AccountLogic(
        ISessionRepository sessionRepository, 
        ILoginRepository loginRepository)
    {
        _sessionRepository = sessionRepository;
        _loginRepository = loginRepository;
    }

    public Guid CreateSession(int loginID, string userAgent, string locale)
    {
        return _sessionRepository.CreateSession(loginID, userAgent, locale);
    }

    public Session GetSession(Guid sessionKey)
    {
        return _sessionRepository.GetSession(sessionKey);
    }

    public void EndSession(Guid sessionKey)
    {
        _sessionRepository.EndSession(sessionKey);
    }

    public bool DoesUserExist(string username)
    {
        return _loginRepository.DoesUserExist(username);
    }

    public string GetPassword(string username)
    {
        return _loginRepository.GetPassword(username);
    }
}

We then have the AccountLogic class, which handles all business logic. In the example given, there is no complicated logic to be performed, so it becomes purely a chain call to the interface methods.

My question is, how do we make this easier to handle will also being testable on a larger scale? If I had a controller which needs to query 10 different repositories, surely it is not manageable to pass 15 interfaces to the controller, and then to a relevant business class? Do we need to change how repositories are handled so that there is not one class per table?

Answer 1

While not entirely in the style of C#, free monad API can help you here. Here's a C# example.

In a free monad API, your interface is modeled as objects:

// Static DSL for composing DbOp
public static class DB {
  public static class Session {
    public static DbOp<Guid> Create(int loginID, string userAgent, string locale) => new DbOp<Guid>.Session.Create(loginID, userAgent, locale, Return);
  }
  public static class Login {
    public static DbOp<Boolean> DoesUserExist(string username) => new DbOp<Boolean>.Login.DoesUserExist(username, Return);
  }
  public static DbOp<A> Return<A>(A value) => new DbOp<A>.Return(value);
}

// The classes that are created by the DSL. Holds the data of each operation
public abstract class DbOp<A> {
  public static class Session {
    public class Create : DbOp<A> {
      public readonly int LoginID;
      public readonly string UserAgent;
      public readonly string Locale;
      public readonly Func<Unit, DbOp<A>> Next;
      public Create(int loginID, string userAgent, string locale, Func<Unit, DbOp<A>> next) =>
        (LoginID, UserAgent, Locale, Next) = (loginID, userAgent, locale, next);
    }
  }

  public static class Login {
    public class DoesUserExist {
      public readonly string Username;
      public readonly Func<Unit, DbOp<A>> Next;
      public DoesUserExist(string username, Func<Unit, DbOp<A>> next) =>
        (Username, Next) = (username, next);
    }
  }

  public class Return : DbOp<A> {
    public readonly A Value;
    public Return(A value) => Value = value;
  }
}

This allows you to represent your entire data interface uniformly in a single file while allowing complete separation of the interface's implementation. You write programs by composing your interface objects in a free monad:

public class BusinessLogic {
  private readonly IDbInterpreter dbInterpreter;
  public BusinessLogic(IDbInterpreter dbInterpreter) {
    this.dbInterpreter = dbInterpreter;
  }

  public Guid login(username) = {
    // Compose our program of DbOp
    DbOp<Guid> program = 
      from userExists     in DB.Login.DoesUserExist(username)
      from guid           in DB.Session.Create(...)
      select guid;

    // Interpret the program to get the result
    dbInterpreter.Interpret(program);
  }
}

Interpreters look like big switch statements (pseudocode):

interface IDbInterpreter {
  A Interpret<A>(DbOp<A> ma);
}

public class DbInterpreter : IDbInterpreter {
    private readonly LoginRepository loginRepo;
    private readonly SessionRepository sessionRepo;
    DbInterpreter(LoginRepository loginRepo, 
                  SessionRepository sessionRepo) {
      this.loginRepo = loginRepo;
      this.sessionRepo = sessionRepo;
    }

    A IDbInterpreter.Interpret<A>(DbOp<A> ma) =>
      ma is DbOp<A>.Return r                ? r.Value
      : ma is DbOp<A>.Session.Create c      ? Interpret(c.Next(sessionRepo.Create(c.LoginID, c.UserAgent, c.Locale)))
      : ma is DbOp<A>.Login.DoesUserExist d ? Interpret(d.Next(loginRepo.DoesUserExist(d.Username)))
      : throw new NotSupportedException();
}

One of the benefits of using this pattern is that you can separate your DB tables separately in the system, but you can interpret them all as one DSL. That means in test you can provide a single mock interpreter to handle all your expected DB calls (and throw an exception if an unexpected call is made) (pseudocode):

class MockInterpreter : IDbInterpreter {
  A IDbInterpreter.Interpret(DbOp<A> ma) =>
    ma is DbOp<A>.Return r                ? r.Value
    : ma is DbOp<A>.Session.Create c      ? throw new Exception("") // simulate exception being thrown on session create
    // we only need to implement the operations we expect to be called in the mock interpreter
    : throw new NotSupportedException(); 
}
BusinessLogic businessLogic = new BusinessLogic(mockInterpreter);
assert businessLogic.login(username) ...

Instead of providing numerous interface mocks.

Answer 2

Have you tried injecting the concrete class 'AccountLogic' into 'AccountController'. Your IOC container should be able to build a concrete class without any added configuration.

public class AccountController : Controller
{
    private readonly AccountLogic _accountLogic;

    public AccountController(AccountLogic accountLogic)
    {
        _accountLogic = accountLogic;
    }
    ...
}

You can Mock a concrete class for testing by adding an empty constructor and changing the methods to 'virtual'.

public class AccountLogic
{
    private readonly ISessionRepository _sessionRepository;
    private readonly ILoginRepository _loginRepository;

    public AccountLogic() { }

    public AccountLogic(
        ISessionRepository sessionRepository, 
        ILoginRepository loginRepository)
    {
        _sessionRepository = sessionRepository;
        _loginRepository = loginRepository;
    }

    public virtual Guid CreateSession(int loginID, string userAgent, string locale)
    {
        return _sessionRepository.CreateSession(loginID, userAgent, locale);
    }
    ...
}

+1 for aligning your constructor parameters vertically instead of horizontally

Also, use

private readonly ISessionRepository _sessionRepository;

instead of

private ISessionRepository _sessionRepository { get; set; }

for injected objects.