3

Usually, I consider a large number of parameters as an alarm bell that there may be a design problem somewhere. I am using a Generic Repository for an ASP.NET application and have a Controller with a growing number of parameters.

public class GenericRepository<T> : IRepository<T> where T : class
{
    protected DbContext Context { get; set; }
    protected DbSet<T> DbSet { get; set; }

    public GenericRepository(DbContext context)
    {
        Context = context;
        DbSet = context.Set<T>();
    }
    ...//methods excluded to keep the question readable

}

I am using a DI container to pass in the DbContext to the generic repository. So far, this has met my needs and there are no other concrete implmentations of IRepository<T>. However, I had to create a dashboard which uses data from many Entities. There was also a form containing a couple of dropdown lists. Now using the generic repository this makes the parameter requirments grow quickly.

The Controller will end up being something like

public HomeController(IRepository<EntityOne> entityOneRepository,
                      IRepository<EntityTwo> entityTwoRepository,
                      IRepository<EntityThree> entityThreeRepository,
                      IRepository<EntityFour> entityFourRepository,
                      ILogError logError,
                      ICurrentUser currentUser)
{
}

It has about 6 IRepositories plus a few others to include the required data and the dropdown list options. In my mind this is too many parameters. From a performance point of view, there is only 1 DBContext per request and the DI container will serve the same DbContext to all of the Repositories. From a code standards/readability point of view it's ugly.

Is there a better way to handle this situation? Its a real world project with real world time constraints so I will not dwell on it too long, but from a learning perspective it would be good to see how such situations are handled by others.

  • Simole: not using generic repository. It is ugly pattern by itself. – Euphoric Aug 19 '14 at 9:56
  • @Euphoric Can you elaborate on how that helps to solve the problem of too many dependencies? – proskor Aug 19 '14 at 10:07
3

In my opinion a controller should only be a thin layer around the businesslogic.

If the controller needs several repositories that might be an indicator that the controller is doing too much and logic should be moved to servicelayers for seperation of concerns:

Instead of having

OrderController(productRepository, priceRepository, 
                  availabilityRepository, orderRepository, orderLineRepository)

i would have Services that know their own repository and subservices they use.

The Example above would become

priceCalculationService(productRepository, priceRepository)
orderService(orderRepository, orderLineRepository, 
          availabilityService, priceCalculationService)

OrderController(orderService)
  • This is a very good point, there could be a service layer in between my COntrollers and Repositories to try reduce this problem. – James Aug 20 '14 at 9:51
2

I've been playig around with using seperate query and command classes instead of using repositories with multiple methods.

Your UserRepository might a GetByEmail(string email) method. Instead of that, I have a GetUserByEmailQuery class and a GetUserByEmailQueryHandler class.

Your UserRepository might have a UpdatePassword(string newPassword) method. Instead of that, I have a UpdateUserPasswordCommand class and a UpdateUserPasswordCommandHandler class.

Everything comes together in my controllers through a QueryAndCommandDispatcher.

First some interfaces:

public interface IQueryHandler<in TQueryData, out TResult> where TQueryData : class
{
    TResult Execute(TQueryData query);
}

public interface ICommandHandler<in T> where T : class
{
    void Handle(T command);
}

public interface IQueryAndCommandDispatcher
{
    void ExecuteCommand<T>(T command) where T : class;
    TResult ExecuteQuery<TQueryData, TResult>(TQueryData query) where TQueryData : class; 
}

As you can see, a query is supposed to return something while a command isn't. A command should change data, a query shouldn't.

Here's how that query to get a user by his or her email would look like (IDataContext is an interface describing my custom DbContext):

public class GetUserByEmailQuery
{
    public string Email { get; private set; }

    public GetUserByEmailQuery(string email)
    {
        Email = email;
    }
}

public class GetUserByEmailQueryHandler : IQueryHandler<GetUserByEmailQuery, User>
{
    private readonly IDataContext dataContext;

    public GetUserByEmailQueryHandler(IDataContext dataContext)
    {
        this.dataContext = dataContext;
    }

    public User Execute(GetUserByEmailQuery query)
    {
        return dataContext.Users.First(u => u.Email == query.Email);
    }
}

Here's how that command to update a user's password would look like:

public class UpdateUserPasswordCommand
{
    public int UserId { get; private set; }
    public string Password { get; private set; }

    public UpdateUserPasswordCommand(int userId, string password)
    {
        UserId = userId;
        Password = password;
    }
}

public class UpdateUserPasswordCommandHandler : ICommandHandler<UpdateUserPasswordCommand>
{
    private readonly IDataContext dataContext;

    public UpdateUserPasswordCommandHandler(IDataContext dataContext)
    {
        this.dataContext = dataContext;
    }

    public void Handle(UpdateUserPasswordCommand command)
    {
        var user = dataContext.Users.Find(command.UserId);
        user.Password = command.Password;

        dataContext.SaveChanges();
    }
}

And here's the implementation of the IQueryAndCommandDispatcher interface:

public class QueryAndCommandDispatcher : IQueryAndCommandDispatcher
{
    private readonly IKernel kernel;

    public QueryAndCommandDispatcher(IKernel kernel)
    {
        this.kernel = kernel;
    }

    public void ExecuteCommand<T>(T command) where T : class
    {
        var handler = kernel.Get<ICommandHandler<T>>();

        handler.Handle(command);
    }

    public TResult ExecuteQuery<TQueryData, TResult>(TQueryData query) where TQueryData : class
    {
        var handler = kernel.Get<IQueryHandler<TQueryData, TResult>>();

        return handler.Execute(query);
    }
}

And everything comes together in a controller like this:

public class HomeController : Controller
{
    private readonly IQueryAndCommandDispatcher dispatcher;

    public HomeController(IQueryAndCommandDispatcher dispatcher)
    {
        this.dispatcher = dispatcher;
    }

    [HttpGet]
    public ActionResult Index(string email)
    {
        var userQuery = new GetUserByEmailQuery(email);
        var user = dispatcher.ExecuteQuery<GetUserByEmailQuery, User>(userQuery);

        return View(user);
    }

    [HttpPost]
    public ActionResult Update(int userId, string password)
    {
        var updateCommand = new UpdateUserPasswordCommand(userId, password);
        dispatcher.ExecuteCommand(updateCommand);

        return View();
    }
}

This reduces the number of dependencies your controllers need while still keeping everything testable.

  • I really like this approach. I will play with it when I have time and see if i like it when its implemented. Could be a good alternative to repositories. – James Aug 20 '14 at 9:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.