Right now, it seems they have changed all thos IObjectSets to Lists... not sure if thats better though

PageIndex and PageCount are validated but later ignored. If IQueryable is not exposed they should at least use compiled queries.

The point should be: you can't abstract away data-access, your O/R mapper, etc.... they all leak into your application, simply because the services / features of the O/R mapper and data-access layer offered to the using code.

Trying to hide a given type of O/R mapper is not going to work, trying to do so is just a waste of time.

I don't think Ayende has anything against exposing IObjectSet. If anything, that's what's good about this example. It enables the Include in the query, which is also properly advertised in the method name.

Guess I have to take a closer look, but the way the unit of work is introduced is the strange part.

Is it me or does Ayende just trail off on the last sentence? Is the code that bad?

I might be wrong, but I think Ayende cringes by the fact that they are abstracting EF away into something that maps directly onto EF. It's like storing binary data in an XML-file. It just serves very little purpose.

"Of course, we expose Entity Framework types in our abstraction, but that is good, we will just have to implement EF on top of NHibernate if we ever want to switch, it is not like we are exposing a concrete class, it is an interface, we can replace it." Doesnt really sound like it

What about Unit Testing? Or should you just use a real EF ObjectContext/DbContext and a real database in your tests?

Well, Ayende was too disgusted to even finish his thought, but Frans did it for him. If you are going to use an ORM, USE the features it provides. You will not be swapping ORMs anytime soon on any "real" project.

On a lesser note, love the null check/exception throwing, I bet that is repeated 1000 times.

My gawd :( how hard is it really to return IQueryable<POCO> ??? Not hard at all :( Isn't that the very first thing we were all taught when we were learning how to use the Repository Pattern with EF?

+1 for Ayende's dark humour.

(and this is why i'm in love with RavenDB).

I think Ayende's point is that on one side they're trying to abstract away the ORM, and on the other side they use the features of that specific ORM (the .Include(...) method for example).

But I think ayende should clarify himself on what direction he would go. The abstracted ORM? Or like Frans Bouma said, using the ORM to it's full capabilities!

@Frans based on this: http://msdn.microsoft.com/en-us/library/ff714955.aspx I managed to create an abstraction over the database and run all the unit tests in memory. I am using the Code First API. It is working very well. I can definitely swap EF with NH but the LINQ provider for NH was not that great last time I checked.

This is starting to feel like the rages of a disappointed father.

It's funny that in 2011 microsoft is trying to teach people how to use ORM and how to write web applications. Didn't they learn anything in past 10 years? What now? Are they going to invent EJB?

"PageIndex and PageCount are validated but later ignored"

This is clearly insanity ... how could have believed?

Jokes aside: Time to short MSFT

@Cosmin

How have you managed to expose eager loading capabilities through your abstract repositories?

@Lee

For that I need to make some assumptions about the underlying framework as described in the article but that is just one place to change in the infrastructure and is hidden from the client.

unitOfWork.Users.FindAll().Include("Stuff").Where(...) as described in the article.

    public static IQueryable<T> Include<T>(this IQueryable<T> sequence, string path) where T : class
    {
        var query = sequence as DbQuery<T>;

        if (query != null)
        {
            return query.Include(path);
        }

        return sequence;
    }

This could be made type safe with some little work. In case of InMemoryRepository the include does nothing so unit tests just work.

To all those who are saying it's bad to abstract out the ORM, I don't necessarily agree. I think it's just bad if you do it like the example does it, or if you are using the Generic Repository pattern.

I have come up to the situation three times where I have wanted to switch out ORMs or even underlying database systems. The first is when I was using Linq-To-Sql and wanted to switch to EF4 because working with L2S became too much of a hassle due to differences in the database and c# data model. The 2nd time is in another project that uses polymorphic types in a relational database, and it appears that EF handles that horribly inefficiently and I probably want to switch to NHibernate instead for it to work decently. Finally the 3rd time is me wanting to switch database systems to RavenDB.

In each of these cases, because my data access wasn't abstracted out I have to go through all my business classes and update the queries to work correctly on the new ORM or database system, when if the data access is sufficiently abstracted out it becomes much easier to change. There are also times when individual queries need to bypass the ORM to be more efficient (patching with RavendB, complex queries that don't get translated right by EF, etc...) and these are much easier to handle with an abstracted data access layer.

Yes, abstracting out your ORM is an absolutely brilliant idea.

Only until you run up against a truckload of select n+1 problems and need to reach into the bowels of your generic one-size-fits-all repository and drag bits and pieces of your ORM kicking and screaming into your business logic so you can get queries that are actually capable of completing, like, this week.

Been there, seen it, done it, got the T-shirt.

@James

LOL. That about sums it up.

@James

:) I've got the Tshirt as well. Nothing stops you from bending the database to fit your requirements using some sort of CQRS where selects are ridiculously simple so simple that the ORM is not even necessary.

@James: I specifically said not to use a generic repository. A better method is to create repository methods that distinctly define your queries in detail and retrieves all of your data without relying on lazy loading. Somewhere in your code you have to create the code to run the exact query you are looking for, and I'm saying that code should be located in an abstracted data layer so that you can easily change it, either to use hardcoded SQL (if your ORM is generating bad script) or if you need to more easily swap out your ORM or database system without having to delve into your business layer.

My question ("What about Unit Testing? Or should you just use a real EF ObjectContext/DbContext and a real database in your tests?") was serious by the way.

How should you unit test classes that use your EF context directly?

@Kristof Claes

No one needs unit testing, what are we, Java? :)

@Krisof Claes: I use my EF DbContext in my "unit tests" (technically they are integration tests) to verify my queries execute correctly against a real database system. It's too easy to write some Linq that works correctly for Linq-To-Objects but fails with Linq-To-Entities, and if you don't make sure it works against a real database then the tests are almost useless (as they won't give you failures that will occur in production)

In my opinion, one of the more subtle problems in that code is the .BankTransfersFromThis property. If BankAccounts behave any where near what my bank account does, there will be loads and loads of BankTransfers from any given account. And when we publish them by making them a property on BankAccount, we throw away any means of paging them. So, that method will over time be a bottleneck.

Also, any a system but trivial ones, we would include some kind of transaction management (ie. which other parts of our domain was involved in that bank-transfer).

Lastly, BankAccount is now supposed to be responsible for saving BankTransfers which can lead to very brittle code down the way. Atomicity and concurrency problems spring to mind.

All problems that will surface long after the system is in production.

Nobody noticed the upcasting of UnitOfWork?

Also, where is the paging?? I don't see any Skip / Take ?

It's very difficult and costly to abstract away a persistence mechanism, even more so if you don't want the abstraction to limit performance, but it can be useful as as long as it

• remains centered on the domain and not on a particular storage medium, and
• includes explicit responsibility for synchronization of access.

It helps to think in "macro-OO" terms, where the running app is a "mega-instance" of the codebase's entire OO superstructure. This mega-instance can respond to simultaneous messages from multiple external actors, and internally contains shared state that these actors ultimately access and mutate. This state must be shaped to the specific needs of the mega-instance's domain (analogous to a single class's responsibility), and because the mega-instance is simultaneously invocable, access to it's state must be synchronized.

The implementation of the mega-instance's state is where persistence technologies fit, so there is a strong tendency to think of this state simply as a persistence store. However, the synchronization controls are just as important to shared state access in general (both in a class instance or an app mega-instance), and therefore synchronization responsibilities must be included in abstraction of the mega-instance's state. Thus the correct abstraction over an app's internal shared state is defined as the domain-specific integration/merging of external access and mutation events across an app mega-instance.

Such a abstraction permits all sorts of actual storage mechanisms, from STM, to RDBMS, document db or key-value store, file-based serialization, many of which may requires synchronization logic to be built on top of the external store before the implementation completely fulfills the abstraction. Transactional logic may or may not be included in the abstraction, in part depending on the preferred flavor of UoW pattern (caller registration vs object registration).

@remco, I'm not sure if its official "guidance" or not but from what I've seen the standard practice is to return all items and do paging at the UI level. As you would imagine, performance is less than great.

Where are pageIndex and pageCount used??? Is this a TDWTF?

Ahhhh stop! ^^ omg this is so painful because years ago I was like this for a few weeks until I understood why it doesn't help. And guess what, my opinion came form best practices.

This came from a group of microsoft services consultants in spain, So my point is this is not an "official" guidance, the official guidance comes from patterns and practices group in redmond , Just my 2 cents

@ Kristof I agree on that, you may want to abstract your UoW because of mocking and Unit Testing reasons, not just because of switching from one O/RM to another, which would imply many other difficulties. It is good to expose an explicit contract of your UoW.

Yeah, in practice, I agree. The characteristics of the persistence medium should fall in the architecture category. Without some very esoteric requirements, you don't abstract the architecture. (But if you do, it should be based on the conceptual foundation I described, which MSFT's clearly didn't.)

Didn't some smart guy on this site state, "The DAL goes all the way to the UI" once before? ;)

Seriously I cringe every time I see query APIs with pageSize, pageCount parameters. That's what your expression tree is for. I can forgive this say if we were back in .Net 1.1 days but this type of pattern is dead as can be. Please stop with the zombie-anti-patterns already.

"It is working very well. I can definitely swap EF with NH but the LINQ provider for NH was not that great last time I checked. "

No.

myOrder.Customer = myCustomer; bool isPresent = myCustomer.Orders.Contains(myOrder);

what is isPresent on EF (e.g. with STEs). And NHibernate? Or <insert ORM here> ? Differs per ORM. (isPresent is false for NHibernate, true for EF with STEs, true for llblgen pro etc.)

Just a simple example of how features bleed into the application code without you knowing it. There are more of these examples.

And then I haven't even mentioned features your code relies on which are present in ORM A but not in ORM B.

I guess I'll concede. After reading more posts on the whole DALs being bad (if created for the sole purpose of data-persistence abstraction) I am now seeing the points I was missing. I guess it is a bit of a waste and just creates extra effort down the road. It's definitely way too easy to get stuck in the "abstract everything" trap

On a project I was on, I decided it might be cool to use the Specification pattern for filtering in our MVC controllers in EF. This ended up being an insane headache with the advanced queries we needed to do plus with all the expression composing, etc. it was just too much abstraction and long-winded setup.

You know what turned out better? Just implementing the methods on our service interfaces and doing it in LINQ within our implementations and using DI within the controllers. I don't like methods that have lots of parameters but damn if it wasn't easy to use, read, understand, and test!

@Jimmy: Expression trees can hose you with some nasty performance problems. They are a leaky abstraction.

See my point to Matthew re abstracting your ORM above (and his reply that generic DALs are a bad idea.)

It violates a pretty basic programming concept. You have this member field, that without, the class cannot function. You pass it in via the constructor, and you null check it there.

You don't null check it everywhere you use it, and mask the error with InvalidOperationException. And you should have a dependency on the actual type you are dependent on (IMainModuleUnitOfWork).

If you did insist on that code, then I think an explicit cast would be a much better fit: IMainModuleUnitOfWork unitOfWork = (IMainModuleUnitOfWork)this.UnitOfWork; ---** Fail here if it's not the type *-- unitOfWork.GetCustomer()...

I also think you should extract the code used to check arguments into a re-usable class. It reduces LoC and makes unit testing easier. Assign the member field and null check all in one operation (at least from a caller's perspective): ctor ( Arg1 arg, Arg2 arg2, long numArg ) { _arg = Guard.CheckArgumentNull( arg, "arg" ); _arg2 = Guard.CheckArgumentNullI( arg2, ""arg2" ); Guard.CheckArgumentRange( numArg, min, max ); }