Ayende @ Rahien

It's a girl

Review: Microsoft N Layer App Sample, part VI–Single responsibility principle is for idiots and morons

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.

Take a look at the following:

image

It is obvious that someone recognized that there are distinct responsibilities in here.

It is just sad that C# doesn’t give us better way to split responsibilities other than regions and partial classes.

I mean, if we could have defined multiple interfaces and multiple classes… but that is crazy talk, no one needs to have multiple classes in their applications, what are we, Java?

Tags:

Posted By: Ayende Rahien

Published at

Originally posted at

Comments

Scooletz
07/08/2011 09:18 AM by
Scooletz

ironyon_ Let's make one big/class/interface file with everything! SRP, ISP? It must have been invented by those Java guys... Forget about it!

Jonas H
07/08/2011 09:25 AM by
Jonas H

I must admit that at the beginning of this series, I was quite sceptical of your crtiticism, as I didn't really buy the "This application is not complex enough for DDD" argument. But never mind that discussion - I see now that you are right ... this pile of crap code should NEVER have been let into the hands of innocent programmers by MS :-(

Dave
07/08/2011 09:33 AM by
Dave

After your first post about this showcase project with the diagram I also took a peek, but the whole notion of SOLID is a mystery to these programmers.

I love this quote from the project page "Domain Driven Design is much more than Architecture and Design Patterns". I Agree, but DDD is also much more than cramming everything in one file..

What scarring me is that this is an official Microsoft guideline, so thousands of unaware programmers think this should be the way they should write their applications..

tobi
07/08/2011 09:34 AM by
tobi

Some people make every method into a region because they do not know that you can collapse methods as well.

flukus
07/08/2011 09:38 AM by
flukus

@tobi - Also fun is when methods contain regions.

Could we possible make it a rule that regions are an anti-pattern to be avoided?

Paul
07/08/2011 09:52 AM by
Paul

Regions in methods... My personal pet hate. Do you think perhaps that method is too big / doing too much?

NC
07/08/2011 10:13 AM by
NC

@tobi - How can you not know that you can collapse methods when it has the same symbol to expand/collapse a method as a region?

Koen
07/08/2011 11:01 AM by
Koen

Why have you not given up on this series yet? Or are you going on until they shut this project down?

Pablo Alarcón
07/08/2011 11:18 AM by
Pablo Alarcón

I think there is a missing thing here, Ayende.

That's not a "regular" interface, it's the definition of a WCF Service , isn't it decored with [ServiceContract] ?.

In the case of the interface of a WCFService there is additional meaning to defining a single big interface with 1 single big class implementation, as broken it in 3 would mean creating 3 small webservices as far as I know.

Probably the interface could be split in 3 and IMainModuleService could derive from those 3 single responsability interfaces... but still a single class implementation would be required by WCF if I'm not wrong.

Delegation to 3 classes could be done too, but what I try to explain is that this is not a "regular" interface and it isn't being noticed by most people.

Kash
07/08/2011 11:27 AM by
Kash

The code that you are reading is an implementation of a facade pattern on the distributed services layer... There are not any logic in the hidden code, but ayende don't mention nothing about it. What is wrong here?

The following links shows what I'm saying:

http://microsoftnlayerapp.codeplex.com/SourceControl/changeset/view/54267#831902

http://microsoftnlayerapp.codeplex.com/SourceControl/changeset/view/54267#831905

http://microsoftnlayerapp.codeplex.com/SourceControl/changeset/view/54267#943291

...

iwayneo
07/08/2011 11:36 AM by
iwayneo

I like to have all of my logic, presentation code, persistence - everything in one bog file. it's called god.cs and it's amazadonk. makes finding stuff super easy. it's my own take on the KISS principle...

i may create a framework around it. it'll be a blank file called god.cs which will be created by calling into the native win32 api so will have millions of lines of code to do it - it's going to be amazadonk.

Mister Derp
07/08/2011 11:39 AM by
Mister Derp

Actually SRP is for idiots and morons when taken to an extreme. As kash points out, WCF services are prime candidates for Facade pattern but a sensationalist asshole would never consider that. No, because everyone just fucking loves working with a dozen different service interfaces. And fuck off about regions already. Asshole.

David Fauber
07/08/2011 11:50 AM by
David Fauber

"no one needs to have multiple classes in their applications, what are we, Java?"

These guys did a good job here given the tools at hand but I could've gotten that down to four characters in lisp.

Craig Wilson
07/08/2011 12:11 PM by
Craig Wilson

@MisterDerp, @Kash

WCF doesn't give any excuse to violate ISP. The purpose of ISP is to protect consumers from changes in unrelated code. In this example, if customer management changes, then whoever cares about banking will have to change as well even though they don't consume anything related to customer management. So, no, I don't think WCF services are a prime candidate for this...

Patrick Huizinga
07/08/2011 12:12 PM by
Patrick Huizinga

@David Fauber

You mean excluding the 512 parentheses, right? :-P

David
07/08/2011 12:17 PM by
David

Taken a look to the real code, that interface is really a WCF Interface/Contract. That WCF is acting as a single Façade so you have a single WCF Endpoint to consume. That is quite common and easier to consume instead of having a lot of 'Web References'. Son it has to aggregate many methods from different internal Services. Then, internally they seem to follow the SRP. So, I don't think that this code (WCF Façade) is wrong at all.

Pablo Alarcón
07/08/2011 12:21 PM by
Pablo Alarcón

@Craig

There is no excuse, but there are many more concerns when the interface defines a WebService.

BobJ
07/08/2011 12:29 PM by
BobJ

@Craig Wilson

So you have never dealt with WCF in a real world/large scale project then?

Mix
07/08/2011 12:45 PM by
Mix

Because of this understanding of Façade a few people mentioned our code base sometimes hurt to look at. This is not a Façade - as per Façade pattern, the pattern does contain some logic. As per Wikipedia:

A facade is an object that provides a simplified interface to a larger body of code

How the above is a Façade??? It does not give me a simplified interface, it gives me more complex interface by aggregating simpler ones. It does hide the fact that there are 3 or more services to which calls are passed on, but you have to be joking to call this a proper pattern.

Anyway, I was just pissed we're calling this Façade, that's all. I propose we call it WCF Conglomerate Interface.

Craig Wilson
07/08/2011 12:52 PM by
Craig Wilson

@Pable, @BobJ

I'm not sure to what you guys are referring to. I have done massive implementations of WCF in real-world scenarios and truly don't know what problems you are experiencing that would excuse this type of service contract.

In what scenario would a consumer of these services need more than 1 of them at a time? I can see getting a customer and then getting sales information. However, if this is the case, then a simple Facade over these 3 internal services is not the answer. Rather, the service should aggregate together the information necessary to service the consumer in a single call. For example, the ISalesService should return back customer information in addition to sales information. This is a foundational aspect of SOA -> compositing data for consumers so they don't need to make umpteen calls to information.

PB
07/08/2011 12:59 PM by
PB

@Mister Derp, you misunderstand the facade pattern...

Paul Cowan
07/08/2011 01:16 PM by
Paul Cowan

Mixins in C# would be a big productivity boon. It would also the negate to be so reliant on an IOC container.

David
07/08/2011 01:21 PM by
David

@Craig, @Mix Sometimes you can compose in order to minimize roundtrips, but many other times you cannot because those are the calls you really need to make, or maybe the internal services already made that composition (Which I think is this case if you take a look to the internal services, the real façade seems to be made by the Application Services). Then, it is a matter of how many WCF Endpoints you wanna have for your WCF consumers. Having a lot of Web-Service References is kind of a nightmare for consumer apps. I personally prefer having the minimum number of WCF endpoints depending on the volume of your Modules.

Jesse Williamson
07/08/2011 01:31 PM by
Jesse Williamson

People are getting HEATED on here. GEEZ! I agree with Craig on this, but if MisterDerp and Kash (and others) want to use WCF as a reason to violate SRP or ISP, fine. Make that decision and move on. I personally, would much rather have to consume multiple services than have to change because something unrelated to me is modified, but that is me. I think MS should make those arguments (no I haven't read "the book" so maybe it's in there) and justify the path they took and acknowledge alternatives. I don't see that here.

I think Ayende's, and other's, concern is that when passing something along as Official Guidance, you have to understand that people will take these things and run with them. Official guidance should explain decisions and viable alternatives. I don't think it's realistic to expect that any large scale project doesn't violate some aspect of SOLID somewhere but those violations should be deliberate and explained.

Oh, and @MisterDerp...chill, the fuck, out. I mean, really, guy. It's just a sample app and it's just one opinion. You're going to burst a blood vessel if you keep that stuff up. :)

Brian
07/08/2011 01:45 PM by
Brian

@David You say that having a lot of endpoints is a nightmare. It really doesn't matter what you prefer - in a real world application it may be required to split to services (following SOA principles), so that separate services like Banking, Sales and Customer management can be separately scaled out and have different SOA requirements. So, take a look at some "real code" out there - I don't think this facade is good at all. It is a pattern being misused. Not all systems run on one machine!

Ayende Rahien
07/08/2011 01:57 PM by
Ayende Rahien

Koen, Look at the publication dates, I wrote all of those posts at one sitting, some weeks ago. From then on, it is pretty much on auto pilot.

Ayende Rahien
07/08/2011 02:01 PM by
Ayende Rahien

Pablo, Because having additional endpoints cost you money? Because naturally they share all the same requirements? Your Sales requires the same security as your Bank ? They sit in the same DB? Accessible to the same people?

Give me a break, that isn't how applications are structured. But since this isn't a real application, they wouldn't know that.

Ayende Rahien
07/08/2011 02:02 PM by
Ayende Rahien

Kash, Again, not real world example. Not actually a facade, it makes things more complicated, not simpler.

And those classes contains reams of logic, just look how big those methods are.

Ayende Rahien
07/08/2011 02:04 PM by
Ayende Rahien

Mister Derp, Yes, I would much rather work with a focused interface than one that has dozens or hundreds of methods. Service interfaces don't cost money, you shouldn't be afraid to have more than one of them. Please note that if you continue to curse on this blog I'll remove your comments.

Alex Simkin
07/08/2011 02:05 PM by
Alex Simkin

Does Interface Segregation Principle break Single Responsibility Principle or vice versa? That is the question!

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

David, This is the first time I even hear about this new fangled thing WCF Facade. This code isn't a facade, it doesn't simplify anything and doesn't really help you to do much. In a real N Layer system, those services would be located on different machines, anyway, so this is a double irony.

geelius
07/08/2011 11:08 AM by
geelius

I hate regions. Personally I don't think you should need them if you are coding properly

Kamran
07/08/2011 03:30 PM by
Kamran

Ayende is right. Where I work all these systems are in SAP, Oracle, and SQL Server. Having one service to rule them all would be a nightmare. They all have different permissions. Much easier to consume multiple services... as he says, it doesn't cost more money to add endpoints to your app.

Even using an "application service layer" type thinking within a single app, you'd probably split your responsibilities into separate "services" like ICustomerService, ITransactionService, etc. to deal with your domain.

Corey
07/08/2011 03:40 PM by
Corey

If WCF is the excuse, why not create an endpoint that meets both needs and doesn't expose several methods over the wire... Use one endpoint one method that passes messages. Don't like the abstraction? Maybe WCF wasn't the best choice?

object Request(object request);

JV
07/08/2011 04:55 PM by
JV

I can see both points being made here.

If these WCF endpoints are being exposed to customers/partners/third parties, the approach they are using has been done to make it is easier for the consumer to have access to everything with only a single reference.

Perfect examples of this: http://msdn.microsoft.com/en-us/library/aa257520(v=sql.80).aspx http://www.magentocommerce.com/wiki/doc/webservices-api/api

On the other hand, Ayende is pointing out that, without a proper domain and understanding of what you are building (and who for), this "offical guidance" can easily be misleading. Other approaches could be more efficient and suitable, depending on what you are setting out to accomplish.

There are many factors that will determine how to structure your SOA (logically and physically) in a way that best suits your domain, infrastructure, development team, and a boss who doesn't understand any of it, anyway.

Ayende Rahien
07/08/2011 04:58 PM by
Ayende Rahien

JV, I don't know this magento thingie, but I don't really like the API it exposes. And NO ONE should hold up Reporting Services as the way to do something.

JV
07/08/2011 05:06 PM by
JV

Ayende: I definitely agree with you. They are crappy.

Just pointing out and taking note of the "single reference" thing others are talking about.

Those examples are more ammunition to backup your point here. ;)

John
07/08/2011 07:31 PM by
John

As some astute commenter cleverly pointed out, this is an example of your everyday, run-of-the-mill "God class" anti-pattern. This isn't a facade. It has nothing to do with WCF technologies.

Microsoft has a history of doing some things very well(Rx anyone?), but much, much more in a mediocre fashion (Zunes came out in brown,

Vista was too bloated that Netbooks won't run on it, Bing and MSN are money losers, they are 3 years behind Apple in producing a true tablet OS, despite working on Project Origami way back in 2006, and on and on, . . . ).

On a side note, WSJ did a story recently about how MS has many more patents than Google. One wonders if the coders at MS spent less time on patent paperwork whether they would write better programs.

Sergio Navarro
07/09/2011 11:12 AM by
Sergio Navarro

I'm seeing too much demagogy on this series of post. Very subjective opinions about very questionable mistakes but with an unmeasured force to prosecute the project.In conclusion much ado about almost nothing

Bartek
07/09/2011 07:44 PM by
Bartek

"what are we, Java?"

I really like this sentence. I am pretty sure that when you ask 10 random Java developers and then 10 random .NET developers questions about such terms as ORM, SoC, SRP, TDD most of Java guys will know the answer while .NET guys will struggle with the answer. That's because MS is promoting so called "ease of development" instead of best practices. And that's why we see such "guidances" from MS as quoted in this blog post.

Bartek
07/09/2011 07:47 PM by
Bartek

Sergio Navarro, Catching NullReferenceException is what you called questionable mistake?

Tien Do
07/11/2011 11:00 AM by
Tien Do

It makes me remember about a project that I joined two months ago, some developers often wrap their code in regions named "Public methods", "Private methods", "Static methods" etc. but then they sometime forgot to put methods in the right region, or they even put private methods into "Public methods" region heheheh. But they're not MS architects :D.

Wayne Molina
07/25/2011 12:47 PM by
Wayne Molina

Sadly this is common, it seems. My company does this; we have "XXXEngine.cs" classes that have either multiple classes in the same file, or worse every method static. It's like a VB6 module. And at least one has a GOTO STATEMENT (in C# 3.5!).

The fact this is touted as "approved guidance" makes me throw up a little, but I'm used to it. In the six years I've worked in software development, I've always been the ONLY person on the entire team, if not the whole company, that listens to the advice of knowledgeable people like Ayende and his peers.

It's fairly distressing. Things like this makes me want to use Java; I love C# but Microsoft, and by extension most .NET developers I've met, just have it all wrong

Comments have been closed on this topic.