Made me sad to see this. It seems the people that wrote this were not aware of some of the features of ORM-s and NHibernate in particular. I wish more people do research about their tools before they code.

While I agree the code is over the top and unnecessary, binding to the data/domain model in the controller is not the way to go.

I'd much rather write a view model specific to each controller action than a custom binder. Validation is a lot simpler (FluentValidation or DataAnnotations) as validating a form isn't always the same as validating the NHibernate model.

In most cases AutoMapper can be used to update the NHibernate model from the view model instead of writing line after line of code like: a.Name = b.Name

@Owen I generally agree but often come across these issues:

1. The view model usually contains more properties than I actually bind to in the post action (additional lookups etc.). For this reason I often have a "Post Model" with those fields I actually want to update, and a "View Model" which inherits from the Post Model and adds any view/lookup data fields. Obviously usually we want to avoid inheritance but so far this has worked out pretty well for me.

The alternative to using inheritance would be to have two separate classes with duplicate properties, one for view and one for form/post, but I would need to make sure I copied any validation attributes between both to make sure that client-side and server-side validation still worked. Using inheritance means the validation is always shared across both cases.

1. I don't like AutoMapper. It means that if I rename a property on my entity, I must remember to rename it on my view (post) model to match - or all of a sudden my forms will stop updating that field without warning. (Unless I have a full suite of acceptance tests to cover that, which I guess is the ideal!).

@morcs,

AutoMapper has an assert method to prevent such problems without having to unit test each property.

@Dmitry Thanks :)

Found it here: http://automapper.codeplex.com/wikipage?title=Configuration%20Validation

I really don't understand why people keep creating these things on top of ORMs context. It's a Repository and Unit Of Work wrapped in one package. You don't need wrappers for CRUD, the basic context does those things just fine. You need wrappers if there's some real work going on.

Oh, those Haskell-styled return values. Anyone else thinks they are useless without at least pattern matching?

I've seen the "Tasks" pattern before, it's basically what the rest of us would know as "Services", the layer around the Repository that everything calls into (and which subsequently just wraps the calls to the Repository).

Thing to remember is this was the "assumed" way of doing data access in .NET before we heard of CQRS and before the geniuses like Ayende started not using Repositories. Most of us learned this style of code from various presentations and screencasts and books which did just this: Service wrapping a Repository wrapping an ORM. I know for a fact I learned this style from watching the MVC Storefront series.

@Wayne Services should do something meaningful, like sending an email, printing a receipt or saving an audit trail. If you use service to wrap a single CRUD operation you're wasting your and more importantly your colleagues time. I think it has something to do with good IDEs which allow programmer to navigate the code real quick. If you're using vim in 80*25 you just can't get your head around all this stuff.

I have always used a single generic repository before LINQ-to-ORM/IQueryable was available in .NET. Now you can just use the Query<> method most ORMs provide.

I never understood the reason of making a repository-per-type or even per-aggregate root with custom methods when you have services.

If Sharp Lite is a simplified version of the full Sharp architecture, then I'm really glad I never took a look at it.This really is way to much for my stomach.

Github just learned the hard way ( http://arstechnica.com/business/news/2012/03/hacker-commandeers-github-to-prove-vuln-in-ruby.ars ) that model binding directly to persisted entities can be really dangerous, and I prefer to have middle man DTOs. Binding directly to the entities solves the Microsoft demo-ability issue, but it's just not worth it.

Renaming the service layer "Tasks" is a really good idea. It's what they are, and the name doesn't imply anything about "backgrounding" etc.

However, reinventing CRUD for the gazillionth time is just...yuck.

I can't really believe you would actually want to call Session.Load inside a custom model-binder. In terms of readability of code, this really sucks as it hides the very important database-request from the one reviewing the code. If I look at an action, I want to understand what is going on there, without digging through the solution explorer.

Seeing from the comments and the mindless followers, I start to think these post series are pointless. If you pick one piece of code and take it out of context, in most cases you'll find it just unecessary and would like to make it simpler and change it completely. However when you start asking the right questions and at least try to understand what were the problems the original developer was trying to solve, it would probably make more sense. Unfortunately the author is not here to defend himself.

@Apostol: Check out who's behind the project before making a fool out of yourself. @Pavel: Making things always simpler is a good thing but there are reasons devs don't want to depend on third party frameworks, hence the intermediary layers. Simple design is good for simple tools, some of us have to deal with enterprise application designs and last time changes.

Thanks to @gunteman who understands the problem. It is indeed really bad practice to bind data to your mapped entity for the reasons he has mentioned.

@Ayende, come on, that last comment about not capturing the exception was just unfair. If you want to capture the exception, then fine update the source code and pass it to the ActionConfirmation or just log it. Now whether this puts the whole design into question is another matter.

Sorry for the tone, but today I was not in the mood to deal with unfair and generalization comments "Oh I don't like that line there, I will never use this project ever!". For the rest of us who wants to understand the architecture, check here http://bit.ly/w0fQxv

@gunteman Actually "Task" is already used in the .NET framework to signify an asynchronous process. http://msdn.microsoft.com/en-us/library/System.Threading.Tasks.Task.aspx

@Jean I've found it's easier to make last minute changes when you have less code. Just because you work for an "enterprise" doesn't mean you need to use enterprise architecture. Simpler is better. Pick and ORM or doc db, learn it well, then use Commands, Queries, and Tasks like Ayende has said before. You've just covered 95% of LOB apps without blowing out the complexity. If you need durability, you can get a long way with a persistent messaging framework like NSB.

@guntenman That's an overly broad characterization of what happened with github (IMO). They did have the Rails equivalent of a DTO (a dictionary of property names and values), but they forgot/didn't bother to do the equivalent of saying "oh, but the DTO should never have an 'IsAdmin' property and if someone tries to pass one you should ignore it." And then they did the equivalent of user = AutoMapper.Map(dto); user.Save();

To the broader topic-- in ASP.NET MVC, an explicit "ViewModel"/"DTO" layer does have the effect of forcing you to think about which properties you really want to modify after the POST action in each unique context. It strikes me as a necessity.

OTOH, sure, AutoMapper is not hard to use, but it is one more piece of infrastructure that you have to test and maintain, just. to. do. CRUD. That's more than a "shucks, can't do the gee-whiz 5-minute demo" problem. That's a "why do I have to write so much code that's not about the domain of my application?" problem.

I like these posts, they make me think.

I haven't figured out where this pattern came from, but in several companies I've encountered code where the data access layer was called an Object Factory. In fact I was talking to a coworker one day and trying to describe the repository pattern and he said "Isn't that an object factory?"

I just found it surprising as I've never seen any MSDN posts or examples like that which use that approach, so I'm not sure where people are picking it up. Maybe it came out of the Java world?

@Ryan By enterprise architecture I meant proper separation of concerns and responsibilities. Like most of the commenters think here is that a Service Layer or any kind of abstraction of the ORM is pointless. I completely disagree. Having a Service Layer is crucial to control/centralize access (auditing/authorization/validation) of your entities across multiple front ends/tiers. When I see a controller accessing directly a repository I feel the developer just doesn't care about future development.

Sure, some time you will end up with a service method with just one line calling directly your repository, but this is for consistency ! If you decide every call to your repository should pass first by the service you're not going to break the design and inject your repo in the controller just because it saves you one line. Discipline always pays off.

The other day I had to implement a new authorization mechanism in a large system. So basically, all GetById, GetBySomething had now to also inject the User info. Let me tell you that if there was no Service layer I would have been completely screwed and it would have taken me days to make changes across ALL presentation layers. Obviously if there was one single controller which had a repository injected directly it would have made the implementation much harder but thank god I work with conscientious and disciplined developers.

Never mind that there are no actual entities in this project but whatever. These 'entities' have no behavior. It's just a read model. There's 0 reason to make this so complex.

@Jean If I only need a web front end at first, why am I coding for additional front ends now? How will I know that my other endpoints will need exactly the same information.

As far as integrating user authentication/authorization, if you have proper separation of concerns, you would have something like Context.CurrentUser which can be set at the start of a request by the appropriate authentication module. Then no matter how you do authentication, everything else just sees Context.UserPrincipal. If you have authorization built into many queries and you are using LINQ then you could just make a FilterForCurrentUser() extension method that keeps this logic in one spot.

Depending on the complexity of your query, NHibernate supports the notion of a global filter. In EF you can easily provide a wrapper around the context to do filtering that does not hide the actual context like the repository pattern does. Same goes for mongo and raven.

@Ryarn Comparing the small cost of adding a Service layer from the beginning and refactoring a HUGE base code, I can see what wins in terms of reusability and speed of development. If your client asks you to implement a new functionality (outside web scope) and it needs to be integrated in your current system, having the ability to plug in your service layer without any worries of inconsistency (security hole,etc..) is priceless in the eyes of your client.

@Luke

One extra level of indirection, but still exactly the same thing. Posted values were blindly bound to the persisted entities.

But I think we agree on the actual issue :)