CodeSOD: Repository, Man

C++ templates are, notoriously, a Turing complete language on their own. They're complicated, even when you're trying to do something simple. Related languages avoid that complexity, and favor generics. Generics are templates with all the sharp bits filed down, but still powerful enough to make datastructures that don't need to know the details of the data they're operating on.

This article isn't about generics, though. It's about ORMs, specifically the .NET Entity Framework system. It's common to use the Repository Pattern to abstract out data access a bit. Coupled with the underlying DBSet class, which roughly represents a database table, you can easily create repository objects and related mocks for testing.

Making your repository class generic is also a good idea. Basic CRUD operations don't really need to know the details the data they're operating on. You can create one repository instance for each type of data you need to store- each table- all based on the same generic type.

But it's possible to make your repository too generic. Take Paulina's company. Here's a few key lines from their generic repository.

public class Repository<T> : IRepository<T>, IDisposable{ MasterContext context = new MasterContext(); // MasterContext derives from Entity Framework's DbContext public void SaveEntity<T>(T entity) where T : class { context.Set<T>().Add(entity); context.SaveChanges(); } public void UpdateEntity<T>(T entity) where T : class { context.Set<T>().Attach(entity); context.Entry<T>(entity).State = System.Data.EntityState.Modified; context.SaveChanges(); } public void DeleteEntity<T>(T entity) where T : class { context.Set<T>().Attach(entity); context.Entry<T>(entity).State = System.Data.EntityState.Deleted; context.SaveChanges(); } public IQueryable<T> Entities<T>() where T : class { return context.Set<T>().AsQueryable<T>(); } // ...}

The first suspicious thing is MasterContext. Deriving from the built-in DbContext is a code smell- you should wrap the DbContext in your own functionality, not extend it.

The real "gotcha" in this, however, is the method definitions. Not only is the class itself generic, but each of its methods is also generic. This creates an odd scenario, where you could do something like this:

var carRepo = new Repository<Car>();carRepo.SaveEntity<Driver>(new Driver());

The repository isn't actually specific to a type. That's not exactly a disaster, but it certainly makes the code more confusing to use. So much more confusing that folks within the organization don't like this version of type safety. So this base Repository class is seated at the base of an inheritance tree.

Derived from Repository is GenericCrudRepository. From that is derived an ExtendedGenericCrudRepository. Then, for any given data entity that we want to save, there are specific types- CarRepository or DriverRepository. Many generic methods are essentially re-implemented with concrete types slotted in, meaning all of these child classes are just piles of similar methods with different calling semantics that all basically do the same thing.

The end result is an incredibly complex and confusing data-access layer. And it's worth noting that the main benefit of a Repository is that it theoretically makes your code more testable, which ah... you'd be surprised to note that since all the repository instances are concrete classes deep down the inheritance tree, they're referred to by specific type and not by interface, meaning no one actually mocked them, and any code that wraps around the database goes untested.

Oh, and this isn't one application. This is a generic data access framework used by every software product in their organization.

Pauline writes: "You can imagine that these projects are very expensive to maintain."

[Advertisement] ProGet's got you covered with security and access controls on your NuGet feeds. Learn more.