17

It seems that every example I find of the repository pattern, the implementation is different in some way. The following are the two examples I mainly find.

interface IProductRepository
{
    IQueryable<Product> FindAll();
}

There is then usually another layer which talks to the repository and calls the FindAll() method and performs any operations such as finding products beginning with the letter 's' or fetching products in a particular category.

The other example I find a lot put all of the find methods into the repository

interface IProductRepository
{
    IEnumerable<Product> GetProductsInCategory(int categoryId);
    IEnumerable<Product> GetProductsStartingWith(string letter);
    IEnumerable<PromoCode> GetProductPromoCodes(int productId);
}

Which path do you recommend I take? Or what are the advantages/disadvantages from each other?

From my understanding having read http://martinfowler.com/eaaCatalog/repository.html the first approach seems to best reflect this?

Scott
  • 281
  • 2
  • 8

5 Answers5

16

The first one is horrible. IQueryable is like a GOD object. It's really hard to find a 100% complete implementation of it (even among all OR/Ms). You can expose your ORM directly instead of using it since you'll probably get a leaky abstraction layer otherwise.

Joel says it best (text is from the wikipedia article):

In Spolsky's article, he calls attention to many examples of abstractions that work most of the time, but where a detail of the underlying complexity cannot be ignored, and thus drives complexity into the software that was supposed to be simplified by the abstraction itself

Joels blog entry

The second approach is much easier to implement and to keep the abstraction intact.

Update

Your repository is violating Single Responsibility Principle since it got two reasons to change. The first is if the Products API is changed and the other is if the PromoCode API is changed. You should imho use two different repositories like:

interface IProductRepository
{
    IEnumerable<Product> FindForCategory(int categoryId);
    IEnumerable<Product> FindAllStartingWith(string letter);
}

interface IPromoCodeRepository
{
    IEnumerable<PromoCode> FindForProduct(int productId);
}

Changed things:

  • I tend to begin methods with Find when several items are returned and Get if a single item is returned.
  • Shorter method names = easier to read.
  • Single responsibility. It's easier to tell what the classes that use the repositories have for dependencies.

Small well defined interfaces makes it easier to spot violations of the SOLID principles since classes the break the principles tend to get bloated constructors.

jgauffin
  • 99,844
  • 45
  • 235
  • 372
  • Thanks for the reply. I thought you were meant to group by aggregates? Should I have one repository per entity? Also here is an example of the first example I provided.http://stackoverflow.com/questions/5049363/difference-between-repository-and-service-layer – Scott Apr 17 '12 at 22:11
  • Per aggregate. I couldn't tell that promo codes was for products only. (since it was named `GetProductPromos` and not just `GetPromos`) – jgauffin Apr 18 '12 at 04:30
  • doesn't have your first interface, also 2 reasons for change? first: changing the FindForCategory method to change default returned Product if it don't find any product by passed categoryId, and second changing the FindAllStartingWith for apply some default filters? – Masoud Jul 23 '13 at 06:57
  • No. The responsibility (and the reason for change) is to abstract away the data source handling of products. If that handling changes, the class change. – jgauffin Jul 23 '13 at 20:33
1

Consensus is building: 2nd option all the way. Aside from query logic leaking all over the place with IQueryable, the difficulty in implementing it right, it is very difficult to TEST and mock.

n8wrl
  • 19,439
  • 4
  • 63
  • 103
0

Personally I would suggest using the second example, that way you are encapsulating the search logic in a single place and the intent of the caller is clearly defined by the name of the method they are calling. If you go with the first example, your query code will leak throughout your application and you will end up duplicating queries.

Trevor Pilley
  • 16,156
  • 5
  • 44
  • 60
0

I recommend to avoid duplications. Thats the first goal. If you have logic which finds products starting with some letter in several places, then it's a special case and its worth being extracted to separate method (also it gives good description for your specific case). Code without duplications much easier to change, understand and maintain.

So, I tend to have one generic search method with IQueryable and set of methods which used more than once:

interface IRepository<T>
{
    IQueryable<T> FindAll();
}

interface IProductRepository : IRepository<Product>
{
    IEnumerable<Product> GetProductsInCategory(int categoryId);
    IEnumerable<Product> GetProductsStartingWith(string letter);
    IEnumerable<PromoCode> GetProductPromoCodes(int productId);
}

Consider also unit-testing. Specific methods are much easier to mock, than IQueryable.

Sergey Berezovskiy
  • 232,247
  • 41
  • 429
  • 459
0

I actually think the 1st one is better. I assume my decision on the following factors:

  1. If product structure will get refactored:

    • IQueryable approach - you only need to change calling methods in code.
    • IEnumerables -you need to change methods names as well.

  2. If many about to derive the interfaces which you want to iterate in polymorphic way:

    • IQueryable approach - benefit of generic uniform method names
    • IEnumerables - some names might not be describing the method you need.

  3. Elasticness

    • IQueryable approach - easy to divide into IEnumerables approach.
    • IEnumerables approach - hard to convert back to IQueryable approach.

So, I suggest you start with IQueryable as default choice and as you progress with your code you can always change to the more specific IEnumerables approach you need.

PaulG
  • 13,871
  • 9
  • 56
  • 78
G.Y
  • 6,042
  • 2
  • 37
  • 54