Generic Repository Pattern With EF Core - Why It Sucks

And one more reason if I use repo pattern is the solution contains web, api and console , so that I add project reference and share use same repo as well as models and service.

I have been struggling on use Repo design pattern or not in long time, I come from 10 years+ .net background and few years work experience in spring boot. My thought is very depend the tech stack used and the project scale.

EF is good, it is very good, it come from most of need for CRUD, work with Dapper for very complex SQL is perfect match.

Handling the EF calls directly in the Application layer seems to go against the Clean Architecture recommendation to handle persistence in the Infrastructure layer? Your videos are always on-point and informative.

Wonderful to find this video, finally someone with common sense and an explanation straight to the point. Thank you!!!

I would argue that a DbSet is already a generic repository

Tried using the dbcontext directly in a project recently and it was fun till I started writing my unit test it felt like I started wrting more code , I still prefer the write a generic respository with all in include params for joins in queries. Better reuse of code for me than witting specific repository. I mean why repeat myself.

But didn't you use a generic repository in your last paid course? 🤔

EF already implements the repository and unit of work patterns. You don’t need to implement them again on top of what is already there.

problems with the repository (applied over EF):
1. you always save UoW (context) and thus, repository save does not make sense
2. repos should handle aggregates, not entities
3. not every aggregate needs add or delete (add is necessary to top-level object, rest can be saved via reachability)

so the only benefit of a repo would be some shielding from EF, and it had to be done without introducing ambiguities and without ignoring basic definitions of that repo.

Oke I get it from this video we shouldnt use repository pattern.

I'll add to the anti-pattern that GetAll method. What happens when someone decides to fetch the whole database with it!!!

Agree and disagree with this. If an application is simple and has straight forward crud operations then I agree, this would be an overabstraction and worst and misdirection at best. However, when there is a good deal of business logic involved, I like to move all my db code (including all entity searching) away from the business logic layer. So in this instance would create a generic repository pattern for this along with a unit of work implementation. I don't want my business logic knowing anything about the database specifics and I especially don't want to inject a db context into it. I can also mock the repositories for testing. It's all very well to just say 'Oh just use it directly' but you soon get into messy cross cutting concerns without that abstraction. I definitely would not have DB LINQ code anywhere but a repository. Also, why do you insist in using MediatR for everything? Just curious.

Don't Repeat Yourself, so Generic Repository gives you that and only that and your GetQueryable() should be protected not public...

For the Generic Repository Pattern (without EF Core, I prefer Query builder like sqlkata), I prefer to have a abstract GenericRepository<TEntity> that implements the base CRUD actions. Every methods should also be virtuel. And Just have a CustomerRepository : GenericRepository<CustomerEntity>. Now the CustomerRepository have all the default methods and you can easily add more methods in the CustomerRepository without creating clutter for other entities/repositories. + if you need to different implementation of the GenericRepository, you can just override it. (afcours, also use interfaces)

I have worked in projects with already existing databases and usinng the repository pattern saved a lot of time and code. It's not the first way to go for coding but I believe it has it's purposes when needed.

100% Agree. And get worse and worse introducing something like IUnitOfWork and IService<TEntity>. And implementation is only passing down and up through layers. Aaand then Big Ball of Mud growing and growing :D

Generic repository in my previous project are there to DRY the code. It contains functions for general crud. But it is the abstract class. It is inherited by specific-repositories. So all specific-repositories will have all simple methods without copy-paste the code.

We have a kind of generic repository pattern i quite like at work. There you still implement a repository for each entity, but inherit from the Generic Repository. This way you can always override the generic repository to do entity specific things that the generic repository doesn't provide and you don't have to implement the same methods again and again.

Gen Rep pattern goes well with Unitofwork and as for querable u can make an iquerable method to return an iquerable so i do not understand why its pointless , u dont want teh devs to use the dbcontext anways hence u wrap it and allow only certain methods , also if u need to do generic or custom filtering or some logging of prev and next state in entities if u dont have a gen repo where u will do it ?

Greetings from Bulgaria. Thank you for your time and efforts to create and share those nice tutorials.
Regarding GetAll (either Async or not), this can not be serious. If we have 1 millions rows in the DB with 50 columns, some may be nvarchar(max), i.e. spreading across multiple data pages ... !!???
For the number of columns the same thing follows even for GetById. We usually do not want to pull all columns. Both for load's sake and for security. So here come my next concern.
Another good and very common practice is to use intermediate VM classes / entities or as some call it DTO. We do not want unload out whole data table to the end user. So ?? We need mapping. Somehow everyone misses to give examples for that case.

Hello Milan, I have a question please.
Why Insert/Update/Delete methods are made sync not async ?

What if all my repositories have common methods only (0 specific methods). In this case what is a better option than the Generic repository pattern?

Insert, update and delete should be in the unit of work. Instead of GetOneById it's better to have GetOneAsync(Expression<Func<TEntity, bool>> filter) - here you don't need to know the type of the primary key. Regarding the GetAllAsync. I think it shouldn't be a part of a contract and should be implemented as protected method that returns IQueryable<TEntity> in the base repository class which implements IRepository interface

Interesting.
I've used it in a few projects I've worked on but not just this implementation;
instead, I've used it as the base repository for every repository I've created, and then implemented the unit of work pattern to wrap the savechanges method with it.. is that ok?

This is not the way to do it!

I don't completely agree. We can implement specification patterns to avoid exposing IQueryable from repository.

But in some cases we also need to expose the IQueryable outside repositories because GraphQL does the paging, filtering, sorting and even projection if we return an IQueryable from GraphQL queries.

How about moving those together?
Repositories should take a specification in and return an IQueryable instead of specific things like IEnumerable, List of IAsyncEnumerable. Let the caller do that however they need it.

Repository is not only used to be able to switch ORMs, it is used to abstract from the db provider code. That way a repository is a single place in your app that knows about EF Core. I have written many assemblies and libraries that were shared among multiple projects so when working in a product company. If you abstract from Ef Core or whatever ORM you use - you can update the version of Ef Core in a single click - just a project with repos implementations and the project where it is referenced (mainly it is Microsoft.Extensions.DependencyInjection type of project that gathers a set of projects under its "rule"). The same with Refit, when you have multiple librariries using miltiple API Refit clients you end up with updating tens of projects and pushing them to nuget just to update the Refit dependency. So we abstract from Refit and all calling projects use ISomeTypeOfApi interface without knowing they're working with Refit

I use a different approach when working with repositories. I encapsulate more logic into my repos: so I would have a single method in my repo that selects a customer and adds an order right into the database. I don't like exposing EF Core to other layers of my code, so my services or CQRS stuff only knows about repositories. That way my application becomes more unit testeable. I use a generic repository but it is abstract. All my real repos inherit from base repo and add any new methods specific to a concrete repository, that way I don't expose IQueryable outside of my repo. Generic repo is not a bad thing in the right hands and can help you get rid of some of the boilerplate code

I used to use generic repository pattern with reflection dependency injector to be able to easily create API endpoints. It also allows to overwrite it with custom implementation

I agree here, but using generic repositories makes writing Unit Tests easy. I use FakeItEasy. Can I use FakeItEasy with DbContext directly?

I totally disagree. You can use the UnitOfWork with the Generic repository to call savechanges. You can also create a regular repository and inherit from the generic to avoid repeating code and use that. You can also create a base service class that calls savechanges automatically on dispose and inherit your service class from that. I have my own implementation of the generic repository with UnitOfWork as explained and it has dramatically reduced the logic I write with absolutely no limitations or issues. I think that the problem of your implementation is that it is designed to be just a wrapper around ef core. A decent implementation is super useful and a productivity booster.

The eternal battle... I agree how you did it is flawed. It works very well when used with Steve Smiths Ardalis Specification package, none of that returning an IQueryable. That plus mediatR gives you excellent separation of concerns.
When you want to make it hard for a dev to do the wrong thing, don't take an application dependency on ef. If that is not an issue or concern then go for it. It's all trade offs there is no "right" way and everyone should be doing X, do what works best for your team.

With all due respect , many developers are doing repository pattern wrong and that's why it become an Antipattern ill have to disagree with you Milan and here is my take on this matter.

A repository pattern should act like a collection of object in memory , this is stated on Martin Fowlers webpage . So it should not have a semantic like a database – there should not be methods like Update and Save in a repository.
If we compare it to EF, EF don’t have a SaveChanges on its dbSet! This is where I see many developer have done it wrong.
Having Save in each repository will lead to confusion, as you stated which repository should I call the Save Upon? Its not the repository responsibility to Save , Its the Unit of Work.
Instead SaveChanges should be implement in Unit of work this way we can handle all transaction in one call and ensure data consistency.

The Context and Unit of work is responsible of saving changes. If we compare EF and RP then the repository is similar to dbset and Unit of Work to context.

One of the benefit of the repository pattern is that it minimizes duplicate query logic. It should return IEnumerable or entities
So a repository pattern should encapsulate those queries, by returning IQueryable from a repository is a wrong implementation , its complete against the idea of using a repository in the first place. And returning IQueryable can give the wrong impression to the upper layers that they can use this GetQueryable to build queries…. But hey repository pattern should encapsulate queries…. you see the problem here?

Without a IBaseRepository or IRepository , we end up writing Add() Remove(..) Get(..) GetAll() Find(predicate) SingleOrDefault(predicate) etc. in every repositories. That’s silly. We want to use RP to minimize complexity not add complexity.
Therefore use a BaseRepository with the generic method thats should be used across all repository , return IEnumrables , and entities not IQueryables, and use Unit of work to save all transaction in a single call.

And Ive seen developer thinks that they have to implement in each repository a method like this GetMovieWithAllCasts(); and in another repository GetCastsAndAllTheirSalary(); whenever they need a custom query and return an IEnumarble or Entity . and it keeps repeating itself across many repositories. Because they could not see how this could be implementet in a generic repository, and the solution to this of what ive seen is The specificationPattern. Ive actually done this aswell but for me its just add more complexity to the code. It helps with the problem but I didn’t like it. Then I figured out how to encapsulate those includes and aggregates in the BaseRepository to return IEnumerable or Entities.

All my repositories is empty, because my BaseRepository can solve all what its required for the business rules, and if I need something that is so speicifc to that repository that the base cannot solve I will add it to that specific repository, we don't have to expose IQueryable and we shouldn't to begin with

I can agree with you that a bad implementation of repository pattern is an anti-pattern and a useless wrapper.
But a good one is not, I have never used repository pattern without Unit of Work and I don’t return IQueryables. RP + UOW helps us adhere to DRY and SOLID principles when done right. Its not a useless wrapper.

And some developers argument is that the repository pattern and entity framework are the same – so there is no reason to implement them together. But the Repository Pattern can be designed to work with multiple ORMs or data access technologies, it provides a layer of abstraction between the application's data access code and the underlying storage mechanism. Where EF is a specific ORM Technology that comes with a set of functionality, we cant incorporate another ORM within it. Its simply is not the same and wrong to say that repository pattern/ Unit of work is an anti-pattern.

Unless you are just saying that Repository Pattern without Unit of Work is an anti-pattern, then I just find the door out myself xD
Ill give you a thumbs up for making a video about this but ill highly disagree with the content

If you like to debate more about this lets do it on linkedIn where we already are conversing xD Best regards

I am completely agree, that it's suck in the way you think this interface up and implement it.

So, what's the cleanest way to encapsulate data access logic instead of having this repository?
Making a custom repository inherit from the generic one for the custom/complex scenarios will help in this case?

And what if we gonna change the db engine, depending on context directly will push us to many changes in many places, right?

yes , I also believe this sometimes trying put everything into project without knowing the purpose of it , and UOW also we can ignore. but I've seen some documentation the repository should not be exposed to IQuearyble, who cares :)

Agreed - I've regretted it each time that I implemented the generic repository pattern. It felt pointless in exactly the way that you described.

Great content! But, for example, you didn't call Dispose() for the DbContext. With the repository, this would be centralized and transparent!

In some projects, we actually mix Queries with Dapper with persistence made with EF. Would using a Repository be better in these cases?

Its important to note that there are different repository patterns. With DDD, it is specifically for interacting with Aggregates, which is different than general DB queries. I'm considering calling it something like an 'aggregate store,' to distinguish.

The general repository pattern is a big problem.

Thanks for the video Milan.
I think Generic repository with EF Core doesn't make much sense. I actually think repository pattern doesn't make much sense with EF Core either.
BUT if you're using Dapper that's a whole different story. Then both repository and generic repository basically become a must have and save you a lot of time and trouble.

Well I've been using generic repository pattern with unit of work and generic specification pattern (my customized pattern). The issues that you told have all can been handled. Moreover as others have mentioned unit testability increases dramatically with this approach. Also when u need a specific type of query you can create it.

You didn't implement Generic Repository pattern correctly. For all methods just add predicate, include and enableTracking parameters, to make it flexible for any type of queries - then use it. Also you can use Unit of Works.

Repository pattern with multiple derived classes work best with unit of work you know 💁‍♂️

I've seen generic repositories using T or TEntity... what's the difference, if any, and which is the best to use for generics?

Totally agree. It is just a wrapper around an already perfect wrapper like ef core is. Anyway I like using repository pattern and depending on abstractions. In the future you might need to change infrastructure, like for example move to dapper or to any other ORM, so sometimes it can be useful

Hmm.... I wonder did any of you heard about Specification pattern which could be used here instead of that method returning IQueryable? Generic repo has much more sense with it, and Ardalis has quite handy implementation I think...