4 Common Mistakes with the Repository Pattern

 As part of my new ASP.NET Core course, I’ve been encouraging my students to post their code to GitHub. While reviewing various solutions, I’ve encountered a few common mistakes in the implementation of the repository pattern.

One repository per domain

You should think of a repository as a collection of domain objects in memory. If you’re building an application called Vega, you shouldn’t have a repository like the following:

1 2 3

public class VegaRepository { }

Instead, you should have a separate repository per domain class, like OrderRepository, ShippingRepository and ProductRepository.

Repositories that return view models/DTOs

Once again, a repository is like a collection of domain objects. So it should not return view models/DTOs or anything that is not a domain object. I’ve seen many students using AutoMapper inside their repository methods:

1 2 3 4 5 6

public IEnumerable<OrderViewModel> GetOrders() {      var orders = context.Orders.ToList();        return mapper.Map<List<Order>, List<OrderViewModel>(orders); }

Mapping is not the responsibility of the repository. It’s the responsibility of your controllers. Your repositories should return domain objects and the client of the repository can decide if it needs to do the mapping. By mapping the domain objects to view models (or something else) inside a repository, you prevent the client of your repositories from getting access to the underlying domain object. What if you return OrderViewModel but somewhere else you need OrderDetailsViewModel or OrderSnapshotViewModel? So, the client of the repository should decide what it wants to map the Order object to.

Save/Update method in repositories

Yet another very common mistake! As I’ve explained in my YouTube video before, your repositories should not have a Save() or Update() method. I repeat: think of a repository as a collection of domain objects in memory. Do collections have a Save() or Update() method? No! Here’s an example:

1 2 3 4 5 6 7

var list = new List<int>(); list.Add(1); list.Remove(1); list.Find(1);   list.Save();    // doesn't exist! list.Update();  // doesn't exist!

Another reason your repositories should not have a Save() method is because sometimes as part of a transaction you may work with multiple repositories. And then you want to persist the changes across multiple repositories in one transaction. Here’s an example:

1 2 3 4 5

orderRepository.Add(order); orderRepository.Save();   shippingRepository.Add(shipping); shippingRepository.Save();

Can you see the problem in this code? For each change, we need a separate call to the Save() method on the corresponding repository. What if one of these calls to the Save() method fails? You’ll end up with a database in an inconsistent state. Yes, we can wrap that whole thing inside a transaction to make it even more ugly!

A pattern that goes hand in hand with the repository pattern is the unit of work. With the unit of work, we can re-write that ugly code like this:

1 2 3

orderRepository.Add(order); shippingRepository.Add(shipping); unitOfWork.Complete();

Now, either both objects are saved together or none are saved. The database will always be in a consistent state. No need to wrap this block inside a transaction. No need for two separate calls to the Save() method!

Repositories that return IQueryable

One of the reasons we use the repository pattern is to encapsulate fat queries. These queries make it hard to read, understand and test actions in ASP.NET MVC controllers. Also, as your application grows, the chances of you repeating a fat query in multiple places increases. With the repository pattern, we encapsulate these queries inside repository classes. The result is slimmer, cleaner, more maintainable and easier-to-test actions. Consider this example:

1 2 3 4

var orders = context.Orders     .Include(o => o.Details)         .ThenInclude(d => d.Product)     .Where(o => o.CustomerId == 1234);

Here we are directly using a DbContext without the repository pattern. When your repository methods return IQueryable, someone else is going to get that IQueryable and compose a query on top of it. Here’s the result:

1 2 3 4

var orders = repository.GetOrders()     .Include(o => o.Details)         .ThenInclude(d => d.Product)     .Where(o => o.CustomerId == 1234);

Can you see the difference between these two code snippets? The only difference is in the first line. In the first example, we use context.Orders, in the second we use repository.GetOrders(). So, what problem is this repository solving? Nothing!

Your repositories should return domain objects. So, the GetOrders() method should return an IEnumerable. With this, the second example can be re-written as:

1	var orders = repository.GetOrders(1234);

See the difference?

What are the other issues you’ve seen in the implementation of the repository pattern? Share your thoughts!

If you enjoyed this article, please share it.