+972 52-548-6969
The Challenger of Architecture Astronauts - Two-Tier Service Application Scenario
It continues to amaze me, the length some people will go to in order to add additional complexity. Let us take this article: Two-Tier Service Application Scenario (REST). I will leave aside the arguments about this article gross misrepresentations of what terms like domain modeling, entities and REST. Greg already called the DDD argument, and Colin is working on the problems with the representation of REST.
What I want to talk about is friction. I am looking at the code that is shown in the article and I cringed. Hard. Do you want to tell me that it is recommended that people would write code like this?
I am assuming that I would need to write one of those for each of my “entities” I honestly cannot figure out why. Why create specialized behavior when it would be easier, simpler, cheaper and better to handle this generically?
Is there any sort of value in this code? I don’t think so.
It gets better when you see what you need to do in your Application_Start.
So now, not only do I have to create those handlers by hand, I now need to take care to register each them. I’ll leave aside the bug in the routing code vs. the employee route handler code (‘employee’ is not a route value), to focus on a more important subject. Even if I decided that I want to code my way to insanity, why do I give a single class two responsibilities (creating EmployeeHandler and EmployeesHandler).
I am not even trying to ask why I need the route handler in the first place. It seems like it is there just so there would be another layer in the architecture diagram. It looks like that was a common requirement, because here comes the next step toward the road of useless code:
Except for creating additional work for the developer, I cannot think of a single reason why I would want to write this type of code unless I was paid by the line count.
Let me see how many things I can find here that are going to add friction:
- As written, you are going to need one of those for each of the “entities” that you have. I assume that this is so that all the other per “entity” types wouldn’t feel lonely. On last count, We had:
- EmployeeRepository
- EmployeeHandler
- EmployeesHandler
- EmployeeRouteHandler
- EmployeeTranslator
- EmployeeScript
- EmployeeFacade
- Mapping “command” whatever that is, to method calls? So every time that I have to add a new method, I have to touch how many places?
- Why on earth do I need to do explicit error handling? That is why I have exceptions for. Those should do the Right Thing and I should not have to explicitly manage errors unless I know about something specific happening.
Oh, and I saved the best for last. Please take a look at the beast. And unlike the story, there is no beauty.
I truly find it hard to find a place to start.
- Magic numbers all over the place.
- Promote(object[] data) ?! Are we in the stone age again? I really hoped that by 2009 we would be able to get to grip with the notion of meaningful method parameters! For crying out load, you can use the ASP.Net MVC binder to do the work, you don’t have to do it yourself.
- Null reference exception that are just waiting to happen.
- Unless PositionEnum is not an enum (a WTF all on its own), then the code wouldn’t even compile! Enums are value types, you cannot use ‘as’ with them.
- busErrors ?
- First of all, what bus?
- More importantly, are we back in the good ole days of return codes? I thought we were beyond that already!
- Really bad resource management. C# has try/catch/finally for a reason. If an exception is thrown, you are going to leak the transactions. This is truly sad since the text is very careful to point out that you MUST dispose of those resources before you return.
As I said, I am not going to even approach the actual guidance that is offered there. I think that it is invalid as best and likely to be harmful.
From the code sample shown, I would surmise that no one actually sat down and actually coded any sort of system with this. Even the most basic system would crumble under the sheer weight of architecture piled on top of the poor system.
I am saddened and disappointed to see such a thing being presented as guidance from the P&P.
Comments
"From the code sample shown, I would surmise that no one actually sat down and actually coded any sort of system with this. Even the most basic system would crumble under the sheer weight of architecture piled on top of the poor system."
After I've looked at the samples. It looked like a "proof-of-concept" for me. Any way, the samples are just coding without any thought over architectural design. A good architecture should take the complexity imho.
Damn, but this gang did get me into TDD.
But it's a fine place to start if you are worse then this.
I feel that this is like staircase, you have to take one step after another before you reach nirvana.
Benny
OK, it's ugly - agreed, we all know it.
Now, the next step would be to contrast this, with a better, more maintainable solution, so that people can see, compare and learn.
Serousely, this post has only the 10% of the value, you can provide by showing how to do better than that.
In other words, telling a blind man that he's going in the wrong direction does not help him much, until you tell him where the right direction is.
I've not looked at the page, but I am guessing that "busErrors" is short for "Business Errors" and nothing to do with a "Bus".
I generally agree with what you say about having to do all that work for each business class in your model. Anything that dictates I need to write something for every class instantly puts me off, because my models are too large for that sort of thing.
With that in mind though, imagine having to write not only something for each class, but also something for each member of each class too :-)
http://code.google.com/p/fluent-nhibernate/
I have to agree with Krzystof on this.
Lead the way!
Peter,
Did you see my initial reaction for Fluent NHibernate?
+1 for Krzystof's comments
I agree with ALMOST everything you said except the bit about return codes. When updating multiple objects or properties at once, if validation errors happen, you need some way of giving that information back to the user. Whether you do it with explicit "return codes" like "UpdateResult result = GoDoSomethingWith(thisStuff);" where UpdateResult contains a list of validation errors, or whether it's done automagically behind the scenes by a ModelBinder filling up a ModelState dictionary, the end result is the same - you get a "result code" of sorts that tells the user what they did wrong.
As for the rest of it - yeah, that stuff's way out in space!
LOL. Sorry Krzysztof, but I am sort of with Ayende here. This is just garbage and should be made fun of. While Ayende is a damn smart man it should not be his "job" to show a massive corporation like M$ how to do theirs.
While I did enjoy being an snob and laughing at the code and am also very disappointed that this stuff has been published. Its not like its on some hack blog site, this is endorsed by M$.
If you are not sure what is wrong with this code, my feeling is that if someone shows you the fix, you will cut and past that and never know why it was crap in the first place.
Books: read them ;-)
Framework Design Guidelines
Head first Design Implementation -or- Implementation patterns
Those 2 books alone should help you identify the problems with the duplication, string literals, magic numbers, ambiguous arguments, exception strategy (moot maybe). Both are a breeze to read.
Once you have digested those hit Evans DDD and then Ayende's one?
Regarding Lee Campbell's comment:
"Once you have digested those hit Evans DDD and then Ayende's one?"
Ayende, after Building DSL w/ Boo is published, please start writing a book about DDD. It would be nice to put together all the valuable content written to the blog for so many years. A book on DDD in C# using all the technology stack: Castle, NHibernate, Linq, Rhino projects would certainly attract many. Focus also on practical side: convention over configuration, fluent interfaces, unit of work, repository, integration, crosscutting concerns, automatic components registration and surely you will have a winner.
Showing how all these apparently disparate concepts can work together and the details what makes them tick surely is the most valuable aspect.
For sure I will be buying this one.
@Krzysztof Kozmic
There are numerous examples of how to design these sorts of systems, I summarized what I'd do in comments on their article but ultimately its just use routing to go to an "action" (so routing to the UserResourceHandler.Create method perhaps) and then use normal object-oriented concepts. Since you've bothered to create repositories and map domain classes use them, for a start by moving behavior into those domain classes.
+1 for Lee Campbel
Hi All,
Just wanted to drop a comment here that we are actively looking at this article and will make modifications to it as needed.
You all know that we work on the Agile process here, right? We get something out (perhaps a little early) and then improve it. Codeplex is for open source and continous improvement with community feedback. The entire apparchKB on this site is on that premise. On MSDN is official and considered "complete".
That said, this artcile obiously needs to be looked at. If something's not right, we work on it and improve it.
I see that there are a flurry of comments on the bottom of the article, so we'll start there with the improvement process and get it to something that makes sense to the community at large.
If you want to mark up the source with revisions, I can make that available to you in Word as well.
Thanks for providing feedback.
Rob Boucher Jr
the fact that you are using "agile development" as an explanation for this kind of design only shows your lack of understanding for the agile development process.
This code is not a planned milestone in a larger project, it requires a re-write.
Crack smoking from the P&P people - there's a surprise.
I think they are saying that the documentation is "agile". Obviously not the case as it reached a point release before they feedback was considered. Try approaching the community about how we do things.
It all looks like regurgitated late 90's COM rubbish rewritten in C#.
It makes me like the Java culture more every day.
@Chris Smith, @Pete W
I don't like your tone. Really, people, give them a break. Ayende pointed out flaws in the document, in a rather strong but not insulting way. What you, or Pete W are doing, is insulting, and I'm not with you on this one.
Instead of throwing meat at P&P we should provide constructive feedback so that they can improve with next iteration. THIS is the gist of agile process.
@Kryzysztof
Seb makes an effort to address some of the downfalls here serialseb.blogspot.com/.../...wo-tier-service.html
Good job Seb, but what a waste of everyone's time! Why not just pay Seb and Oren to write the article correctly in the first place ;-)
This reminds me a lot of how the CSLA framework over-complicates the simplest of things.
Comment preview