Generic Repositories: The good, the bad and the ugly

08 February 2017 Published By: Stephen Bradbury
The good, the bad and the ugly

The generic repository is a pattern that is often used with Entity Framework as a quick way to access data. It does however come with it's own set of problems and is often a trap for some developers that think they are taking shortcuts. This can lead to some very interesting and at times ugly code.

The Bad

A generic repository you may come across or even write may look like the below:

    public interface IRepository<T> 
    {
        Task<int> AddAsync(T t);

        IQueryable<T> Find(Expression<Func<T, bool>> match);
 
        IQueryable<T> GetAll();

        Task<int> RemoveAsync(T t);

        Task<int> UpdateAsync(T t);
    }

This is a leaky abstraction as it will leak IQueryable, this has a major issue with having to trust your developers not to persist IQueryable ALL the way up the stack into the UI layer along with any data objects - Yes, I have seen people writing code that passes IQueryable all the way up and wonder why all the rest of the code is almost instant till the razor template gets rendered.

Eg from UserService.cs

var result = await this.userRepository.Find(x => x.City == City).ToListAsync();

This will also mean we have a dependency on Entity Framework wherever we call ToListAsync()

The Ugly

To solve the issues discussed above and make a more solid generic repository with less generalisation you may be tempted to write your repository without returning IQueryable:

    public interface IRepository<T> 
    {
        Task<int> AddAsync(T t);
		
        Task<int> RemoveAsync(T t);
		
        Task<List<T>> GetAllAsync();
		
        Task<int> UpdateAsync(T t);
		
        Task<int> CountAsync(Expression<Func<T, bool>> match);
		
        Task<bool> AnyAsync(Expression<Func<T, bool>> match);

        Task<T> FindAsync(Expression<Func<T, bool>> match);
		
        Task<List<T>> FindAllAsync(Expression<Func<T, bool>> match);

    }

The repository doesn't leak IQueryable to the calling service, it does however leak the data models into the service layer and allows the developer to easily call into the database with simple commands such as:

var result = await this.userRepository.FindAllAsync(x => x.City == City);

And here lies the trap! When, for example 'FindAsync' is called a database query is constructed and sent to the database. Any other queries you may want execute on the returned list such as pagination and sorting is executed on the list of data in memory. So even if you use:

var result = await this.userRepository.FindAllAsync(x => x.City == City);
return result.Take(5);

The database query generated by Entity Framework will still get ALL the data from the database and return the top 5 from the in memory collection.
So, with this in mind we will need to add another method to the IRepository:

Task<List<T>> FindAllAsync(Expression<Func<T, bool>> match, int page, int pageSize)

But what if we also need sort but NOT page:

Task<List<T>> FindAllAsync<TK>(Expression<Func<T, bool>> match, Expression<Func<T, TK>> sortExpression);

But what if we also need to sort AND page...etc etc you get the point. This will probably spiral until code in our service will look something like this:

results = await this.aRespository.FindAllAsync(x => x.IsActive &&
 x.HasItems.Any() &&
 (!searchFilters.ItemType.Any() ||
  x.ItemTypes.Any(
   vt =>
   searchFilters.ItemType.Contains(
    vt.TypeId)))

 , y => y.Name,
 searchFilters.PageNumber,
 searchFilters.PageSize,
 true);

It's not as flexible as returning IQueryable and it's ugly!

The Good

So how can we resolve this? The repository pattern as seen in the 1st example does leverage some handy functionality is relatively flexible and can help our code base to be less repetitive. So we could use the generic repository inside a more defined contract. A bit like a proxy class.

    public class UserRepository : IUserRepository
    {
        private readonly IRepository<Data.User> userRespository;

        public UserRepository (IRepository<Data.User> userRespository)
        {
            this.userRespository = userRespository;
        }
	
        public Task<Data.User> FindByCity(string city)
        {
            return this.userRepository.Find(x => x.City == city).ToListAsync();
        }
    }

 This gives you the advantages of having a flexible generic repository along with a defined contract on top of it.