Ayende @ Rahien

Refunds available at head office

Review: Microsoft 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:

image

That is why I was shocked to read method after method that read like this:

image

And this:

image

And this:

image

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?

Tags:

Posted By: Ayende Rahien

Published at

Originally posted at

Comments

Bunter
07/06/2011 09:36 AM by
Bunter

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"?

Ayende Rahien
07/06/2011 09:40 AM by
Ayende Rahien

Bunter, I especially like the fact that they know that they are using an Anti Pattern, but gave themselves a license to ignore that.

SteveR
07/06/2011 10:06 AM by
SteveR

@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.

Dirk
07/06/2011 10:06 AM by
Dirk

Should they be catching ArgumentNull Exceptions?

Isn't that handled by the .Net Framework?

Patrick Huizinga
07/06/2011 10:12 AM by
Patrick Huizinga

Besides the copy/paste here are some other things I noticed:

catch (ArgumentNullException ex)

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?

traceManager.TraceError(ex.Message)

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.

iwayneo
07/06/2011 10:16 AM by
iwayneo

aaaaaaaaaaaaahahahahahhahahahhaha

hahahha

ohhhhh... man.. my sides...

aaaaahahahhahahahhahahahahhahahahhahahhahaaaaarrrrrrrrrstopstopstop

it hurts

i cant laugh any more...

oh seriously.

David Neale
07/06/2011 10:27 AM by
David Neale

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?

Ayende Rahien
07/06/2011 10:34 AM by
Ayende Rahien

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

Gary McAllister
07/06/2011 11:12 AM by
Gary McAllister

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..

Christian Specht
07/06/2011 11:17 AM by
Christian Specht

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!

Ayende Rahien
07/06/2011 11:21 AM by
Ayende Rahien

Christian, You might want to look at my github profile, specifically, Alexandria might be a good candidate.

Dmytrii Nagirniak
07/06/2011 11:31 AM by
Dmytrii Nagirniak

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 :)

Belvasis
07/06/2011 12:34 PM by
Belvasis

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.

tobi
07/06/2011 01:31 PM by
tobi

I am deeply disturbed by the fact that they are catching ArgumentException, thereby suppressing potential bugs. What a disgrace.

Mani
07/06/2011 01:31 PM by
Mani

Shocking!!

Manu
07/06/2011 01:38 PM by
Manu

Clearly this code was generated by some script. It is of course a most embarrassing violation of (among many) the DRY principle.

Manu
07/06/2011 01:41 PM by
Manu

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.

David Fauber
07/06/2011 01:43 PM by
David Fauber

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!".

Derek
07/06/2011 02:08 PM by
Derek

@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/

BobJ
07/06/2011 02:09 PM by
BobJ

@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?

Ayende Rahien
07/06/2011 02:58 PM by
Ayende Rahien

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

Greg Young
07/06/2011 03:12 PM by
Greg Young

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.

Justin A
07/06/2011 03:14 PM by
Justin A

/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

BobJ
07/06/2011 03:17 PM by
BobJ

@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?

Darren Kopp
07/06/2011 03:35 PM by
Darren Kopp

What happens when resolving ITraceManager throws an ArgumentNullException? Why aren't they handling THAT scenario?

Ayende Rahien
07/06/2011 04:48 PM by
Ayende Rahien

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

Alex Simkin
07/06/2011 05:59 PM by
Alex Simkin

@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.

Leonardo
07/06/2011 06:59 PM by
Leonardo

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?

Cosmin
07/06/2011 09:08 PM by
Cosmin

@Manu This is why log and throw is no ideal http://skillsmatter.com/podcast/agile-scrum/fractal-tdd-using-tests-to-drive-system-design

Olcay
07/06/2011 11:18 PM by
Olcay

@Ayende sometimes i am thinking that God blessed open source and .Net community with you.

Steve
07/07/2011 03:35 AM by
Steve

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.

degorolls
07/07/2011 03:47 AM by
degorolls

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

Felice Pollano
07/07/2011 07:49 AM by
Felice Pollano

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.

iwayneo
07/07/2011 03:36 PM by
iwayneo

@kash

anything to add? :D

Kash
07/07/2011 05:34 PM by
Kash

@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.

Tien Do
07/08/2011 03:29 AM by
Tien Do

Because it's an Architecture official guidance, not a Coding guidance hehehehhe :D

juanjo
07/09/2011 03:31 PM by
juanjo

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.

KierenH
07/19/2011 04:02 PM by
KierenH

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...

BNewer
08/15/2011 06:20 PM by
BNewer

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

Brenda
08/22/2011 06:06 PM by
Brenda

Bad code, but probably payed it a lot.

Comments have been closed on this topic.