Aren't GetAll & FindBy redundant?

Wouldn't it be clearer with a single "AsQueryable()" method or similar?

Having the single queryable method makes mocking ridiculously easy. Using a similar interface, along with Machine.Specifications & Machine.Fakes.Moq

List<Foo> FooRepository = new List<Foo> { ...... }

The<IRepository<Foo>>()
    .WhenToldTo(x => x.AsQueryable())
    .Return(FooRepository.AsQueryable());

Nothing like being able to mock a database with a list. With a 2nd method you'd end up needing to basically duplicate the above mock but forward the where predicate to FooRepository.AsQueryable().Where(). Kinda lame to mock the same thing twice but only adding a .Where() to it.

I had come up with a similar pattern in my own work and I followed a similar path to you.  The main differences were that I was using an ObjectContext and I had to return IList<T> rather than IQueryable<T> due to architectural constraints.  But I missed the FindBy with a predicate function - I'll have to start using that.

One thing I'd suggest is to make the Context property in GenericRepository protected so that only the child repositories can get at it otherwise you defeat the object of encapsulating the DbContext.

Anyway, great article.  Keep 'em coming!

Nice, but how to use a linq join to other tables!

Woudlnt you need to dispose the dbcontext? Im used to using the "Using" statement to make the connections, and im not seeing this being done... it may be my lack of experience in the EF, but i would assume you would need a way to dispose automagically... Does it need to be explicitly called?

I may have answered my own question with more research. I guess it doesnt need to be explicitly called since the context doesnt hold open the connectiion...

http://www.whitneyland.com/2008/06/linq-datacontex.html

http://weblogs.asp.net/scottgu/archive/2007/08/16/linq-to-sql-part-6-retrieving-data-using-stored-procedures.aspx (see comment by scott gu near the bottom)

Thanks though... i do love this DRY method a lot...

You would have some issues in the case that certain logic behaviors that are shared across multiple business classes, will use different repositories and, the same way, would use different contexts

so here's my beef and I can't seem how to navigate this.  Even when you use POCOs you're still coupled to some EF responsibilities.  I want to be able to make model classes any ole way I want and persist them with EF (with some generic implementation if possible) or any other persistence model/device be it NHibernate, XML or simpleDb.  Any suggestions?

@Chris

IGenericRepository is not specific to the EF. IGenericRepository would be defined in your Model somewhere. The EF project would define an implementation of IGenericRepository in the abstract class GenericRepository as was demonstrated above. If you needed to access any other data sources, you could build out your own implemenation of the IGenericRepository for that type of datasource, whether it be NHiberante, XML, or whatever. If the code you are writing to hit your repositories references the Interfaces and not the Implementations and using Dependency Injection it would be really simple to change from one implementation to another with a simple factory class or StructureMap etc... with very little effort.

@Tugberk

Thank you for the nice generic Base Repo class. I have one question.

Why did you not put the GetSingle method to the other generic methods in the base repo class? 

Just like this sample code:

public T GetSingleById(Expression<Func<T, bool>> predicateId)

        {

            var query = GetAll().Where(predicateId).FirstOrDefault();

            return query;

        }

@Tugberk

Well the reason why I suggested that because I wanted to avoid using another class which has to implement the IFooRepository or IBarRepository. I think if this repository is really a generic repository then it should be used like this within a ASP.NET MVC controller:

public ActionResult Index(IGenericRepository repo)

{

var customers = repo.GetAll() // and all the other concrete methods from the GenericRepository class

  return View(customers);

} 

I do not want the overhead of a FooRepository or BarRepository.

I would also like to see or vote for it ;-) that you implement the the suggestion from Chris Marisic about the FindBy and GetAll().

I do Unit tests on my mvc controller and integration tests for my repository including the business logic inbetween. So I do not need to mock the DbContext as I do not unit test my repository. Hope I got it right and understood you right. Thats an interesting link: http://stackoverflow.com/questions/6766478/unit-testing-dbcontext

@Tugberk

ah I see you already found it out :P

http://stackoverflow.com/questions/8623664/best-way-of-testing-repositories-which-use-dbcontext

@Tugberk

Still one thing came to my mind. Do you see any reason why not to include the save method

public virtual void Save()

        {

            _entities.SaveChanges();

        }

into each Add/Delete/Edit repository method? That way I have not to call the Save explicitly and sometimes I could forget it...

Thanks a lot for this article !

My current implementation was exactly the one you used as First Approach (worst one) ... guess we read the same ASP.NET MVC book ;) I knew it was not the correct approach for my project and I was trying to find a way to refactor it. Your article greatly helped me through this.

I have however changed something in your implementation. What I thought was bad in my implementation (your first approach) was not only the fact that there was a lot of code duplication but also that I was completly trashing the nice UnitOfWork pattern being DbContext.

Indeed I was instanciating one DbContext PER repository, which is not (I think) a good thing. I had to run through a concrete problem to understand that. Everything was ok until a few days ago I had to do a complex linq query requiring doing an union on two DbSets (coming from two distinct repositories). I was thrown an ugly exception letting me know that no union was possible because unions needs to be done in the same DbContext (seems logical). 

I then realized that a single DbContext instance should be shared by all repositories, which is more than logical if you think about it, and correctly exposes the DbContext as a single UnitOfWork.

So what I did is that I changed GenericRepository<C,T> as being only GenericRepository<T>.
I have exposed also exposed the inner DbContext in GenericRepository as a protected field (so that only concrete repository implementations have access to it, I really did not want the domain library user to have access to the context, too dangerous I think and what is the point of the repo in that case ;)).
My DbContext implementation also inherits from an interface called IDataContext which is just empty (solely used for injection purposes). 

The concrete repository implementations are in charge of setting their GenericRepository dbContext upon creation (the dbContext instance is injected via NInject in concrete repository constructors, "InRequestScope").

This way with only a few modifications, the dbContext is only known by the concrete Repositories (for extensions) and to the GenericRepository but not to the outside world, and most of all, the dbContext is shared by all the repositories during the request (UnitOfWork spirit).

To put it in a nutshell, here is what I got :

** IGenericRepository **
public interface IGenericRepository<T> : IDisposable where T : class

** EfGenericRepository **
public abstract class EfGenericRepository<T> : IGenericRepository<T> where T : class
{
/// The concrete implementation ***MUST*** set this context 
protected EfDbContext _efDbContext; 

** EfDbContext **
public class EfDbContext : DbContext, IDataContext

** IDataContext ** 
public interface IDataContext
{        
} 

** ConcreteRepo **
// IDataContext is injected by NInject, InRequestScope
public EfGwConfigRepository(IDataContext context)
{
// Set GenericRepo context
_efDbContext =  context as EfDbContext;

** NInject config **
_ninjectKernel.Bind<IDataContext>().To<EfDbContext>().InRequestScope();

And that's it ;)

Please note that I'm far from being an expert in this field (or I wouldn't have implemented the worst approach in first place ;)), there may be better way to do this, but it worked fine for me and has the advantage of not complexifying the implementation.

Thanks again for your article. 

I prefer to handle my unit of work (e.g. DbContext) outside of my repositories. In your example you are creating a DbContext per repository. Normally I prefer to have a DbContext-per-request that is shared by all repositories.

I do this by abstracting DbContext into an interface and injecting this using a DI tool. Details on my post http://blogs.planetcloud.co.uk/mygreatdiscovery/post/EF-Code-First-Common-Practices.aspx 

Hello Tugberk,

Is it not that every time you create a FooRepository or BarRepository a new instance of type DbContext is created too? If yes, will there not be a problem with having multiple instances of the DbContext going out of sync?

Cheers,

Lisa 

This is a really nice implementation of a repository. But I've been wondering if a repository is the best approach for making testable data access with the Entity Framework. I've heard some conflicting arguements that left me a bit confused.

I blogged about them in this post: Do we need the repository pattern?

I've also blogged about the alternative method I used: Mocking your Entity Framework data context and testing it in .NET MVC

I'd be really interested to see what people think. I like my alternative and it's worked well for me, but I'd like to hear from anyone who knows what the pros and cons of both approaches are.

Just seen Ben's comment. Seems like he is using a similar method.

[Peluca suggests](http://www.tugberkugurlu.com/archive/generic-repository-pattern-entity-framework-asp-net-mvc-and-unit-testing-triangle#2303) a GetSingle(...) method which takes in a predicate:

    public T GetSingleById(Expression<Func<T, bool>> predicateId)
    {
        var query = GetAll().Where(predicateId).FirstOrDefault();
        return query;
    }

I like not having to implement the `GetSingle` method in all of my concrete repositories, but I don't like having to pass a predicate. However, can't we use the native `T DbSet<T> Find(params object[] keyValues)` method to take care of this for us?

> DbSet(Of TEntity).Find Method

> Uses the primary key value to attempt to find an entity tracked by the context. If the entity is not in the context then a query will be executed and evaluated against the data in the data source, and null is returned if the entity is not found in the context or in the data source. Note that the Find also returns entities that have been added to the context but have not yet been saved to the database.

So our implementation can be as simple as:

    public T GetById(int id)
    {
        return _context.Set<T>().Find(id);
    }
   
Granted this means we are constraining our method to accept an integer as the primary key, but that seems like a common enough situation to make this a feasible implementation.

Actually I think the proper approach is as Ben suggests to create a context-per-request, not per-repository. What if you need to interact with objects/queries across multiple repositories, as is often the case? Your unit of work object can keep track of the single context, and you can pass this from repository to repository.

In fact, if you dispose of the context too soon and then create a new one, you will also run into conflicts ;-)

Your generic repository can be modified very slightly to accept a context in its constructor:

    private C _context;

    public GenericRepository(C context)
    {
        _context = context;
    }

And then your unit of work, or whatever container class you want, can be in charge of passing the shared context into your various repositories.

Good post. Definitely agree, per-request is best (actually, per-whatever is needed is best, depending on your implementation, so let the IoC implementation dictate) - the repository doesn't need to know about the persistence details, the unit of work takes care of that, including transactions, etc...
Using a UnitOfWork across repositories minimizes the need to open and close connections to the database, and also allows you to create service methods where the business logic transcends a particular repository. In SharePoint for example, I can then have 1 repository talk to a sharepoint list, another talk to a set of database tables, and if the repository that talks to the sharepoint list fails a check, I can kill the transaction which effectively kills the transaction for the entire request, across repositories, which is nice. The UnitOfWork is not thread-safe, and doesn't need to be, it's per thread (typically). So passing the DbContext into a repository is a necessity in this case, although, the DbContext here is just an implementation of one UnitOfWork class, which doesn't only need to span a DbContext, but could be a broader Persistence abstraction on top of different types of persistences stores (like sql, sharepoint, and media storage for example all in one request).
I have one version of an implementation of a Repository and UoW (although not for Code-First, but it's pretty much the same and wouldn't be hard to convert) over here if you're interested.