Ayende @ Rahien

Hi!
My name is Ayende Rahien
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

@

Posts: 5,947 | Comments: 44,540

filter by tags archive

Kobe – Data Access done wrong


From the Kobe documentation:

The Repository pattern is applied to mediate access to the data store through a Repository provider.

From PoEAA:

A system with a complex domain model often benefits from a layer, such as the one provided by Data Mapper (165), that isolates domain objects from details of the database access code. In such systems it can be worthwhile to build another layer of abstraction over the mapping layer where query construction code is concentrated. This becomes more important when there are a large number of domain classes or heavy querying. In these cases particularly, adding this layer helps minimize duplicate query logic.

I think that Repository is a fad, which is why I don’t use it much anymore myself, and looking at the usage semantics of the “repositories” in Kobe, they certainly don’t fit the bill.

Repositories are here to encapsulate query logic, that is not the case with Kobe. Here is a partial view of some of the repositories that it have:

image

In general, looking at the any of the repositories in detail, I really want to cry. I mean, just take a look:

image

More to the point, let us look at GroupRepository.AcceptGroupInvite (I removed all the copy & paste crap to concentrate on the actual code):

GroupInvite groupInvite = _context.GroupInviteSet
                                 .Where(gi => gi.GroupInviteId == groupInviteId)
                                 .FirstOrDefault();

if (groupInvite == null)
    throw new DataException("Invalid Invite Id", null);

groupInvite.InvitationStatuses = _context.InvitationStatusSet
                                        .Where(s => s.InvitationStatusName == "Accepted")
                                        .FirstOrDefault(); ;

_context.SaveChanges(true);

There are so many things wrong with this approach that I don’t even know where to start.

You can see how the architects or developers resented having to use an ORM. If they couldn’t have stored procedures in the database, then by Jove and his keyboard, they will write stored procedures in code.

Kobe uses Entity Framework for data access, and they treat it as if it was a dataset. As an OR/M aficionado, I am insolated. This shows complete lack of understanding how Entity Framework work, doing a lot more work than you should, brute force & big hammer approach.

From an architectural perspective, most of what Kobe is doing is calling down to the repository to perform the data access details. Business logic, such as there is, is spread between the data access, the controllers and the service layer, in such a way that make is very hard to have a good idea about what is actually happening in the application.

Data access details are quite pervasive throughout the application, and the whole feeling of the application is of a circa 2001 sample app using stored procedures and hand rolled data access layers.


Comments

Nathan

I think that Repository is a fad, which is why I don’t use it much anymore myself

Would you mind writing some more about this in a new post? I'm intrigued.

Rafal

How dare you say bad things about next sample from Microsoft? You barbarian, biting a hand that feeds you! MS programmers were working on it since 2001 so how can you expect the app to be modern and cutting edge?

AdamB

What alternative would you use to a Repository? Just ORM code such as the NHibernate DetachedCriteria API, HQL or LINQ in your services? Or even in your controllers? I'm asking because I respect your opinion and I've been using repositories for a while now, and find that they're a pretty useful abstraction.

CaliCoder

"Data access details are quite pervasive throughout the application, and the whole feeling of the application is of a circa 2001 sample app using stored procedures and hand rolled data access layers."

HAHA this is classic! Writing those factories is like chasing watching a cat chase its tail... go find a biz problem that needs data access, abstract it, go find a biz problem that needs data access, abstract it. Makes my hand shake just thinking about it =(

James

I agree with AdamB. Ayende, could you please expand on this?

Nik
Nik

Another agreement with AdamB :) I wanna know!

Nick

+1 on repo alternative.

J Healy

Count me in with the RepositoryAsFad crowd...

Adriaan

+1 on repository alternative

James Brechtel

Ditto on what everyone else is saying... :)

Flaker

Popular demand!!! repository alternative!!!

LukeB

I think the point that Ayende is making is that a lot (most) of those methods are really domain behavior that don't have anything to do with data access per se and thus shouldn't be part of the data access layer.

Without looking at the code I suspect that this codebase has an anemic domain model.

tawani

Who ever wrote that IUserRepository should be fired immediately. As a matter of fact, they shouldn't be allowed to touch code any more.

... Since everybody now an interface should have 100 methods...!

josh

hmm.. interesting comments. I'm partial to Rails's ActiveRecord with dynamic accesors, but you don't get that in c#. Otherwise, I have no strong bias for or against repository, and am curious why so many comments are.

Rob Gibbens

I also agree...I post on repository alternatives would be great. What's the strategy to keep the data access/orm choice (NHibernate / EF / Linq2Sql / etc) isolated and out of the "UI" layer?

Jon Galloway

Agree with the rest of the commenters - I'd love to see more explanation here. The rest of your Kobe posts have said "This is what is wrong, and this is how it should have been done". This one just says "This is wrong".

For those of us who don't use repositories and have never used EF, how should that that method be written? Or how should the repository be structured so the method was unnecessary?

Ayende Rahien

Jon,

The problem is that this method should NOT be written.

This is not the proper layer for it.

As for constructing the repository for this. All you need is transparent persistence and you are golden.

Jim
Jim

Lot's of examples on the web of how not to do things...how about some examples of doing things right, or refactoring bad code like this into something that's ideal?

redsquare

There seems plenty of people who are willing to moan at every app that is put out in the public domain and actually spend hours writing blog posts to pull them apart. What is funny is that they all state they cannot put together a best practise demo because nobody will pay them to do so. Surely the time spent moaning would be better spent providing?

stuartd

re insolated = 'insolate - expose to the rays of the sun or affect by exposure to the sun' did you mean insulted?

Dave Meah

Don't just say it's bad show how it can be improved, with examples. Good and bad examples should be shown when writing this type of article.

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

No future posts left, oh my!

RECENT SERIES

  1. RavenDB Sharding (2):
    21 May 2015 - Adding a new shard to an existing cluster, the easy way
  2. The RavenDB Comic Strip (2):
    20 May 2015 - Part II – a team in trouble!
  3. Challenge (45):
    28 Apr 2015 - What is the meaning of this change?
  4. Interview question (2):
    30 Mar 2015 - fix the index
  5. Excerpts from the RavenDB Performance team report (20):
    20 Feb 2015 - Optimizing Compare – The circle of life (a post-mortem)
View all series

RECENT COMMENTS

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats