Ayende @ Rahien

KierenH commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Tue, 19 Jul 2011 15:25:26 GMT

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 ); }

James McKay commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Wed, 13 Jul 2011 23:53:15 GMT

@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.)

Kamran commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Wed, 13 Jul 2011 19:47:07 GMT

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!

Matthew Shapiro commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Wed, 13 Jul 2011 15:13:03 GMT

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

Frans Bouma commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Wed, 13 Jul 2011 11:00:51 GMT

"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 ? 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.

Jimmy Zimms commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Tue, 12 Jul 2011 16:18:35 GMT

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.

Mike McG commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Tue, 12 Jul 2011 14:13:30 GMT

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.)

David commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Tue, 12 Jul 2011 10:00:30 GMT

@ 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.

Ayende Rahien commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Tue, 12 Jul 2011 05:15:27 GMT

Mike, That seems very nice in theory, but it doesn't work in practice. If you don't respect your persistence medium, and tailor yourself to its need, you are going to suffer perf issues.

Murcia commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Tue, 12 Jul 2011 03:44:13 GMT

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

tobi commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Tue, 12 Jul 2011 00:38:15 GMT

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.

David Barone commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Tue, 12 Jul 2011 00:01:18 GMT

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

flukus commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 22:51:25 GMT

@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.

Mike McG commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 22:01:04 GMT

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 commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 21:14:27 GMT

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

Remco commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 21:11:13 GMT

Nobody noticed the upcasting of UnitOfWork?

Magnus commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 19:50:37 GMT

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.

Matthew Shapiro commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 19:10:46 GMT

@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)

Alex Simkin commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 18:56:18 GMT

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

Kristof Claes commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 17:51:16 GMT

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?

Matthew Shapiro commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 17:17:16 GMT

@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.

Cosmin Onea commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 16:41:09 GMT

@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.

BobJ commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 16:18:38 GMT

@James LOL. That about sums it up.

James McKay commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 15:56:59 GMT

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.

Matthew Shapiro commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 15:23:47 GMT

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.

Cosmin Onea commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 15:08:51 GMT

@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 Include(this IQueryable sequence, string path) where T : class { var query = sequence as DbQuery; 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.

Lee commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 14:32:18 GMT

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

tawani commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 14:30:45 GMT

"PageIndex and PageCount are validated but later ignored" This is clearly insanity ... how could have believed? Jokes aside: Time to short MSFT

Rafal commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 14:24:55 GMT

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?

Josh commented on Review: Microsoft N Layer App Sample, part VII–Data Access Layer is GOOD for you

Mon, 11 Jul 2011 14:09:23 GMT

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