20

it seems to me that a lot of specialised repository classes share similar characteristics, and it would make sense to have these classes implement an interface that outlines these characteristics, creating a generic repository

to illustrate my point, say we have this code

public class IEntity
{
    public int Id; 
}

public interface IRepository<T> where T: IEntity
{

    IEnumerable<T> List { get; }
    void Add(T entity);
    void Delete(T entity);
    void Update(T entity);
    T FindById(int Id);
}

[Table("Author")]
public partial class Author : IEntity
{
    public int Id { get; set; }

    [Required]
    public string authorname { get; set; }
}

and then we go onto implement these interfaces to create our specific repositories

public class AuthorRepository : IRepository<Author>
{

    Model1 _authorContext;

    public AuthorRepository()
    {
        _authorContext = new Model1();

    }
    public IEnumerable<Author> List
    {
        get
        {
            return _authorContext.Authors;
        }

    }

    public void Add(Author entity)
    {
        _authorContext.Authors.Add(entity);
        _authorContext.SaveChanges();
    }

    public void Delete(Author entity)
    {
        _authorContext.Authors.Remove(entity);
        _authorContext.SaveChanges();
    }

    public void Update(Author entity)
    {
        _authorContext.Entry(entity).State = System.Data.Entity.EntityState.Modified;
        _authorContext.SaveChanges();

    }

    public Author FindById(int Id)
    {
        var result = (from r in _authorContext.Authors where r.Id == Id select r).FirstOrDefault();
        return result; 
    }
}

before i implemented this, i went out a did a bit of research about whether it was a good idea or not, and all the information i could find were statements calling it an anti-pattern but without explaining why.

Why is a generic repository considered an anti-pattern?

bbqchickenrobot
  • 3,592
  • 3
  • 45
  • 67
  • 5
    It's an abstraction of an abstraction and leads to the ever-widening interface problem. Rob Conery sums it up nicely (as a bit of a sidebar to UoW on top of Repository) here >> http://rob.conery.io/2014/03/04/repositories-and-unitofwork-are-not-a-good-idea/ – Matt Jul 20 '15 at 12:20
  • 4
    Can you provide a link to the sites that called this an anti-pattern? – Nimrand Jul 20 '15 at 12:20
  • 2
    You're better off asking this question on [Programmers](http://programmers.stackexchange.com/) – Jason Evans Jul 20 '15 at 12:22
  • 7
    not only are you wrapping a well known, tested repository (Entity Framework) inside a less feature rich interface, you are also artificially limiting the functionality that the consumers have, for little benefit. For example, why have an implementation of `FindById` that performs a where clause when `db.Author.Find(someId)` would do the same thing? – Claies Jul 20 '15 at 12:24
  • 1
    @Claies: That's what I called "wrappitis" :-) http://stackoverflow.com/questions/1667608/is-it-a-good-practice-to-create-wrapper-over-3rd-party-components-like-ms-enterp – Stefan Steinegger Jul 20 '15 at 12:33
  • 43
    Ah, I really do hate it when folk close a good question, alleging it to be opinion-based. :( – David Arno Jul 20 '15 at 12:33
  • 3
    Unfortunately, because the question is closed I can't answer properly, but will do so via comments as best I can. Any such generic repository has to be structured in such a way that it will work with any query on any part of a data source. To do that, it has to expose an entire data set through `IQueriable` or `IEnumerable`. By doing that, the implementation details are being leaked out in order to allow the code using this repository to build its own queries to obtain subsets of data. This is what's known as a leaky abstraction... – David Arno Jul 20 '15 at 12:37
  • 2
    I disagree wholeheartedly. The anti-pattern is letting a generic repository be used in the application. Implementing a non-generic repository from a generic-repository interface is just fine. You *want* to restrict what downstream callers can do. You *want* to keep EF specific logic well contained within your repositories. – MetaFight Jul 20 '15 at 12:37
  • ... (eg see http://www.joelonsoftware.com/articles/LeakyAbstractions.html). Leaky abstractions are regarded (at least by some) as an anti-pattern, eg see http://www.ben-morris.com/why-the-generic-repository-is-just-a-lazy-anti-pattern/ – David Arno Jul 20 '15 at 12:38
  • 4
    Repositories are a great abstraction. Each repository should be specifically coded to meet only the needs of that data set, eg you might need to keep a record of transactions forever, so there should be no delete for that repository. It can therefore be argued that DRY is a less important principle than a good abstraction in this case, thus the anti-pattern claim. – David Arno Jul 20 '15 at 12:41
  • @Matt, did you even read the article you linked? It has more to do with abusing the Unit of Work pattern than generic repositories. – MetaFight Jul 20 '15 at 12:43
  • 1
    @Claies it sounds like you're arguing *against* encapsulation. – MetaFight Jul 20 '15 at 12:51
  • @MetaFight Yes. I mentioned in my initial comment that the article is more to do with the usage of UoW and Repository, but there is an, admittedly small, passage about why Repository is a bad idea. In my humble experience generic repositories are nearly always implemented prematurely - by the time you discover that the abstraction is leaking it is a significant enterprise to remove it. – Matt Jul 20 '15 at 12:51
  • The problem here, though, is that the OP isn't actually using a **generic repository**. He's using non-generic repositories that implement a **generic interface**. As such, he doesn't have the classic leaky-abstraction problem that generic repositories have. The point the article you linked is making is that this pattern still doesn't solve the repository-UoW awkwardness. – MetaFight Jul 20 '15 at 12:53
  • @MetaFight there is a time and a place for encapsulation; generic repositories are not encapsulation, they are an attempt to generalize something just because method signatures are similar, and that's not the same thing. – Claies Jul 20 '15 at 12:54
  • Agreed, and the OP isn't actually using a generic repository. – MetaFight Jul 20 '15 at 12:55
  • 1
    @DavidArno: Just by looking at the comments you can see that it is opinion based ... (which doesn't mean that it isn't a good question.) It just means that it cannot be answered "correctly" but causes discussions and therefore it is off topic on SO. – Stefan Steinegger Jul 20 '15 at 13:01
  • @MetaFight it may look like it's not generic, but in reality, 90% of the implementations of the `IRepository` that is presented here are going to look identical (take an object of this type, add it to the context, save changes). That being said, this discussion in the comments over the semantics in the code is why the question was closed as opinionated. – Claies Jul 20 '15 at 13:02
  • It doesn't look generic because it isn't generic. The fact that 90% of the implementations will be nearly identical has nothing to do with the Generic Repository pattern either. It has to do with the fact that this generic interface should actually be a generic abstract base class. – MetaFight Jul 20 '15 at 13:04
  • 2
    I would consider exposing persistence details (like EF-contexts, DbSets, ...) to your domain an *anti-pattern* - I hope you all have fun mocking those for your unit-tests ;) – Random Dev Jul 20 '15 at 13:05
  • @StefanSteinegger, You are right. I was just frustrated that I'd nearly finished writing my answer when it was closed. – David Arno Jul 20 '15 at 13:06
  • @Carsten please actually read the code. That isn't happening in this instance. – MetaFight Jul 20 '15 at 13:12
  • 1
    @MetaFight what code - the OPs code would be fine with me - I am talking about some of the upvoted comments that seem to be fine with exposing those – Random Dev Jul 20 '15 at 13:16
  • This seems to be better moved to https://codereview.stackexchange.com/ – zwcloud Jul 07 '20 at 10:20
  • @zwcloud This question would be off-topic on Code Review as it is asking about generic best practices. Please familiarize yourself with what is [on-topic](//codereview.stackexchange.com/help/on-topic) and our [guide to Code Review for Stack Overflow users](//codereview.meta.stackexchange.com/a/5778). – Peilonrayz Jul 07 '20 at 10:24
  • Because Entity Framework implements repository pattern and unit of work in DbContext. Writing Repository for DbContext you’re creating abstraction over abstraction which additionally removes some of EF features. – masoud rafiee Oct 24 '22 at 09:08

0 Answers0