Maybe it's me but I fail to see the point of this 'review'. All I see is you bashing someone's code which was written for the most time a long time ago and which isn't really production code. Sure it's used to illustrate a point which isn't really that great, but that's actually subjective.

I don't like the over-the-top over-engineered architecture, but one can describe that in 1 post. You go on-and-on about details which gets me the feeling you want to bash the author personally: the 'best' thing one can learn from these posts is how not to build your software, but I think the author himself already knows that.

The posts furthermore suggests you think you have the authority to judge other people's code. While everyone's entitled to their opinion about anything, and therefore you too, isn't it a little 'holier than thou' when you dig so deep into someone else's code just to get more visitors on your blog? After all, one only has to take a little peek into the NHibernate code base to know your code isn't always up to par either.

Just stay a gentleman, Oren. There are no 2 developers on this planet who like eachother's code: there's always something wrong, they always would have done things differently.

This is actually the style of code I am looking at right now.

In my case, all classes including "Services", "Managers", "Repositories" and DTOs are ... interfaces. With one implementation (or a second which has only NotImplemented stub method).

Do you think you could show how you would do this using Raven? I'm all for simplicity and think that we need to start seeing concrete examples of how to implement using RavenDB which would perhaps give the doubters pause for thought?

What I would like to see from this review is how it SHOULD be done.

For example, you dismiss the repository pattern, but what are the alternatives? For example, in an ASP.NET web application you have controllers. I do NOT want to see this code in my controllers:

var sessionFactory = CreateSessionFactory();

using (var session = sessionFactory.OpenSession()) { using (var transaction = session.BeginTransaction()) { // do a large amount of work

// save entities
session.SaveOrUpdate(myEntity);

transaction.Commit();

} }

That is ugly, repetetive code. I want in my service methods to Get, update, save, and not have to worry about the above.

Further, if my aggregate root query should exclude entities that have, for example, and IsActive = false flag, I also don't want to repeatedly exclude the IsActive = false entities. Using the repository pattern I can expose my Get method where internally it ALWAYS does this.

Although his solution is messy, I get the feeling this is what the author was trying to do. How would you recommend do this?

Let's see some constructive advice please.

@pete this isn't about RavenDB, it's about misusing Entity framework. Using RavenDB the same way would be just as bad.

@Matt (I'm presuming we're talking about ASP.NET MVC3)

why not use IOC to inject your Session directly into the Controller class? why not have a filter that automatically opens a transaction every time the controller has a Session injected into it?

Also, EF/Nhibernate sessiosn by themselves implement the repository pattern, just use them directly. You can always add extension methods (You can look at the Racoon Blog repo, I believe they also have a few queries that they've added as extension methods).

and, IMO, you can always use a thin layer of abstraction, that doesn't abstract the fact that you're using EF/Nhibernate/Raven/Whatever, but helps you fine tune the API to your needs.

Frans, you said it beautifully. His point is reasonable, but his approach is awful and filled with hubris and after so many of these reviews it's borderline beating a dead horse. I am this close to unsubscribing if it wasn't for all the other useful stuff he actually does post once in awhile.

Oren, what do you use for your sequence diagrams?

@linkgoron

Yes, IoC does help with ISession creation, but I still like a repository pattern to separate commands from queries.

This is hard to explain succinctly, but say you have a windows service and a web project (yes, MVC3). Both use shared services that handle your domain commands. As the creator of the API, you do not know what COMBINATIONS of commands are required by the various platforms, you just provide the domain layer.

If, for example, there are two service methods that can be used separately in some instances, or together in others. You don't want a transaction opened in each case, but you HAVE to, because they can be called independently and NHib doesn't supported nested transactions.

I would like to see good advice on how these layers SHOULD be implemented, taking into account the following:

• how to implement a service layer, where the exposed methods can be used independently or together (when is the right time to create transactions, sessions, load entities, etc)
• where the BLL can be used from different tiers e.g. you want to put ISession objects in a "using" statement for a desktop app, but you don't want to do that in a web app - you want it cleaned up at the end of an Http web request using StructureMap or whatever.

I'm waffling now, but all I'm saying is I'd like to see more suggestions on best practice - what has been done in the Starter Kit project and how he SHOULD have done it, instead of just nailing the poor guy.

http://community.devexpress.com/blogs/seth/archive/2011/03/09/interview-with-ayende-rahien-aka-oren-eini.aspx

Interesting take on the repo pattern.

@Frans you are correct, but the NSK project is declared to be: "intended to be used as a blueprint when designing and implementing a .NET layered application architecture.", it is not just a someone else messy code...

"In my next post, I’ll show a better way to handle this scenario. A way that is actually simpler and make use an of actual read model and not infinite levels of indirection."

Thanks.

@Felice Declared by who? The author? So what! There are a lot of repositories on e.g. github where the author/developer thinks it's gods gift to humanity while it's a stinking pile of goo. Bashing that doesn't make the world better. It only makes the basher look like a person who thinks he never wrote bad code nor claimed the same thing.

Sure it's extremely over-engineered and therefore isn't the best choice as an example, but you can tell that 1 sentence (I just did) and move on.

@Frans Like anything in life, pick the good and leave the bad behind. Putting his style aside, Ayende is one of the most .NET prolific bloggers and I thank him for his posts.

Note1: I wrote a comment very similar to yours (http://ayende.com/blog/152706/application-review-northwind-starter-kit).

Note2: BTW, I am interested in see a comparison between your profiler an Ayende's. (I don't mean one by you)

This series is awful, and smacks of cyber-bullying. i'm gonna buy a copy of Raven so I know I'm safe and he won't attack my code.

Frans Bouma, Bryan... I fully respect your point of views, but this blog isn't about pedagogical instructions and avoid calling bad code for bad code.

I think it's a blog for serious people who are willing to discuss and learn from each other, if some one wants a more educational soft approach they should attend a seminar instead.

Unfortunately the world is overflowed with people calling them self for developers, where in fact they only produce bad and smelly code. These people are producing bad code and gives software projects a bad reputation. Why do you think nearly all public software projects takes twice as long as supposed and drain the public cent by cent?

+1 for Abdu

Regardless of context, this is a great post isn't it? Showing us precisely how we can abstract data access so far down the layers we don't realise what calls are being made to the database by our high-level code.

When we suddenly do realise, we've got to twist all those layers to gain performance...or in many case die trying.

I've written a lot of code with these abstractions, and it has been Ayende's posts that have taught me to really consider adding all those layers.

Btw, Ayende has been on this developer's life and he stressed these weren't personal attacks.

I shall remain impartial and continue to learn from "the master".

horrible architecture. i'm sorry for the poor coders that need to maintain this layers of shit

A lot of us have to work with architecture this bad. Showing bad examples is just as important as showing good examples, it can help you realize when your going/gone down the wrong path.

@Frans and Bryan

I wholeheartedly agree, but to fling some cold truth in the other direction, asking Ayende to be a gentleman is pointless. Such skills are not his forte. Zing.

Most of the bashing and bullying that happens on the net (and elsewhere) comes from people who can't assert themselves in any other way than by belittling others. Ayende is most certainly not in that position. Most of us envy his skills and insight. Still, for some reason, this continues to happen.

Also, I actually think this is hurting Hibernating Rhinos' bottom line. I know I'm not the only one who's very reluctant to send any money the way of such behavior. My loss, I guess, but still...

@Frans Yes, declared by the authors, of course.But such a declaration requires, if someone disagree, a well explained motivation, and this is what Ayende is trying to do.

@fellce that's not the point. That some self-proclaimed super-dev is wrong on the internet isn't an incentive to write a series of blogposts to prove that person wrong, as there are a LOT of people wrong on the internet, every day.

The main issue I have with the series of articles is that it's apparently a good thing to simply bash a person's code, as that apparently teaches people something. But that's silly, it only shows someone made bad choices and ended up with a pile of shit. Gee.

If Oren wants to teach people how to do things right, he should write a series of blogposts about how to do it right. As that's 1) more efficient, 2) less bullying and 3) it actually teaches something.

Because there are a million ways to do things the wrong way. Telling your audience this person's code is one of them isn't teaching anything. Well, only if you are first convinced that the way the code bashed to pieces is how you'd write it yourself.

Take a look at the upcoming posts. It seems a few blog entries will be devoted to "how-to" instead of "how-not-to".

@Frans, in this serie he writes how not to do it and then later he will give examples of a better way to do it. It's a serie.

Also you're not helping calling names like 'self proclaimed super dev'.

You can argue his style but not critizing (bashing in your words) other people's code because you have a populair blog is no argument at all.

We (I) love your many constructive posts, but hate that your approach to show bad practices is almost always to take a public piss at someone else. I no longer think you're doing it to teach, but simply that you get a kick out of the pissing act. It's not uncommon behavior. Most of us do it, in some way or other, but it's seriously reducing the value of your blog, your legacy and, I think, also your business.

Learning from mistakes is one of the most effective ways of learning. So I for one am grateful for Ayende taking on this task of pointing out these mistakes.

@Frans you wrong beacuse the autors of NSK are professional coders that would show me (beginner) the rigth (ideal) code!!! and offer me a book! ayende is rude but real and he yes help me!!

Personally, I think Fabio had it right with his idea of Enhanced Query Objects (http://fabiomaulo.blogspot.com/2010/07/enhanced-query-object.html) - if you need ORM-agnostic support for data access, this approach has little downside.

I wish Ayende would actually take time to create the Macto project instead of making very similar negative posts about various code bases.

Morning Ayende, could you tell me what tool did you use to draw the UML? Was it generated by this tool or been written by you?

Thanks

@Frans Bouma

Yeah buddy. I've been saying for quite some time now half the post on this big mouths website are simply there to troll for traffic.

Dudes what he is - an assh*le.

@Dmitry

Dude, MATCO was just this windbag blowing hard. He said forever he was going to write an end-to-end sample app so everyone would finally see the right way to do it - errrrrr - I mean the Oren way. Whatever.

Dude serves up about a dozen screenshots a year ago and the project goes dark. Typical big mouth. He's too busy reading other peoples code and bashing to finish writing his own.

Ayende, good review. There's one thing I am missing from this post... Did you actually look run this code through a profiler?

Obviously I don't have all the code in front of me but I am sure you are fully aware that entity framework won't load the same PK twice if it's already loaded in the context unless you force it too? Since the unit of work seems to come from the WorkerService, the entity you think is loaded twice should not be... Doesn't make it good anyway, but I think one of your assumption is wrong in this post...

My next question would be, how do you unit test your controllers through a unit test framework without a connection to a database if you do not abstract the DA layer out?

Right-click method, hit Generate Sequence Diagram. OMG, I never knew that was there. :-))

@Frans I agreee with you Frans. Why don't you blog about the right way to this? Gosh it cannot be that long since we read "Stored procedures are bad, m'kay?"