ReviewMicrosoft 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:
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?
Comments
irony_on Let's make one big/class/interface file with everything! SRP, ISP? It must have been invented by those Java guys... Forget about it!
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 :-(
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..
Some people make every method into a region because they do not know that you can collapse methods as well.
@tobi - Also fun is when methods contain regions.
Could we possible make it a rule that regions are an anti-pattern to be avoided?
Regions in methods... My personal pet hate. Do you think perhaps that method is too big / doing too much?
@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?
Why have you not given up on this series yet? Or are you going on until they shut this project down?
I hate regions. Personally I don't think you should need them if you are coding properly
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.
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
...
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.
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.
"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.
@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...
@David Fauber
You mean excluding the 512 parentheses, right? :-P
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.
@Craig
There is no excuse, but there are many more concerns when the interface defines a WebService.
@Craig Wilson
So you have never dealt with WCF in a real world/large scale project then?
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.
@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.
@Mister Derp, you misunderstand the facade pattern...
Mixins in C# would be a big productivity boon. It would also the negate to be so reliant on an IOC container.
@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.
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. :)
@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!
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.
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.
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.
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.
Does Interface Segregation Principle break Single Responsibility Principle or vice versa? That is the question!
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.
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.
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);
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.
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.
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. ;)
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.
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
"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.
Sergio Navarro, Catching NullReferenceException is what you called questionable mistake?
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.
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
Comment preview