Putting aside the inefficient database access for second, let me ask this: do the entities in this application actually encapsulate any meaningful behaviour whatsoever? And if not then what's the point of using an ORM to materialize stuff as an entities?

I'm seriously asking as I haven't looked at the application myself, but seeing even basic calculations performed outside of the entities raises a warning flag to me. It's possible that building all this fancy architecture in front of NHibernate makes people forget the purpose of using an ORM in the first place in addition to making the kind of mistakes you point out in your post much more likely.

@SteveR

A lot of people will use NHibernate simply to streamline data access and to abstract the complexities of a data model they may not have control over, as well as mapping the relational world to C#....and little more. The objects end up being DTOs and thats it, not entities in the OOP sense.

Then any kind of business logic, behavior, or process, is located in service classes.

@Francois - Why bother with NHibernate in this case? WebMatrix.Data by all means will be a better choice.

These posts would be of so much more value if Ayende would post the "correct" way of doing it. Yes, we see the wrong way, now what is the right way?

@Tim, my thoughts exactly.

You should include a screenshot of the NH Prof warnings? Or are you finding these issues by browsing the source?

The correct way is to buy UberProf and use it to make your data access queries better. No, seriously!

All kidding aside, this is database-101 and ORMs-101 (select n+1). That's what HQL FETCH is for, but I don't know how to do that with the new IQueryable queries being that I don't yet have the luxry of using them.

@Francois

Sure, but presumably some of those reasons don't apply in this case, for instance, having a wacky database schema that you can't control. Moreover, if NHibernate is just an implementation detail for a bunch of DAOs and the objects returned from them have their guts spilled out for a service class to go poking around in, what benefit are you deriving from, say, NHibernate's change tracking mechanisms? If the methods of your service class are just Transaction Script style procedures that go "I get this data, I change this data, I save this data and I'm done" then there's no need for anything to be tracking what's changed since the service method knows full-well what it's done.

I guess all I'm saying is that if people want to do things that way then they probably want something simpler than NHibernate.

@Tim & @Fred,

You fix it by using a combination of NHibernate Futures

ayende.com/.../nhibernate-futures.aspx
&

using fetch in certain queries to avoid n+1 situations.

Also, it is very difficult to do both of these correctly with a repository structure like the application above because the repository structure makes it difficult to access these useful features by trying to hide NHibernate too much.

The point of these reviews is not to specifically fix this project's code, but to point out that it is very easy to write very nasty code with the conventional repository pattern used in this project. I mean it takes 38 queries to process an order with 5 item where each item has 5 attributes.

As Ayende pointed out, these types of scenarios are far too easy to build. I would also appreciate a post showing a more efficient way of performing the same logic, both NH specific and with more general patterns.

I have some code that is very similar to the ProductAttributes loop in the demo code using the Entity Framework 4. Since EF4 allows assigning foreign keys directly, I have simply created a cache of the IDs to dramatically reduce the DB hits. I'd be interested in seeing other options that solve the scenario of "if in the DB, assign, and if not, create and assign" pattern.

@SteveR: IMO intelligent entities are much overrated. The domain model should contain stable domain stuff. Usually that turns out to be the basic form of the domain/data, not the behavior. So to me having dump or barely smart data in the data layer is usually a good thing.

@Christian

Unless there is no meaningful behaviour to model, then I'd dispute that what you describe qualifies as a domain model. If it doesn't embody the rules of the business domain then how can it be said to model it?

If all you want to do is bring back a bunch of dumb data containers and manipulate them procedurally then I don't see the point of using an ORM; something with much simpler capabilities would suffice. If you're not going to use OOP techniques to deal with the complexities and variability of the rules in your domain there's little point in manifesting that data in terms of classes.

Isn't it enough to call session.Load(userName) instead of loading all data about user for this case ?