+972 52-548-6969
ReviewMicrosoft N Layer App Sample, Part V–Cross Cutting is a fine line
Continuing my review of http://microsoftnlayerapp.codeplex.com/, an Official Guidance (shows up at: http://msdn.microsoft.com/es-es/architecture/en) which people are expected to read and follow.
From the evidence, it seems that there has been some (lacking, insufficient and usually causing damage, but some) attention given to cross cutting concerns:
That is why I was shocked to read method after method that read like this:
And this:
And this:
I mean, did at no point someone stand up and said: “My Ctrl+C / Ctrl+V keys just fell off, we might need to think about a better way of doing this"?
I mean, cross cutting concerns didn’t raise the question of error handling?
I mean, seriously, you want to tell me that this type of code is an Official Guidance from Microsoft?
Comments
Sometimes I wonder if such "guidance" apps are some kind of evil joke they push out to hopless cubic developers just to keep them busy until next paradigm shift in architecture. My alternatively theory is that these apps are produced by people who won't get to work in real teams (for obvious reasons). This kind of copy-paste guidance has been around since good old VB6 and ASP times, anybody remembers MTS/COM and ASP "best practices"?
Bunter, I especially like the fact that they know that they are using an Anti Pattern, but gave themselves a license to ignore that.
@Ayende: this is precisely why I can't understand those defending the sample by saying that you have to read the guide that comes with it before judging the code. The code violates the principles advocated in the guide all over the place, so if anything reading the guide should lower one's opinion of the code.
Should they be catching ArgumentNull Exceptions?
Isn't that handled by the .Net Framework?
Besides the copy/paste here are some other things I noticed:
Why (only) catch ANEs? Well written code should never have to catch ANEs (hint: you can check for null). In fact I believe you shouldn't ever have to catch ArgumentExceptions, as thrown AEs scream "bug". And is it really not possible that any other exception gets thrown?
Who in their right F-ing mind logs only the exception message? People who don't log stack traces obviously never had to hunt bugs.
aaaaaaaaaaaaahahahahahhahahahhaha
hahahha
ohhhhh... man.. my sides...
aaaaahahahhahahahhahahahahhahahahhahahhahaaaaarrrrrrrrrstopstopstop
it hurts
i cant laugh any more...
oh seriously.
I'm surprised this wasn't tagged with 'Humor'. If I was to review a colleague's code then this would jump right out.
Ayende, are there any open source projects that you believe are good examples of DDD being implemented? Or is the lack of this your main impetus behind Macto?
David, One of the problems with DDD is that you need a domain, and most OSS are focused on infrastructure stuff. Most sample projects are too small
This looks like wcf code in which case...
1, I'd hate to be on the end of exceptions that are not of type NullArgument.. As a client I want a consistent exception contract I shouldn't have to infer inner faults under any circumstance.
2, why are they not using the IInstanceProvider to inject the dependancies as part of the WCF IoC implementation. Holding static references to a container can cause all sorts of issues depending on your service instancing.
Without seeing all the code I might be wrong..
Ayende, okay, then do you know any OSS projects whose infrastructure stuff is a good example? Even if they lack a "real" domain...if there are projects with what you would consider a good infrastructure, I'd like to see them anyway.
Of course, there are LOTS of DDD examples on the web, but most I've seen look overkill/made by architecture astronauts to me (like the one you're just reviewing). I have a domain, but I'm just starting to try to understand DDD and I'm struggling to find a good, understandable example that I can use with my domain.
I'm really looking forward to Macto!
Christian, You might want to look at my github profile, specifically, Alexandria might be a good candidate.
My appologies but that is so fucking ridiculous. How could it possible be ah guidance!!
That's just a fucking joke. I wonder when people will stop doing stupid "practices" and actually start doing something useful.
One more time, my apologies for the language, but it is a kind scream from the soul :)
The real problem is, that a lot of people will adopt this "guidance" in their real world projects, because if MS says it is good and proven practice...it must be correct.
I am deeply disturbed by the fact that they are catching ArgumentException, thereby suppressing potential bugs. What a disgrace.
Shocking!!
Clearly this code was generated by some script. It is of course a most embarrassing violation of (among many) the DRY principle.
Also, can someone elaborate why "Log and throw" is an anti-pattern? In my opinion all exceptions that are not handled explicitly otherwise should be logged and thrown.
Man, this so closely resembles some code I've just inherited and "I mean, did at no point someone stand up and said: “My Ctrl+C / Ctrl+V keys just fell off, we might need to think about a better way of doing this"?" pretty much sums up my feelings exactly. Its pretty terrifying that people that write such code can now be like "hey its best practice per MSFT!".
@Christian a couple DDD examples, I don't think they fall under the N-Layered category though. That's why I've been following this review and will follow Macto. What is the best way to implement DDD and have different clients, like MVC, SL, and WPF, for example.
http://codecampserver.codeplex.com/ http://code.google.com/p/ndddsample/
@Ayende -
Fair enough. If you were doing a code review and you saw this, what would be your suggested resolution be? While this is funny, why not make it constructive too?
BobJ, Move all error handling out to an aspect? I mean, it isn't like WCF comes with a builtin way to handle errors, right: http://msdn.microsoft.com/en-us/library/system.servicemodel.dispatcher.ierrorhandler.aspx
re: DDD Examples.
Part of the problem with building a "DDD example". Is that all you can really see is the code (likely the end point of the code). Very often you will come into this code and go "hey this is pretty simple" but what you can't see is the vast amount of work that went into getting there, you can only see the endpoint, not the journey.
Most of the value in DDD is in the journey, not in the endpoint.
/me checks the date. Nope, not april 1st.
hehe! love the comments :)
@Manu => what happens if the 'logging' errors? zomg :( Also .. more info: http://tinyurl.com/6dua7lq
@Ayende
Yes, precisely. I know this, and use this same exception strategy with WCF. Perhaps others don't, or maybe they use another less effective method, or maybe they do what the guidance suggests (cut/paste). My point was, I know this post was meant to be humorous(and it was, the "guidance" is quite shocking), but why not add your own guidance in as well?
What happens when resolving ITraceManager throws an ArgumentNullException? Why aren't they handling THAT scenario?
BobJ, The problem is that then it puts means a lot of work on me to also points out the fixes. However, most devs can find out by googling the terms I used. The link I gave you? I had no idea about that, but I googled WCF error handling, and got that in under a minute
@Greg Young
"Most of the value in DDD is in the journey" - yes, for consultants, not for the end customer. And, by the way, what we see here has nothing to do with DDD, authors are fighting with technologies that should be orthogonal to domain logic.
Also, by looking these extracts only, all that code seems to justify that not needed layer that is only delegating to other layer that offers the SAME services. What would be the point of doing that?
@Manu This is why log and throw is no ideal http://skillsmatter.com/podcast/agile-scrum/fractal-tdd-using-tests-to-drive-system-design
@Ayende sometimes i am thinking that God blessed open source and .Net community with you.
Come on BobJ, the responsibility needs to be put on the developers. They work for Microsoft and are putting out a guidance application, they shouldn't need Ayende to tell them how to do Exception Handling.
Wow Ayende! I was reading some of your initial posts and thought you might be being a little harsh but looking at this I am truly shocked. This thing should be pulled down off the web immediately. If Microsoft want to put out guidance they need to use developers that have a wealth of real world experience not some guys that are clearly bloody novices. This is absolutely apalling and following this guidance will be a recipe for disaster
Yes it is poor code, maybe the whole concept of "Official Guidance" is wrong. Guidance come from distilling experience from many different places and getting something satisfying you and others in your team. Pretending to be Official is evil.
@kash
anything to add? :D
@iwayneo
About your first commentary, I have to say a lot of things, but I think that all of them are not very convenient to say here.
Apart of that, I think that all code is improvable, and this code in particular is very improvable. This is why they are developing v2.0...
I would like that you check out all the sample code, not only the code that Ayende is showing.
Because it's an Architecture official guidance, not a Coding guidance hehehehhe :D
definitely, this sample app and the guide it's an msft error. i'm sure they have the knowledge and the resources to do a better job.
Agree, ArgumentNull is an exception you don't handle - it means, you screwed up calling the API.
Then the Exception handler shakes head: - super lame using a static property (Current) to get a reference to the Inversion of Control container, and in the process coupling your class to the container - Unnecessary to abstract the container in your app - or, if you do want to abstract the actual container implementation, maybe perhaps steer towards the service locator pattern (which is pretty synonomous with IoC): http://commonservicelocator.codeplex.com/
Now, the Exception handling block: The EntLib includes a class 'ExceptionManager', which is intended to be injected into your class. It allows you to handle exceptions based on a policy. The latest API lets you do:
exceptionManager.Process(() => { DoWork(); }, "policy"); (http://msdn.microsoft.com/en-us/library/microsoft.practices.enterpriselibrary.exceptionhandling.exceptionmanager.process(v=PandP.50).aspx)
The policy configures how exceptions are handled. Typical actions you might configure are logging the exception, and the subsequent processing action, whether to handle, rethrow, or throw a new exception.
You also get the ExceptionShieldingAttribute which you can apply to service boundaries, so that you don't have to to try-catch within every service method, you know, for the sake of it...
Hillbillies, This is an Architecture guide, not an example app for coding best practices. When I look at this thread and think about how lost you all are it makes me sick to my stomach. B
Bad code, but probably payed it a lot.
Comment preview