OneTrueError - Automated exception handling

Repository pattern, done right

The repository pattern has been discussed a lot lately. Especially about it’s usefulness since the introduction of OR/M libraries. This post (which is the third in a series about the data layer) aims to explain why it’s still a great choice.

Let’s start with the definition:

A Repository mediates between the domain and data mapping layers, acting like an in-memory domain object collection. Client objects construct query specifications declaratively and submit them to Repository for satisfaction. Objects can be added to and removed from the Repository, as they can from a simple collection of objects, and the mapping code encapsulated by the Repository will carry out the appropriate operations behind the scenes

The repository pattern is an abstraction. It’s purpose is to reduce complexity and make the rest of the code persistent ignorant. As a bonus it allows you to write unit tests instead of integration tests. The problem is that many developers fail to understand the patterns purpose and create repositories which leak persistence specific information up to the caller (typically by exposing IQueryable<T>).

By doing so they get no benefit over using the OR/M directly.

Common misconceptions

Here are some common misconceptions regarding the purpose of the pattern.

Repositories is about being able to switch DAL implementation

Using repositories is not about being able to switch persistence technology (i.e. changing database or using a web service etc instead).

Repository pattern do allow you to do that, but it’s not the main purpose.

A more realistic approach is that you in UserRepository.GetUsersGroupOnSomeComplexQuery() uses ADO.NET directly while you in UserRepository.Create() uses Entity Framework. By doing so you are probably saving a lot of time instead of struggling with LinqToSql to get your complex query running.

Repository pattern allow you to choose the technology that fits the current use case.

Unit testing

When people talks about Repository pattern and unit tests they are not saying that the pattern allows you to use unit tests for the data access layer.

What they mean is that it allows you to unit test the business layer. It’s possible as you can fake the repository (which is a lot easier than faking nhibernate/EF interfaces) and by doing so write clean and readable tests for your business logic.

As you’ve separated business from data you can also write integration tests for your data layer to make sure that the layer works with your current database schema.

If you use ORM/LINQ in your business logic you can never be sure why the tests fail. It can be because your LINQ query is incorrect, because your business logic is not correct or because the ORM mapping is incorrect.

If you have mixed them and fake the ORM interfaces you can’t be sure either. Because Linq to Objects do not work in the same way as Linq to SQL.

Repository pattern reduces the complexity in your tests and allow you to specialize your tests for the current layer

How to create a repository

Building a correct repository implementation is very easy. In fact, you only have to follow a single rule:

Do not add anything into the repository class until the very moment that you need it

A lot of coders are lazy and tries to make a generic repository and use a base class with a lot of methods that they might need. YAGNI. You write the repository class once and keep it as long as the application lives (can be years). Why fuck it up by being lazy? Keep it clean without any base class inheritance. It will make it much easier to read and maintain.

The above statement is a guideline and not a law. A base class can very well be motivated. My point is that you should think before you add it, so that you add it for the right reasons.

Mixing DAL/Business

Here is a simple example of why it’s hard to spot bugs if you mix LINQ and business logic.

var brokenTrucks = _session.Query<Truck>().Where(x => x.State == 1);
foreach (var truck in brokenTrucks)
{
   if (truck.CalculateReponseTime().TotalDays > 30)
       SendEmailToManager(truck);
}

What does that give us? Broken trucks?

Well. No. The statement was copied from another place in the code and the developer had forgot to update the query. Any unit tests would likely just check that some trucks are returned and that they are emailed to the manager.

So we basically have two problems here:

a) Most developers will likely just check the name of the variable and not on the query.
b) Any unit tests are against the business logic and not the query.

Both those problems would have been fixed with repositories. Since if we create repositories we have unit tests for the business and integration tests for the data layer.

Implementations

Here are some different implementations with descriptions.

Base classes

These classes can be reused for all different implementations.

UnitOfWork

The unit of work represents a transaction when used in data layers. Typically the unit of work will roll back the transaction if SaveChanges() has not been invoked before being disposed.

public interface IUnitOfWork : IDisposable
{
    void SaveChanges();
}

.

Paging

We also need to have page results.

public class PagedResult<TEntity>
{
    IEnumerable<TEntity> _items;
    int _totalCount;
    
    public PagedResult(IEnumerable<TEntity> items, int totalCount)
    {
        _items = items;
        _totalCount = totalCount;
    }
    
    public IEnumerable<TEntity> Items { get { return _items; } }
    public int TotalCount { get { return _totalCount; } }
}

We can with the help of that create methods like:

public class UserRepository
{
    public PagedResult<User> Find(int pageNumber, int pageSize)
    {
    }
}

Sorting

Finally we prefer to do sorting and page items, right?

var constraints = new QueryConstraints<User>()
    .SortBy("FirstName")
    .Page(1, 20);
    
var page = repository.Find("Jon", constraints);

Do note that I used the property name, but I could also have written constraints.SortBy(x => x.FirstName). However, that is a bit hard to write in web applications where we get the sort property as a string.

The class is a bit big, but you can find it at github.

In our repository we can apply the constraints as (if it supports LINQ):

public class UserRepository
{
    public PagedResult<User> Find(string text, QueryConstraints<User> constraints)
    {
        var query = _dbContext.Users.Where(x => x.FirstName.StartsWith(text) || x.LastName.StartsWith(text));
        var count = query.Count();
        
        //easy
        var items = constraints.ApplyTo(query).ToList();
        
        return new PagedResult(items, count);
    }
}

The extension methods are also available at github.

Basic contract

I usually start use a small definition for the repository, since it makes my other contracts less verbose. Do note that some of my repository contracts do not implement this interface (for instance if any of the methods do not apply).

public interface IRepository<TEntity, in TKey> where TEntity : class
{
    TEntity GetById(TKey id);
    void Create(TEntity entity);
    void Update(TEntity entity);
    void Delete(TEntity entity);
}

I then specialize it per domain model:

public interface ITruckRepository : IRepository<Truck, string>
{
    IEnumerable<Truck> FindBrokenTrucks();
    IEnumerable<Truck> Find(string text);
}

That specialization is important. It keeps the contract simple. Only create methods that you know that you need.

Entity framework

Do note that the repository pattern is only useful if you have POCOs which are mapped using code first. Otherwise you’ll just break the abstraction using the entities. The repository pattern isn’t very useful then.

What I mean is that if you use the model designer you’ll always get a perfect representation of the database (but as classes). The problem is that those classes might not be a perfect representation of your domain model. Hence you got to cut corners in the domain model to be able to use your generated db classes.

If you on the other hand uses Code First you can modify the models to be a perfect representation of your domain model (if the DB is reasonable similar to it). You don’t have to worry about your changes being overwritten as they would have been by the model designer.

You can follow this article if you want to get a foundation generated for you.

Base class

public class EntityFrameworkRepository<TEntity, TKey> where TEntity : class
{
    private readonly DbContext _dbContext;

    public EntityFrameworkRepository(DbContext dbContext)
    {
        if (dbContext == null) throw new ArgumentNullException("dbContext");
        _dbContext = dbContext;
    }

    protected DbContext DbContext
    {
        get { return _dbContext; }
    }

    public void Create(TEntity entity)
    {
        if (entity == null) throw new ArgumentNullException("entity");
        DbContext.Set<TEntity>().Add(entity);
    }

    public TEntity GetById(TKey id)
    {
        return _dbContext.Set<TEntity>().Find(id);
    }

    public void Delete(TEntity entity)
    {
        if (entity == null) throw new ArgumentNullException("entity");
        DbContext.Set<TEntity>().Attach(entity);
        DbContext.Set<TEntity>().Remove(entity);
    }

    public void Update(TEntity entity)
    {
        if (entity == null) throw new ArgumentNullException("entity");
        DbContext.Set<TEntity>().Attach(entity);
        DbContext.Entry(entity).State = EntityState.Modified;
    }
}

Then I go about and do the implementation:

public class TruckRepository : EntityFrameworkRepository<Truck, string>, ITruckRepository
{
    private readonly TruckerDbContext _dbContext;

    public TruckRepository(TruckerDbContext dbContext)
    {
        _dbContext = dbContext;
    }

    public IEnumerable<Truck> FindBrokenTrucks()
    {
        //compare having this statement in a business class compared
        //to invoking the repository methods. Which says more?
        return _dbContext.Trucks.Where(x => x.State == 3).ToList();
    }

    public IEnumerable<Truck> Find(string text)
    {
        return _dbContext.Trucks.Where(x => x.ModelName.StartsWith(text)).ToList();
    }
}

Unit of work

The unit of work implementation is simple for Entity framework:

public class EntityFrameworkUnitOfWork : IUnitOfWork
{
    private readonly DbContext _context;

    public EntityFrameworkUnitOfWork(DbContext context)
    {
        _context = context;
    }

    public void Dispose()
    {
        
    }

    public void SaveChanges()
    {
        _context.SaveChanges();
    }
}

nhibernate

I usually use fluent nhibernate to map my entities. imho it got a much nicer syntax than the built in code mappings. You can use nhibernate mapping generator to get a foundation created for you. But you do most often have to clean up the generated files a bit.

Base class

public class NHibernateRepository<TEntity, in TKey> where TEntity : class
{
    ISession _session;
    
    public NHibernateRepository(ISession session)
    {
        _session = session;
    }
    
    protected ISession Session { get { return _session; } }
    
    public TEntity GetById(string id)
    {
        return _session.Get<TEntity>(id);
    }

    public void Create(TEntity entity)
    {
        _session.SaveOrUpdate(entity);
    }

    public void Update(TEntity entity)
    {
        _session.SaveOrUpdate(entity);
    }

    public void Delete(TEntity entity)
    {
        _session.Delete(entity);
    }
}

Implementation

public class TruckRepository : NHibernateRepository<Truck, string>, ITruckRepository
{
    public TruckRepository(ISession session)
        : base(session)
    {
    }

    public IEnumerable<Truck> FindBrokenTrucks()
    {
        return _session.Query<Truck>().Where(x => x.State == 3).ToList();
    }

    public IEnumerable<Truck> Find(string text)
    {
        return _session.Query<Truck>().Where(x => x.ModelName.StartsWith(text)).ToList();
    }
}

Unit of work

public class NHibernateUnitOfWork : IUnitOfWork
{
    private readonly ISession _session;
    private ITransaction _transaction;

    public NHibernateUnitOfWork(ISession session)
    {
        _session = session;
        _transaction = _session.BeginTransaction();
    }

    public void Dispose()
    {
        if (_transaction != null)
            _transaction.Rollback();
    }

    public void SaveChanges()
    {
        if (_transaction == null)
            throw new InvalidOperationException("UnitOfWork have already been saved.");

        _transaction.Commit();
        _transaction = null;
    }
}

Typical mistakes

Here are some mistakes which can be stumbled upon when using OR/Ms.

Do not expose LINQ methods

Let’s get it straight. There are no complete LINQ to SQL implementations. They all are either missing features or implement things like eager/lazy loading in their own way. That means that they all are leaky abstractions. So if you expose LINQ outside your repository you get a leaky abstraction. You could really stop using the repository pattern then and use the OR/M directly.

public interface IRepository<TEntity>
{
    IQueryable<TEntity> Query();
    
    // [...]
}

Those repositories really do not serve any purpose. They are just lipstick on a pig.

Learn about lazy loading

Lazy loading can be great. But it’s a curse for all which are not aware of it. If you don’t know what it is, Google.

If you are not careful you could get 101 executed queries instead of 1 if you traverse a list of 100 items.

Invoke ToList() before returning

The query is not executed in the database until you invoke ToList(), FirstOrDefault() etc. So if you want to be able to keep all data related exceptions in the repositories you have to invoke those methods.

Get is not the same as search

There are to types of reads which are made in the database.

The first one is to search after items. i.e. the user want to identify the items that he/she like to work with.

The second one is when the user has identified the item and want to work with it.

Those queries are different. In the first one, the user only want’s to get the most relevant information. In the second one, the user likely want’s to get all information. Hence in the former one you should probably return UserListItem or similar while the other case returns User. That also helps you to avoid the lazy loading problems.

I usually let search methods start with FindXxxx() while those getting the entire item starts with GetXxxx(). Also don’t be afraid of creating specialized POCOs for the searches. Two searches doesn’t necessarily have to return the same kind of entity information.

Summary

Don’t be lazy and try to make too generic repositories. It gives you no upsides compared to using the OR/M directly. If you want to use the repository pattern, make sure that you do it properly.

Changes

2013-01-15 Rewrote entity framework implementation and some other updates.
2014-02-20 Try my new data mapper in your repository implementation.

This entry was posted in Architecture, CodeProject and tagged , , , . Bookmark the permalink.
  • Rafael

    Very good article! But I a have a question, how about more complex query cenarios where user can build your own custom search based on objects properties(Redmine has this feature eg) . How implement this in the repository?

    Sorry for the bad english.

    • JonasGauffin

      imho the specification pattern is better for that. Create different possible specifications and then let the user combine those.

      Create something like a SpecificationExecutor class which uses the OR/M internally to execute the specifications.

  • http://www.facebook.com/superjmn José Manuel Nieto Sánchez

    Hi Jonas, I’m “SuperJMN” in Twitter. I’m very interested in your articles regarding patterns, abstractions and such modern technologies that are in the limelight. I promise I will be a close follower. In the recent past I worked with EF in a Code First approach using the advices presented by Unai Zorrilla and César de la Torre in Microsoft Spain (NLayerApp) that I pointed out in some tweet. I also used generic repositories, but I felt I was duplicating functionalities since DataContexts are in fact a combination of Repository+UnitOfWork. I have read you article about Abstraction and you’re right: If you want to swith OR/Ms the most abstract approach requires the less effort. And in terms of testing, I think your point of view is also more flexible. But, actually, do you think that a project might switch OR/M after being in production? Does it make sense to expend so many time coding thousands of methods like “FindXByThis” methods that can make use of LINQ to make queries and filter data?

    Please, take into consideration that I’m myself a newbie in this and I don’t have any kind of expertise, so this comment is more a doubt than an argument.

    • JonasGauffin

      You should never use abstractions to be able to switch OR/Ms, since it’s unlikely. I’m saying that OR/S is just a layer which simplifies the development using databases. But they do not guarantee that the code that you write will work.

      However, you might find that the OR/M is not the best fit for all of your queries. And if you have used repositories you can easily change from OR/M to ADO.NET or similar for queries (not all, only for the troublesome) without affecting any other code.

      Writing LINQ statements is really no difference from writing SQL statements. They are as fragile as any changes to the database (or the mappings or to the LINQ provider) will affect them. Hence you still need some way to make sure that they work correctly.

      You can do it by creating integration tests for all methods that use the OR/M. But then again nothing will say if it’s the business logic or the LINQ statements that fail.

      If you instead create abstractions (no matter if it’s using repository pattern or something else) you have successfully removed all DB responsibility from the business code. So if anything fails in them, it’s the business rules (as long as you have created proper tests for the data layer).

      • ep

        So basically, what we’re saying is that you have to weigh the future liklihood of replacing out or largely changing the DB layer against the extra work of defining the repos / UoW. As an additional consideration, a well structured application should use the layers to effect good information hiding, but if you’re not doing any real information hiding via adding a seam between your app and the db layer, then perhaps its not worth adding the seam at all?

        • JonasGauffin

          I’m saying that if you use the OR/M directly in your business layer you’ll give each method two responsibilities (and thus violating Single Responsibility Principle):

          1. The business logic
          2. Querying your data source

          So if something goes wrong, you can’t be sure if it’s because your query is incorrect or if the business logic failed. Same goes for your unit tests.

          If you on the other hand breaks out the data access and put it into a repository you do know what the cause is.

          So basically using an abstraction layer reduces complexity.

  • http://www.facebook.com/superjmn José Manuel Nieto Sánchez

    By the way, the part that I don’t understand about this post is when you say “Do note that the repository pattern is only useful if you have POCOs which are mapped using code first. Otherwise you’ll just break the abstraction using the entities instead (= the repository pattern isn’t very useful then).”

    Can you give as a brief example or what this means? When using CodeFirst, I intended to use classes with business logic (behavior) not POCOs, but I didn’t face any problem since I changed my job before we can added more logic to the infrastructure. Do you mean that CodeFirst is not advisable for non-POCO classes in the model?

    Thanks a lot for your time!

    • JonasGauffin

      I’ve updated that part of the article. Please read again.

      • ep

        Would still appreciate an explanation!

        • JonasGauffin

          If you use CodeFirst you can add behaviour to your classes without having to worry about the entity designer writing over them.

          Don’t confuse DTOs with POCOs. DTOs are anemic (without behaviour) while POCOs stands for “Plain Old CLR objects”. i.e. no proxies or auto generated classes.

          • Mickaël

            Hey Jonas,
            I had trouble understanding that part, too, I’ve been thinking about it and wanted to ask you the question. I see I’m not the only one :-) I still don’t get why CodeFirst is compelling.
            The designer generates partial classes which allows you to add behavior without worrying about the designer.
            Still, I love your posts and I’m looking forward to reading you again.

          • JonasGauffin

            I haven’t used the model signer in a long time. I might be wrong. Let me find some time to play with it again.

  • http://www.Marisic.Net/ dotnetchris

    Not to be a hater, but for you to open up with a blog title “Repository pattern, done right” you directly drive off the cliff and show the wrong ways to use a repository.

    TruckRepository is just shit code entirely. That TruckRepository example for entity framework just shouldn’t exist EVER.

    NHibernateRepository is better, but not much better.

    public class TruckRepository : NHibernateRepository, is just flat out wrong. This is one of the worst design choices you can ever make. This leads to aenmic code, you end up with lots of crap classes that are

    public class FooRepository : NHibernateRepository { }
    public class BarRepository : NHibernateRepository { }

    This then leads to further design crap, to do something meaningful you then need to inject dependencies of TruckRepository FooRepository BarRepository where you can end up with half a dozen or a dozen anemic repositories, and so far you still have not written 1 single line of business code. Furthermore, now that you need some kind of Orchestrator/Mediator/Service class that binds all of these disparate repositories together to do even the slightest bit of meaningful action, this will result in huge amounts of mocking work to unit test this code (even if you use an automocking container).

    Now the generic repository pattern which you implement with NHibernateRepository, that is a fine pattern. Where the wheels fall off is where you expect users to inherit from it. Generic repository should be a dependency, not an implementation. The generic repository should be injected into your service, not be the service.

    I talk about this in regards to NHibernate: http://dotnetchris.wordpress.com/2010/03/15/creating-a-common-generic-and-extensible-nhiberate-repository-version-2/ but the concept is independent of the persistence mechanisms.

    Ironically where you discuss “public interface IRepository { IQueryable Query();” you fail to see the very code you advocate is proverbial lipstick on the pig. I find even further irony in your statement about “refusing to see the truth”. My critique might be overly harsh but for you to make a bold statement like your blog title and then post disparaging remarks in your post that you’re directly guilty of. Expect fireballs to be thrown back.

    • JonasGauffin

      Thank you for your comment ;)

      You state that specialized repositories as in my example are anemic, which is not correct. The only thing that really is the same is the CRUD methods.

      Any QUERIES are created per repository. Those methods should be created when you need them, and not up front because you *think* that you need those.

      Now the generic repository pattern which you implement with NHibernateRepository, that is a fine pattern. Where the wheels fall off is where you expect users to inherit from it. Generic repository should be a dependency, not an implementation. The generic repository should be injected into your service, not be the service.

      Absolutely not. That defeats the whole purpose of the repository pattern. If you want to create a generic repository which can be used everywhere you have just put lipstick on the OR/M. What’s the purpose? Any shortcomings of the LINQ to SQL implementation of the OR/M will surface up in the business layer.

      • http://www.Marisic.Net/ dotnetchris

        They are anemic. Take States and ZipCodes, those are very closely related and you will likely need to use them in combination. This would lead to likely wanting to provide a meaningful LocationRepository or GeoRepository etc. This is just not achievable when you force people to inherit from a base class like that.

        Regardless of what you **feel** you are doing, you are not using abstraction with NHibernateRepository. Your TruckRepository does not truly extend NHibernateRepository, you do not do List Repositories, so there’s no use of polymorphism , there is absolutely no reason for inheritance. You are composing functionality, which is the delegation pattern. The repository should be injected. Your NHibernateRepository is a violation of the open closed principle, your class now has 2 reasons to change, it will change if TruckRepository needs to change (valid) but it will also have to change if NHibernateRepository changes (not valid to change TruckRepository)

        If you feel I am incorrect, show me one single instance where you act upon NHibernateRepository directly. If you never use NHibernateRepository, your base type is invalid and is not a base type.

        “If you want to create a generic repository which can be used everywhere
        you have just put lipstick on the OR/M. What’s the purpose?” Now you’re getting closer. Most repository code serves no purpose, just like your TruckRepository, it’s all just shit code. Now if you look at my blog post, you would see my Repository implements code almost exactly the same as the code as your TruckRepository (EF) and your NHibernateRepository. This code in all scenarios is shit code, but it’s important and needs to be wrote once. This is the code that makes you physically depend on EF or NH.

        The substantial difference is with my code is you are **consuming** the generic repository which is in reality truly happening. You are looking to use NH/EF/persistance store, you aren’t looking to extend it. Your code makes false assertions by requiring inheritance. Because of this false assumption you introduce more design issues down stream from having tons of little repositories that are fully isolated, this is an incredibly heavy handed dictation by you to say that zip codes and states can never ever be used together, you must stitch them together at a higher level. Using Generic Repository as a dependency complies with the Open Closed principle. I could create new methods, even abstract methods that will only require implementation in 1 location, in your code if you add an abstract method to your NHibernateRepository you will have to implement that method in 100% of your repoistories leading to methods like

        public bool MyImportantMethod { } or throw new NotImplementedException. Which raising a NIE further affirms you’re not actually doing anything with the base class and instead are using it as a dumb way to do delegation.

        • JonasGauffin

          [...] This is just not achievable when you force people to inherit from a base class like that.

          I do not force anyone to inherit. On the contrary. Quoting myself in the article: Do note that some of my repository contracts do not implement this interface (for instance if any of the methods do not apply)..

          Regardless of what you **feel** you are doing, you are not using abstraction with NHibernateRepository.

          Of course not. NHibernateRepository is an implementation. The ITruckRepository is the abstraction

          Your TruckRepository does not truly extend NHibernateRepository, you do not do List Repositories, so there’s no use of polymorphism , there is absolutely no reason for inheritance. You are composing functionality, which is the delegation pattern.

          Yes it does. It extends the NHibernateRepository and add more functionality. The TruckRepository implementation is-a nhibernate repository. It would not work without nhibernate. This could of course have been implemented by using composition. But what purpose would that fill? It would just make the CRUD call the underlying class.

          The repository should be injected. Your NHibernateRepository is a violation of the open closed principle,

          Feel free to expand on that. I don’t see how any extension (in this scope) would require a modification of the nhibernate repository?

          your class now has 2 reasons to change, it will change if TruckRepository needs to change (valid) but it will also have to change if NHibernateRepository changes (not valid to change TruckRepository)

          Are you talking about SRP now? Inheritance do make the subclass coupled with the superclass, that’s a price that you have to pay with inheritance. So inherently all sub classes do change if the parent class is modified. But I wouldn’t go as far as saying it that gives the TruckRepository two reasons to change.

          If you feel I am incorrect, show me one single instance where you act upon NHibernateRepository directly. f you never use NHibernateRepository, your base type is invalid and is not a base type.

          So you are saying that all classes which are not used directly are invalid? Come on… Let’s ban all abstract classes.

          If you want to create a generic repository which can be used everywhere

          you have just put lipstick on the OR/M. What’s the purpose? Now you’re getting closer. Most repository code serves no purpose, just like your TruckRepository, it’s all just shit code.

          Why is it “shit code”?

          Now if you look at my blog post, you would see my Repository implements code almost exactly the same as the code as your TruckRepository (EF) and your NHibernateRepository. This code in all scenarios is shit code, but it’s important and needs to be wrote once. This is the code that makes you physically depend on EF or NH.

          Now you lost me. Are you saying that your Repository is shit code?

          The substantial difference is with my code is you are **consuming** the generic repository which is in reality truly happening. You are looking to use NH/EF/persistance store, you aren’t looking to extend it. Your code makes false assertions by requiring inheritance. Because of this false assumption you introduce more design issues down stream from having tons of little repositories that are fully isolated,

          I could discuss your code and what I think about it’s design. But really. There is no point. I prefer having contracts which says `ITruckRepository.FindBrokenTrucks` over using the specification pattern (which you have renamed to criterias).

          this is an incredibly heavy handed dictation by you to say that zip codes and states can never ever be used together, you must stitch them together at a higher level. Using Generic Repository as a dependency complies with the Open Closed principle. I could create new methods, even abstract methods that will only require implementation in 1 location.

          Well. It all comes down to how the aggregates are designed. Is a zip code a child aggregate to a state? It might be in some applications while on others it isn’t. If it is, I would create methods for them in my StateRepository. But in other projects they might only be treated as value objects and stored in, for instance, the order entity.

          in your code if you add an abstract method to your NHibernateRepository you will have to implement that method in 100% of your repoistories leading to methods like

          public bool MyImportantMethod { } or throw new NotImplementedException. Which raising a NIE further affirms you’re not actually doing anything with the base class and instead are using it as a dumb way to do delegation.

          Again you make the assumption that I force all repositories to implement the base classes. Again, incorrect. Go ahead and tell me where I make such statements. You are over generalizing.

          • http://www.Marisic.Net/ dotnetchris

            There is no way to have any more meaningful discussion given the limited capabilities of disqus threading.

            The fact you inherit from your NHibernateRepository is wrong. NHibernateRepository is a dependency and has no valid reason to be a base class. Forcing classes to inherit is about the furthest reaching api decision you can ever enforce upon consumers, it is nearly unilaterally wrong. The time it is valid to force inheritance is if the class had an abstract method that needed to be implemented by the consumer.

            When you inherit from NHibernateRepository, you are not extending it. The code you write in TruckRepository has nothing to do with NHibernateRepository, except that it would like to consume funtionality that NHibernateRepository provides. This is 100% unequivocally the delegation pattern, not a place for inheritance.

            To put it bluntly, seeing the need for inheritance from NHibernateRepository is simply inexperience at software architecture.

          • JonasGauffin

            The fact you inherit from your NHibernateRepository is wrong. NHibernateRepository is a dependency and has no valid reason to be a base class. Forcing classes to inherit is about the furthest reaching api decision you can ever enforce upon consumers, it is nearly unilaterally wrong. The time it is valid to force inheritance is if the class had an abstract method that needed to be implemented by the consumer.

            The API against the data layer is the interfaces and not the classes. The repositories themselves can be created using the factory pattern or a more modern approach like an inversion of control container. There is no reason that the user (i.e. the calling code) should be aware of that nhibernate (or my base class) is used at all.

            You keep saying that I force inheritance of the nhibernate repository class. Don’t you read what I wrote in my article or my answers?

            When you inherit from NHibernateRepository, you are not extending it. The code you write in TruckRepository has nothing to do with NHibernateRepository, except that it would like to consume funtionality that NHibernateRepository provides. This is 100% unequivocally the delegation pattern, not a place for inheritance.

            I could have named the base class something like “NHibernateCrudOperations” and then inherited it with “TruckRepository” or “TruckNhibernateRepository”. I could have named the base class just “CrudOperations” since it’s just an implementation detail. The fact remains that it’s just naming. It’s still perfectly valid inheritance.

            You must have failed to read the abstraction part of my article. The only way to remove the dependency to nhibernate for the CRUD methods is to use the delegation pattern. Each repository could have called nhibernate for the CRUD operations directly. But why? It would just mean duplicate code. The alternative would have been to expose nhibernate to the using code just to not use the delegation pattern. But that in turn would increase complexity and not reduce it.

            It’s great that you have gone from “shit code” to just complain about the inheritance. Congratulations. It could have been an interesting discussion, but looking on the impact on the overall architecture in an application it’s worthless. It doesn’t really matter if the repositories inherit a base class or not. If you haven’t understood my reasoning about the base class you’ll never will. So there is no point in continuing this discussion.

  • hasibul

    Thanks for nice post.

    Is it possible to share any complete sample project done by you where you implemented NHibernate & EF together?

    I need some suggestion; if i have bounded db context then how i can manage transaction between them.

    • JonasGauffin

      Yep. I’ll add complete examples to my Samples repository at github as soon as I get some time.

  • Oli

    My understanding of the contravariance stuff that I couldn’t create a TruckRepository where the keys are “int”. Is that correct?

    I’m running into this issue myself where my Controller doesn’t want to know what type the keys in my repo are, it should just pass whatever comes back from the client (e.g. as a querystring param). Do you recommend using string as key and let the concrete repo parse it?

    • JonasGauffin

      No. The repository should always use the correct type according to the business rules.
      What kind of error do you get in your controller? I might be able to help.

  • Daskul

    Can you explain why the base repository does not implement IRepository, but they have the same methods?

    • http://blog.gauffin.org/ jgauffin

      No :) No reason at all, most likely just something that I missed when I wrote the article.

  • http://www.dotnetjalps.com/ Jalpesh Vadgama

    Nice explanation!! I am also agree with you we should not force to inherit and it should only have common methods!!

  • Whatever

    IMHO, EF sucks ass and doesn’t hold a candle to stored procs. If you’re using them successfully in your project, then you’re in a very low traffic situation. Enterprise applications do NOT use EF / LINQ for anything that is truly under load.
    EF / LINQ 2 SQL are great for quick prototypes little else.

    • http://blog.gauffin.org/ jgauffin

      SPs have no performance gain over parameterized queries from SQL Server 2005 and forward.

      • Joe Smith

        Well, no perf gain is a little strong of a claim. First of all sending the name of a stored procedure over requires moving less data than a complicated query, which might have a tiny (mostly insignificant) gain.Slightly larger is the fact that sqlserver can in theory (I’m not sure if it does) avoid reparsing a stored procedure, but cannot avoid reparsing a passed qeury. It is true that both can use cached query plans, and the other costs are insignifcant relative to query plan generation, and plan execution.

        • whatever2

          completely agree with Joe’s remark… and not to mention all the goodies a good DBMS offers (optimization, locks …). EF looks cool on paper, not sure of the benefits yet.

          • http://blog.gauffin.org/ jgauffin

            My comment had nothing to do with Entity Framework but parameterized queries vs stored procedures. You need a very large database to see any real gains with stored procedures.

            in my experience the cost of maintaining an application where a lot of business logic is in stored procedures is a lot larger than buying new hardware to compensate for the performance loss.

  • icokk

    I am trying to build repository pattern example. However, I have stumbled upon two problems: where do I manage the session lifetime (I am building a desktop application and I read it shouldn’t have one session alive for the lifetime of the application) and what class should be responsible for creating units-of-work?

    • http://blog.gauffin.org/ jgauffin

      Desktop applications do not have an obivous context seperation as HTTP Request/reply has. But I typically create a session each time some information should need to be fetched (for instance when a new Form is created or when a button is pressed) and dispose the context directly after.

      Example:

      https://gist.github.com/jgauffin/6523472

  • MC

    I do not agree on the fact that you should not expose any IQueryable and in addition you have to call ToList before returning something. This is basically bullshit to say something like that because if you return e.g. Query().ToList() and the client just needs 2 out of 2million rows, this will mess up things pretty quickly and noone will use your crappy and slow repository implementation…

    To do it right, you would have to wrap the queryable maybe, build your own expression tree and apply it to the underlying datasource (nhibernate or what ever)…

    Of course this is a lot of work and one might ask why on earth would one need just another wrapper for things which do work just fine already and have documentation and everything?!

    • http://blog.gauffin.org/ jgauffin

      The repository pattern is an abstraction layer. If you use it, you should do a complete abstraction. It’s useless otherwise. Use Entity Framework directly in that case. More details here: http://stackoverflow.com/a/17449231/70386

      A repository should not have a “GetAll” method. It should have methods for every use case (or use the specification pattern).

    • SuperSalad

      Can you just use a method in the repository with a lambda expression parameter that you can use in the WHERE clause so you avoid pulling the whole records from the database?

  • sam

    Good article!

    Quick question… how do you get the ISession or DbContext out of the UnitOfWork and into the repository?

    I’ve seen some people make individual repositories properties of the UnitOfWork. It kind of makes sense because the underlying data access layer (EF/NH) is going to determine the implementation of UnitOfWork and Repository. Example: It wouldn’t make sense that you would inject/mock a NH UnitOfWork with a EF Repository.

    This also simplifies things on the controller side (MVC) because you are only injecting IUnitOfWork, instead of IUnitOfWork + every repository you use in the controller.

    • http://blog.gauffin.org/ jgauffin

      I usally use an inversion of control container in my projects. Hence I never have to create the unit of work directly. The repositories can then request the concrete class instead of the IUnitOfWork interface (as there is really no point in using unit tests for repositories since only integration tests can verify that the repositories work).

      At a client I created a IAdoNetUnitOfWork which has a “IDbCommand CreateCommand()” method. The repositories took that interface a dependency while the ASP.NET MVC controller used the UnitOfWorkFactory which returns IUnitOfWork (but created a AdoNetUnitOfWork instance internally)

      • sam

        Is it possible to see a brief example of what you are talking about? It makes sense that your concrete repository would accept the conrete data context. This guarantees that you are using the proper data access technology. What I keep struggling with is how?

        For example: In your EfUnitOfWork, your DbContext is private and I see no way to access it outside of the class. How are you getting the data context out of the UoW and into the constructor of your concrete EfTruckRepository? Or is the EfTruckRepository part of your EfUnitOfWork and I’m just missing it somewhere?

        • http://blog.gauffin.org/ jgauffin
          • sam

            Thanks for the example. So you are using concrete repositories directly instead of injecting them, correct? Then, within the concrete repository’s constructor, you cast the IUnitOfWork to UnitOfWork and throw an exception if the data layer doesn’t match. That is what I was trying to figure out. Thanks.

          • http://blog.gauffin.org/ jgauffin

            Yes, when I’m not using a DI.

            For a DI I inject the repositories in my MVC controllers etc.Search for the TransactionAttribute in this blog to see how I handle the unit of work then.

            I have used an IAdoNetUnitOfWork interface before, but there is really no point since integration tests are the only way to test repositories properly.

  • Masoud Sanaei Ardakani

    In your EntityFrameworkRepository class we have basic CUD methods, but how can we write a method that apply CUD operations on a graph of objects(aggregation root in DDD)?
    For example I have an Order class that has one-to-many relation with OrderLines. In the client side the user edit an Order with 3 OrderLines in following order:
    1- Edit OrderDate in Order
    2- Add a new OrderLine
    3- Edit QTY in OrderLine #2
    4- Delete OrderLine #3
    5- Send a request for save the Order.

  • Hung Nguyen

    Agree with you about “Do not expose IQueryable from repository” because of the drawback that it will make your entire app is tied up to LINQ. There’re also a few good article over the internet debating about “IQueryable or IEnumerable” But, your sample code are so simple (like a lot of examples I’ve googled). For a more complex case, say an e-commerce application where there’re a few different entities related each other like Product, User, Order, OrderLine etc. Then how can you do a complex join between the entities without implement a Generic Repository with a method GetAll() return an IQueryable ?

    • http://blog.gauffin.org/ jgauffin

      When you do joins you typically have a specific use case in mind which need a subset of the properties that the joined entities provide. Simply create a new entity type for that use case and create a repository which does the join and return the new type.

      • Hung Nguyen

        So you mean: inside a OrderRepository I need to write a method which do a join between Order, OrderLine, User, Product ?

        • http://blog.gauffin.org/ jgauffin

          Hard to say. It really depends on the usecase. If the number of records are limited I would typically create a OrderListService which first fetches the orders, go through them to find the user ids, do a query on the wanted userids and do a query on a productid list. Finally I would build the new entities and return them.

          In other cases I would not use the repository pattern but a Query (Command/Query separation pattern) as it is a better fit when creating information for the UI.

          In my own service I only use repositories within my business layer. For all UI related data fetching I use queries which load data from a read model database.

          • Hung Nguyen

            Yeah, so I need to continue finding a solution to my concern. To be honest, I don’t really find that the Repository pattern is a good pattern. In a real life application, the behaviour are not always simple CRUD methods. And I don’t like to build an OrderRepository which have methods doing JOIN between different other entities because doing that will make the OrderRepository itself does not like “an in-memory collection of Orders to do Add, Insert, Update, Delete” as what the pattern is originally defined.

            There’re also a few argues about whether we should use the Reposiroty pattern, for example this one from Ayende http://ayende.com/blog/3955/repository-is-the-new-singleton. But people over the world are still using the pattern without any concern about what it’s really useful for. Maybe because the pattern is described by Martin Fowler :) :) :)

          • http://blog.gauffin.org/ jgauffin

            The pattern is really useful. The thing is that people tend to take a technology and then try to fit the pattern to the tech instead of the other way around.

            In this case your struggle is trying to make repository pattern work with EF when you really should try to make EF work with the repository pattern.

            The reason that I like the repository pattern are mainly to things:

            1. Less complexity, I don’t have to care about what tech is used to fetch the data, I just know that I get the data that I need.

            2. It makes unit tests a lot easier as the repos have clearly defined methods instead of some LINQ chunk that can’t be verified.

          • Hung Nguyen

            Thank you for replying me quickly. It looks like we’re discussing F2F :).

            Sorry, I’m well aware about your two reasons and also other ones about the beauty of the pattern. And yes, my problem is I try to make a specific technology like EF to work with the pattern, yes it maybe also a reason why MS guys recommend to NOT apply this pattern from EF version 6 (http://www.asp.net/mvc/tutorials/getting-started-with-ef-using-mvc/advanced-entity-framework-scenarios-for-an-mvc-web-application#repo). But I believe that my problem is good example for something say “a distance between theory and reality” :)

            With regards to my case, it would be great if you can post a new blog with a real sample code about “Best practice to effectively implement the Repository pattern with a complex JOIN using Entity Framework / NHibernate ?”

            Anyway, I really like your blog and I’ve learnt a lot from your articles.

          • Hung Nguyen

            Where are you gauffin ? Don’t tell me that you’ve added me to your haters list he he he

          • http://blog.gauffin.org/ jgauffin

            nope. just have a lot to do. :)

          • Hung Nguyen

            Man, now I understand. Repository is actually a pattern introduced with Domain Driven Design. The problem is, almost of us, not include you jgauffin :), is trying to apply the pattern in the approach and thinking of Data Driven Design. If we go with DDD, then when modeling the business domain we MUST forget the underlying storage mechanism, forget about each specific RDBMS table, and focus on define business entity which are aggregate roots. Then a repository is an in-memory collection of an aggregate root. For example, we should have an OrderRepository for every order-related specific tables, then Add an Order means “adding an order with all of its related tables”

          • http://blog.gauffin.org/ jgauffin

            I use repository pattern to achieve and abstraction between the business logic and the persistence layer, and by doing so reducing complexity.

            That will of course require more from the repositories. Your pain is probably from trying to have a 1-1 mapping between your domain model and the persistence layer, when in fact the repository might return different classes from the same repository (which requires manual mapping between your entity framework entity and the domain entity).

            Sure, it requires more initial work, but in the long run it will pay off thanks do a less complex domain model.