Some very obvious problems with the idea of a generic IRepository<T>:
It assumes that every entity uses the same type of key, which is not true in almost any non-trivial system. Some entities will use GUIDs, others may have some kind of natural and/or composite key. NHibernate can support this fairly well but Linq to SQL is pretty bad at it - you have to write a good deal of hackish code to do automatic key mapping.
It means that each repository can only deal with exactly one entity type and only supports the most trivial operations. When a repository is relegated to such a simple CRUD wrapper it has very little use at all. You might as well just hand the client an IQueryable<T> or Table<T>.
It assumes that that you perform exactly the same operations on every entity. In reality this is going to be very far from the truth. Sure, maybe you want to get that Order by its ID, but more likely you want to get a list of Order objects for a specific customer and within some date range. The notion of a totally generic IRepository<T> doesn't allow for the fact that you'll almost certainly want to perform different types of queries on different types of entities.
The whole point of the repository pattern is to create an abstraction over common data access patterns. I think some programmers get bored with creating repositories so they say "Hey, I know, I'll create one über-repository that can handle any entity type!" Which is great except that the repository is pretty much useless for 80% of what you're trying to do. It's fine as a base class/interface, but if that's the full extent of the work that you do then you're just being lazy (and guaranteeing future headaches).
Ideally I might start with a generic repository that looks something like this:
public interface IRepository<TKey, TEntity>
{
TEntity Get(TKey id);
void Save(TEntity entity);
}
You'll notice that this doesn't have a List or GetAll function - that's because it's absurd to think that it's acceptable to retrieve the data from an entire table at once anywhere in the code. This is when you need to start going into specific repositories:
public interface IOrderRepository : IRepository<int, Order>
{
IEnumerable<Order> GetOrdersByCustomer(Guid customerID);
IPager<Order> GetOrdersByDate(DateTime fromDate, DateTime toDate);
IPager<Order> GetOrdersByProduct(int productID);
}
And so on - you get the idea. This way we have the "generic" repository for if we ever actually need the incredibly simplistic retrieve-by-id semantics, but in general we're never actually going to pass that around, certainly not to a controller class.
Now as for controllers, you have to do this right, otherwise you've pretty much negated all the work you just did in putting together all the repositories.
A controller needs to take its repository from the outside world. The reason you created these repositories is so you can do some kind of Inversion of Control. Your ultimate goal here is to be able to swap out one repository for another - for example, to do unit testing, or if you decide to switch from Linq to SQL to Entity Framework at some point in the future.
An example of this principle is:
public class OrderController : Controller
{
public OrderController(IOrderRepository orderRepository)
{
if (orderRepository == null)
throw new ArgumentNullException("orderRepository");
this.OrderRepository = orderRepository;
}
public ActionResult List(DateTime fromDate, DateTime toDate) { ... }
// More actions
public IOrderRepository OrderRepository { get; set; }
}
In other words the Controller has no idea how to create a repository, nor should it. If you have any repository-construction going on in there, it's creating coupling that you really don't want. The reason that the ASP.NET MVC sample controllers have parameterless constructors that creates concrete repositories is that the sites need to be able to compile and run without forcing you to set up an entire Dependency Injection framework.
But in a production site, if you aren't passing in the repository dependency through a constructor or public property, then you're wasting your time having repositories at all, because the controllers are still tightly coupled to the database layer. You need to be able to write test code like this:
[TestMethod]
public void Can_add_order()
{
OrderController controller = new OrderController();
FakeOrderRepository fakeRepository = new FakeOrderRepository();
controller.OrderRepository = fakeRepository; //<-- Important!
controller.SubmitOrder(...);
Assert.That(fakeRepository.ContainsOrder(...));
}
You can't do this if your OrderController is going off and creating its own repository. This test method isn't supposed to do any data access, it just makes sure that the controller is invoking the correct repository method based on the action.
This isn't DI yet, mind you, this is just faking/mocking. Where DI comes into the picture is when you decide that Linq to SQL isn't doing enough for you and you really want the HQL in NHibernate, but it's going to take you 3 months to port everything over, and you want to be able to do this one repository at a time. So, for example, using a DI Framework like Ninject, all you have to do is change this:
Bind<ICustomerRepository>().To<LinqToSqlCustomerRepository>();
Bind<IOrderRepository>().To<LinqToSqlOrderRepository>();
Bind<IProductRepository>().To<LinqToSqlProductRepository>();
To:
Bind<ICustomerRepository>().To<LinqToSqlCustomerRepository>();
Bind<IOrderRepository>().To<NHibernateOrderRepository>();
Bind<IProductRepository>().To<NHibernateProductRepository>();
And there you are, now everything that depends on IOrderRepository is using the NHibernate version, you've only had to change one line of code as opposed to potentially hundreds of lines. And we're running the Linq to SQL and NHibernate versions side by side, porting functionality over piece by piece without ever breaking anything in the middle.
So to summarize all the points I've made:
Don't rely strictly on a generic IRepository<T> interface. Most of the functionality you want from a repository is specific, not generic. If you want to include an IRepository<T> at the upper levels of the class/interface hierarchy, that's fine, but controllers should depend on specific repositories so you don't end up having to change your code in 5 different places when you find that the generic repository is missing important methods.
Controllers should accept repositories from the outside, not create their own. This is an important step in eliminating coupling and improving testability.
Normally you'll want to wire up the controllers using a Dependency Injection framework, and many of them can be seamlessly integrated with ASP.NET MVC. If that's too much for you to take in, then at the very least you should be using some kind of static service provider so you can centralize all of the repository-creation logic. (In the long run, you'll probably find it easier to just learn and use a DI framework).
