Repository pattern: One repository class for each entity?

Question

Say you have the following entities defined in a LINQ class:

Product
Customer
Category

Should I have one repository class for all:

StoreRepository

... or should I have:

ProductRepository
CustomerRepository
CategoryRepository

What are the pro & cons of each? In my case, I have several "applications" within my solution... the Store application is just one of them.

Stain

Answer 1

Here's my point of view. I'm a strict follower of the Repository pattern. There should be 3 methods that take a single entity. Add, Update, Delete, generically defined.

public interface IRepository<T>
{
     void Add(T entity);
     void Update(T entity);
     void Delete(T entity);
}

Beyond those methods, you're dealing with a "Query" or a service method. If I were you, I'd make the repository genrically defined as above, add a "QueryProvider" as shown below and put your business logic where it belongs in either "Services" or in "Commands/Queries" (comes from CQRS, Google it).

public interface IQueryProvider<T>
{
     TResult Query<TResult>(Func<IQueryable<T>, TResult> query);
}

(Hope my opinion is somewhat useful :) )

Borchert

What about Get() Entity? Where would that go in the repository? – Rabin

Following this, Get() would be a query with a single result. – Tshombe

@Necros, I agree. This could probably even be implemented using an extension method, if that's how you roll. – Borchert

@Borchert personally i like my repositories a bit richer. I roll like that: github.com/Necroskillz/NecroNetToolkit/tree/master/Source/… – Tshombe

@Tshombe I would contend that that approach isn't always better. All of the methods that you have in IRepository<T> could easily be put into extension methods. Also, your IRepository<T> also handles querying. While I've seen this a lot (even as Martin Fowler describes it), I like to avoid it for the simple fact that a query isn't a command, it's a query. The added benefit is, if you're running .NET 4.0, my IRepository is contra-variant and my IQueryProvider is co-variant. (IRepository<in T> and IQueryProvider<out T>) – Borchert

In my opinion, having a Query(q) method makes things more complicated to test. Why not have IRepository<T> : IQueryable<T>. It's very simple to implement if your underlying data provider supports the LINQ abstraction. Also, I'd remove Update(entity) and use transactions (at the web request or service method level) with persistence by reachability to save changes. Of course you should wrap your repository with a service layer and do your testing there. I would never expose an IQueryable<> to the MVC/controller layer. Also, I agree with Necros that IRepo should have a Get(id) method. – Travax

@Travax First, separating query from "repository functions" is an important concept. A repository is not a query layer, it's an object that manages objects in a backing store, which is completely ignorant to the consumers of the repository. Inheriting from IQueryable is a terrible design... you don't want instances of IQueryable floating around in memory because of things like session lifetimes (in NH for example). THAT is the reason why you add a query method that takes in a function, to avoid "leaking" IQueryable. Like I said, get can be extension. Not necessary in IQueryProvider interface!!! – Borchert

@Travax Repositories needs to not be aware of transactions. Think SINGLE RESPONSIBILITY PRINCIPLE. Update is fine in the repository. Taking Update out and putting it somewhere else would be really stupid. It BELONGS in the repository and notions of transactions do NOT belong in repositories, they belong in service layers. – Borchert

@Borchert Of course the repository shouldn't know about transactions. Mine don't. The underlying NHibernate/Linq To Sql implementation does. I handle transactions with a HTTP module. I don't follow your argument about leaking IQueryables. A repository and an in-memory list should be indistinguishable. It's still up the responsibility of the programmer to understand when a certain query would cause too much data to be loaded from the database. In my experience, writing a separate query object is a waste of time with no additional benefit. – Travax

@Travax Leaking IQueryables is doing something like return it from a property or have repository inherit from IQueryable. When you give up control of your IQueryable object like that, you're allowing consumers of your IRepository to misuse the API and access the query outside of session boundaries. That leads to a significant disconnect between how the repository should be used and how it will actually work when you're giving up a piece of raw data like an IQueryable. – Borchert

@Travax A repository is not an in-memory and should not be thought of as one. Sure, a repository could act as an abstraction ontop of a list, but it should not have all the guarantees that a list has. For example, behavior outside of session boundaries is significantly different when there are actual session boundaries involved in contrast to an in-memory list. You bring up a good point about programming responsibility. Personally, I restrict consumers of my APIs to what I intend. For example, I don't RETURN IQueryable, I allow a function to operate over an IQueryable of my choosing to RESTRICT. – Borchert

@Travax You're also missing a key point to my argument and have yet to give a consise, logical explanation as to why NOT separating query and repository. I will give you my definitions for both. A repository issues a command to a persistance-ignorant backing. A query provider is a producer of a projection by accepting a query specification (Func<>). Seriously, look up CQRS. There's a WHOLE laundry list as to why maxing commands and queries at any level is totally a mess. – Borchert

@zowens: any chance you can show an example of implementing your IQueryProvider<T>. Fairly new to LINQ, so not mastered the Func<>y stuff yet. – Transcendent

@zowens: what benefit do you get from making IRepository<in T> contravariant? – Oler

@NickLarsen This really isn't required... but if you are subclassing entities it can be helpful. For example, I have something called a Widget and maybe a ContentAreaWidget that inherits from widget. Well, when the repo is contravariant, I can assign an IRepository<Widget> to IRepository<ContentAreaWidget>. But again, not necessary. – Borchert

Answer 2

This all depends on how "Domain Driven Design" your going to be. Do you know what an Aggregate Root is? Most of the time a generically typed on with can do all your basic CRUD will suffice. Its only when you start having thick models with context and boundaries that this starts to matter.

Blink

I had to look it up, but I think I understand... So my Product would be an aggregate because it encapsulates Categories. And even though my Customer object "has many" Products, it is a separate "idea" and therefore a separate aggregate. Does that sound correct? Are you suggesting a class for each aggregate root? – Stain

Woah, woah, woah, slow down. So if you try and implement DDD without fully understanding DDD you'll become very frustrated and your quality will suffer. Keep it simple for now and just implement a repository for each entity. – Blink

Answer 3

Basically there will be one repository per aggregate root object. There are some interesting points about DDD and aggregate root object and how we should design repository classes in the book ASP.NET MVC 2 in Action, look at it if you want to know more.

Kaule

Answer 4

I'd have one repository/object because invariably, there'd need to be a map from my EntityTable to my domain object (such as in the body of the GetIQueryableCollection(). How I got around writing this repetitive code is by making a T4 template to generate it for me.

I have an example project which generates the repository pattern on codeplex http://t4tarantino.codeplex.com/ T4 Toolbox example Templates for business classes and repository. It may not work exactly the way you'd like without some tweaking, unless you're already implementing Tarintino and a few other goodies, but the templates are easy to configure.

using System;
using System.Collections.Generic;
using System.Linq;
using Cses.Core.Domain.Model;
using StructureMap;

namespace Cses.Core.Domain
{
    /// <summary>
    /// Core class for Schedule E
    /// </summary>
    public class ScheduleERepository : IScheduleERepository
    {

        private Cses.Core.Repository.SqlDataContext _context = new Cses.Core.Repository.SqlDataContext();

        /// <summary>
        /// constructor
        /// </summary>
        public ScheduleERepository() { }

        /// <summary>
        /// constructor for testing
        /// </summary>
        /// <param name="context"></param>
        public ScheduleERepository(Cses.Core.Repository.SqlDataContext context)
        {
            _context = context;

        }

        /// <summary>
        /// returns collection of scheduleE values
        /// </summary>
        /// <returns></returns>
        public IQueryable<ScheduleE> GetIQueryableCollection()
        {           
            return from entity in _context.ScheduleEs                  
               select new ScheduleE()
               {    
                    Amount = entity.Amount,
                    NumberOfChildren = entity.NumberChildren,
                    EffectiveDate = entity.EffectiveDate,
                    MonthlyIncome = entity.MonthlyIncome,
                    ModifiedDate = entity.ModifiedDate,
                    ModifiedBy = entity.ModifiedBy,                      
                    Id = entity.Id                          
               };           
        }

Pulverable

I don't mean to sound harsh but I have a lot of feedback for this code. 1) You should never reference System.Web in your data or domain layer. 2) Use an IOC container like Windsor to scope one data context per web request (PerWebRequestLifestyle) so you can take advantage of caching, SQL batching, transactions, etc. 3) Never swallow exceptions. 4) Separate your queries from your "commands" (code that modifies the DB). 5) Compose Method Refactoring 6) Single Responsibility Principle 7) Never key off strings. Where(x => x.MonthlyIncome.ToString() == key) is hackish. 8) Avoid code generation. – Travax

Thanks for your insight, I shortened the code to the relevant parts, System.web was an oversight. Get Key By string is a required interface using Tarintino, Why avoid code generation? If it reduces reduncant effort, enforces consistency etc? Don't understand the downside... – Pulverable