Base controller implementation in ASP.NET MVC 2

Question

I'm using ASP.NET MVC 2. I got the sample application from Darin (the guy that seems to answer all the MVC questions). I'm not sure if any one else has this sample project that he created? He has a base controller in his application, and the code looks like this:

public abstract class BaseController<TRepository> : Controller, IModelMapperController
{
   protected BaseController(TRepository repository, IMapper mapper)
   {
      Repository = repository;
      ModelMapper = mapper;
   }

   public TRepository Repository { get; private set; }
   public IMapper ModelMapper { get; private set; }
}

The questions that I have regarding this code is what does BaseController mean?

If I need to specify more than one repository how would the BaseController constructor code look like?

Is it best practices to have only one repository per controller? The reason why I ask is because in my ApplicationController class I make use of a Create() action method to populate my view from custom view model. In this view model I need to populate 2 different drop down lists. Bank names and account types. Each has their own repository. BankRepository has a method called GetBanks() and AccountTypeRepository has a method called GetAccountTpes(). So in my Application controller I have to call these 2 methods to populate the drop downs. So the Application controllers implements Base controller. How would base controller look like if I have to pass it more than one repository?

Please can someone shed some light regarding this.

@Darin: Thanks for the sample application, I am learning a lot from it already.

Answer 1

so each of your controllers could point to a different repo. something like this:

public class UsersController : BaseController<UserRepository>
{
    public UsersController() 
        : base(new UserRepository(), new UserModelMapper())
    {
        // do stuff
    }

    public ActionResult Index()
    {
        // now you can use syntax like, since "Repository" is type "UserRepository"
        return View(Respository.GetAllUsers());
    }

    public ActionResult Details(int id)
    {
        return View(Respository.GetUser(id));
    }
}

UPDATED for Addresses

public class AddressesController : BaseController<AddressRepository>
{
    public AddressesController() 
        : base(new AddressRepository(), new AddressModelMapper())
    {
    }

    public ActionResult Addresses(int id)
    {
        return View(Respository.GetAllByUserId(id));
    }
}

UPDATED for Factory

public static class RepositoryFactory
{
    internal static Hashtable Repositories = new Hashtable();

    internal static T GetRepository<T>() where T : class, new()
    {
        if (Repositories[typeof(T)] as T == null)
        {
            Repositories[typeof(T)] = new T();
        }
        return Repositories[typeof(T)] as T;
    }

    public static AccountTypeRepository AccountTypeRepository
    {
        get { return GetRepository<AccountTypeRepository>(); } 
    }

    public static BankRepository BankRepository
    {
        get { return GetRepository<BankRepository>(); } 
    }
    /* repeat as needed or change accessibility and call GetRepository<> directly */

Now rather than even using a BaseController you could just write this:

public class ApplicationModel
{
    public Application Application { get; set; }
    public IList<Bank> Banks { get; set; }
    public IList<AccountType> AccountTypes { get; set; }
}

public class ApplicationController : Controller
{
    public ActionResult Index()
    {
        ApplicationListModel model = new ApplicationListModel()
        {
            Applications = RespositoryFactory.ApplicationRepository.GetAll();
        }
        return View(model);
    }

    public ActionResult Details(int id)
    {
        ApplicationModel model = new ApplicationModel()
        {
            Application = RespositoryFactory.ApplicationRepository.Get(id),
            Banks = RespositoryFactory.BankRepository.GetAll(),
            AccountTypes = RespositoryFactory.AccountTypeRepository.GetAll()
        }
        return View(model);

    }
}

Answer 2

I'm not sure if I'll answer all of your questions, but here goes...

I use a BaseController as well, but I don't do what your example does. Here's how my code looks like (my application also uses DI for the constructors...):

public class BaseController : Controller {
    private readonly IProvider AddressProvider = null;
    private readonly IProvider EmailProvider = null;
    private readonly IProvider PhoneProvider = null;

    [Inject] // Using Ninject for DI
    public BaseController(
        AddressProvider AddressProvider,
        EmailProvider EmailProvider,
        PhoneProvider PhoneProvider) {
        this.AddressProvider = AddressProvider;
        this.EmailProvider = EmailProvider;
        this.PhoneProvider = PhoneProvider;
    }
}

And here's my AdministrationController which inherits from BaseController:

public class AdministrationController : BaseController {
    private readonly CustomerProvider CustomerProvider = null;
    private readonly EmployeeProvider EmployeeProvider = null;

    [Inject]
    public AdministrationController(
        CustomerProvider CustomerProvider,
        EmployeeProvider EmployeeProvider,
        AddressProvider AddressProvider,
        EmailProvider EmailProvider,
        PhoneProvider PhoneProvider) : base(AddressProvider, EmailProvider, PhoneProvider) {
        this.CustomerProvider = CustomerProvider;
        this.EmployeeProvider = EmployeeProvider;
    }
}

My AdministrationController only cares about CustomerProvider and EmployeeProvider and it passes AddressProvider, EmailProvider and PhoneProvider to the BaseController.

AddressProvider, EmailProvider and PhoneProvider are in the BaseController because I consider Address, Email and Phone to be low-level objects. My reason for that is because they can be linked to Customer, Employee or anything else as far as the database is concerned. So, instead of having multiple methods for Customer or Employee to interact with each of their objects, I just have one. For example:

public class BaseController : Controller {
    //  GET: /Addresses/{AddressId}/Delete
    public void DeleteAddress(
        int AddressId) {
        this.AddressProvider.DeleteAndSave(AddressId);

        Response.Redirect(Request.UrlReferrer.AbsoluteUri);
    }

    //  GET: /Emails/{EmailId}/Delete
    public void DeleteEmail(
        int EmaildId) {
        this.EmailProvider.DeleteAndSave(EmailId);

        Response.Redirect(Request.UrlReferrer.AbsoluteUri);
    }

    //  GET: /Phones/{PhoneId}/Delete
    public void DeletePhone(
        int PhoneId) {
        this.PhoneProvider.DeleteAndSave(PhoneId);

        Response.Redirect(Request.UrlReferrer.AbsoluteUri);
    }
}

And with that I take care of my low-level objects. Keep in mind, in my application I have additional methods to further manipulate those objects as needed.

Now, in my AdministrationController I'm working with CustomerProvider and EmployeeProvider. These are more specialized because I consider Customer and Employee to be high-level objects. That being said, their providers do a bit more work than Delete. For example they also provide the view models used by the views (durp...):

public class AdministrationController : BaseController {
    public ActionResult Customer(
        int CustomerId) {
        return this.View(this.CustomerProvider.GetView(CustomerId));
    }

    public AciontResult Customers() {
        return this.Veiw(this.CustomerProvider.GetAllView(CustomerId));
    }

    public ActionResult CustomerAddresses(
        int CustomerId,
        Address Address) {
        if (ModelState.IsValid) {
            this.CustomerProvider.AddAddressAndSave(CustomerId, Address);
        };

        return this.RedirectToAction("Customer", new {
            CustomerId = CustomerId
        });
    }

    public ActionResult Employee(
        int EmployeeId) {
        return this.View(this.EmployeeProvider.GetView(EmployeeId));
    }

    public ActionResult Employees() {
        return this.View(this.EmployeeProvider.GetAllView());
        //  OR
        //  return this.View(this.EmployeeProvider.GetActiveView());
        //  OR
        //  return this.Veiw(this.EmployeeProvider.GetInactiveView());
        //  ETC...
        //  All of these return the exact same object, just filled with different data
    }

    public RedirectToRouteResult EmployeeAddresses(
        int EmployeeId,
        Address Address) {
        if (ModelState.IsValid) {
            this.EmployeeProvider.AddAddressAndSave(EmployeeId, Address);
            //  I also have AddAddress in case I want to queue up a couple of tasks
            //  before I commit all changes to the data context.
        };

        return this.RedirectToAction("Employee", new {
            EmployeeId = EmployeeId
        });
    }
}

Is it best practices to have only one repository per controller?

I'm going to say no because your repositories will only work for the object they're instanced for. You can't have (well, you can, but that's just bad...) a repository that handles an Address, Email and Phone all at once because you'll have to specialize it just for it to work the way you need it.

My AddressProvider, EmailProvider and PhoneProvider are all essentially the same because they implement IProvider, however they each instance a generic repository (Repository<T>) for the object they're working with.

Furthermore, you're controller shouldn't be interacting with the repositories directly, but indirectly through the providers.

My CustomerProvider and EmployeeProvider each instance specialized repositories for Customer and Employee (CustomerRepository, EmployeeRepository), but they also instance other repositories they'll need when they for example construct the view models. For example, they'll instance a StateRepository which is Repository<State> or PhoneTypesRepository which is Repository<PhoneType>, and they'll use those repositories to pass additional objects/collections to the view to build up forms with drop downs or whatever. They'll also instance other providers to further help with building the view model such as CookieProvider which they use to get the currently active Cookie and pass it to the view model.

All in all it's a mesh of independent/generic providers or repositories which are combined to accomplish a specialized task.

I hope this sheds some light for you through an alternative way of writing code, or at the least I hope it just helps you understand a little bit better.

P.S. In case you're wondering what Provider is, most all other people choose to call them Service, but to me that word is misused, so I just call them Providers because they provide the controller with specialized functions or data as needed.

Answer 3

Well, it seems like the sample mentioned is somewhat special case or its authors taste.

I'd advise to not comlicate things around and avoid such a layer supertype as a Base Controller unless it is clearly necessary. This approach lets you define as much Repositories (and other dependencies) per Controller as you like without forcing you to use special-case BaseController supertype.

The other advice is when you need some common logic between your Controllers prefer Composition over Inheritance.

So, to summarize - yes, it is normal to have different repositories injected in your Controller if that is your case and for me it seems as the better approach than some kind of unnecessary BaseController.