Category: Code

A couple of years ago I would have told you that if you are accessing the a database you need a repository pattern. This allows you to hide away your data access code behind a nice interface – then at least you can mock the data access layer and change your data access code if you ever wanted to do that.

So say we are in a web application and want to get a “customer” by a specific reference. This is how we would do it with the repository pattern…

How different my opinion is today. I can’t help but feel that the pedal pushed repository pattern (in the C# .Net world) that is on the internet is fundamentally broken. The key thing for me is that everyone seems to want to create the following interface:

 public interface IRepository<TEntity, in TKey> where TEntity : class  
 {  
      IEnumerable<TEntity> GetAll();  
      TEntity GetBy(TKey id);  
      void Save(TEntity entity);       
      void Delete(TEntity entity);       
 }  

Not bad so far hey? This seems pretty “standard” apart from the fact you’ve broken single responsibility (SRP). How many responsibilities does the IRepository already have – 4 public methods which do very different things…

The other thing I’ve found is that things can & generally will get worse from here – then follows:

 public class Repository<TEntity, TKey> : IRepository<TEntity, TKey> where TEntity : class  
 {  
   private readonly IUnitOfWork _unitOfWork;  
   public Repository(IUnitOfWork unitOfWork)  
   {  
     _unitOfWork = unitOfWork;  
   }  
   public IEnumerable<TEntity> GetAll()  
   {  
     return _unitOfWork.Session.QueryOver<TEntity>().ToList();  
   }  
   public TEntity GetBy(TKey id)  
   {  
        return Session.QueryOver<TEntity>().FirstOrDefault(x => x.Id == id);  
   }  
   public void Save(TEntity entity)  
   {  
   }  
   public void Delete(TEntity entity)  
   {  
   }  
 }  

So now all you need to do to get access to something is as follows:

 public class CustomerRepository : Repository<Customer, int>, ICustomerRepository  
 {  
   public CustomerRepository(IUnitOfWork unitOfWork) : base(unitOfWork)  
   {  
   }  
 }  

Does this look familiar? It does to me – I’ve done this and I’m pretty sure its wrong!

Again there are a couple of things that appear wrong:

1. SRP is broken
2. What happens if you’ve got a complicated queryable object and you are using an ORM – you’ll end up with lots of SELECT N+1.
3. We’ve over complicated a simple task with – 3 interfaces, a base class (coupling), and 2 classes.
4. If you want to read from one data store and write to another data store how would you do this?

We’ve introduced so many layers and complexity that it seems that even to do a simple thing it’s been massively over complicated. I’ve forgot to show you the IUnitOfWork which is equally badly suggested and implemented on the internet…

Now for the alternative:

 var customer;  
 using(var transaction = Session.BeginTransaction())  
 {  
      customer = Session.QueryOver<Customer>().FirstOrDefault(x => x.Id == customerId) ?? new NullCustomer();  
      transaction.Commit();  
 }  

Wow, is this simple or what? But what about testing? Well I tend to use NHibernate with an InMemoryDatabase so I don’t have to mock the data access layer… I don’t tend to use mocks but that’s another story… The code is simple and clear.

Now if things advanced and I wanted to put it in a class then I’d put it behind something that only reads customer information:

 public class ReadCustomers  
 {  
      private ISession _session;  
      public ReadCustomers(ISession session)  
      {  
           _session = session;  
      }  
      public Customer GetBy(int customerId)  
      {  
           var customer;  
           using(var transaction = _session.BeginTransaction())  
           {  
                customer = _session.QueryOver<Customer>().FirstOrDefault(x => x.Id == customerId) ?? new NullCustomer();  
                transaction.Commit();  
           }  
           return customer;  
      }  
 }  

Again with storing information if you really wanted to push it in a class then something like this:

 public class StoreCustomers  
 {  
      private ISession _session;  
      public StoreCustomers(ISession session)  
      {  
           _session = session;  
      }  
      public void Add(Customer customer)  
      {  
           using(var transaction = _session.BeginTransaction())  
           {  
                _session.SaveOrUpdate(customer);  
                transaction.Commit();  
           }  
      }       
 }  

The classes have clear responsibility & a single responsibility – you can even easily change where you read and write data from and too! Plus the code is much simpler and easier to optimise.