The .NET Buffet

The Dangers of Hammers (or, Why SRP Isn’t Dangerous)

If you haven’t noticed, David Cooksey and I are having a bit of a back-and-forth about the Single Responsibility Principle and its use (or misuse) in software development. And I’m obviously not here to talk about the dangers of hammers.   But as David Cooksey has come up with another example of how SRP (the Single Responsibility Principle) is dangerous when completely misunderstood, I figured I had to reply with something.

I’m not going to spend any time discussing the code in his latest example, as I don’t think there’s anything new to discuss.  It’s just another “wrong-headed-application-of-SRP-causes-bad-code” post.

I guess we both agree that many people don’t necessarily understand SRP correctly (and, honestly, I’m sure I don’t always apply it perfectly every time I try either).  However,  it feels to me that David wants us to fear using SRP, and I want to try to help people understand how to apply it properly.

Rather than continue to rehash what SRP means to me, I’d like to defer to those from whom I’ve learned and point you at some good articles on SRP, The Open/Closed principle, and other SOLID topics in the hopes that you can see past the fear and start applying these powerful tools properly and without fear.

Jeremy Miller (of StructureMap fame, if you don’t already know) wrote a nice article in the June 2008 MSDN Magazine entitled The Open Closed Principle, in which he also talks about SRP.  In his words:

The point of the Single Responsibility Principle isn't just to write smaller classes and methods. The point is that each class should implement a cohesive set of related functions. An easy way to follow the Single Responsibility Principle is to constantly ask yourself whether every method and operation of a class is directly related to the name of that class. If you find some methods that do not fit with the name of the class, you should consider moving those methods to another class.

I highly recommend reading his article in its entirety (as well as about anything else I’ve ever read that he has written), as I’m pretty sure that article is the subconscious inspiration for my refactoring of David’s original post.

Another good reference is the originator of the grouping of principles we know as the SOLID principles, Robert Martin.  In his post “Getting a Solid Start” “Uncle Bob” says:

Following the rules on the paint can won’t teach you how to paint

This is an important point. Principles will not turn a bad programmer into a good programmer. Principles have to be applied with judgement. If they are applied by rote it is just as bad as if they are not applied at all.

Having said that, if you want to paint well, I suggest you learn the rules on the paint can. You may not agree with them all. You may not always apply the ones you do agree with. But you’d better know them. Knowledge of the principles and patterns gives you the justification to decide when and where to apply them. If you don’t know them, your decisions are much more arbitrary.

I could make the same arguments David makes about SRP for just about any software development technique.  Have you ever seen a 12-level-deep inheritance structure used to avoid a bunch of nested if statements (or, god-forbid, the 1200-line method filled with those nested if statements) when applying The Chain of Responsibility Pattern or some other form of composition would have been a much better, and more maintainable, solution.  So, is inheritance dangerous?  Yes, if used incorrectly.

In conclusion, hammers are not dangerous when used correctly.  However, most tools are dangerous when they are misused, and thousands of people hit themselves in the thumb with a hammer every year.  Hammers are dangerous - handle with care!

In response to "The Dangers of Single Responsibility in Programming"

David Cooksey writes an interesting article titled "The Dangers of Single Responsibility in Programming" in which he proposes that there is a certain level of granularity below which SRP is not appropriate.  Although I understand where he's coming from, I tend to disagree with his conclusion, and I think it's because his hypothetical programmer didn't actually find the right responsibilities, not because the method was already granular enough.  I'd propose a slightly different breakdown of the responsibilities, leveraging Inversion of Control and creating several Pricing Calculators that each handle a different kind of discount (In his scenario, there are Sales prices and Gold Level Discounts).  I would see each of these items as their own calculator, and a separate "Pricer" class that would use each of these.  Note that in a real application I would probably leverage something like StructureMap to find all classes that implement IPriceCalculator and have some way to control the orderring of these (probably a Priority field on the IPriceCalculator interface to sort by) but to keep things simple I'm hard-coding the calculators list in this example.  So, something like this:

Notice that now, each type of discount is contained in its own class which takes the customer, product, and current price and applies its business rules to the price.  In this way, the GetPrice method isn't going to turn into a Monster Method as new business rules are added. Also notice that it's easy to add a new pricing calculator.   So, for grins, let's add the "FixedDiscount" version alluded to in David's example.  First, create a new price calculator:

Now, we simply add this to the collection of price calculator used by our pricer (again, this would be handled by our DI container of choice in the real world):

And we're done.  Now, we've got a well-factored system which follows SRP and is easily extendable with new pricing rules (which, I would assume, could become significantly more complex than the examples given).

Well, we're not really done - if the theoretical developer had actually modified this original functionality in a Test-driven manner, you'd have a large number of unit tests to prove that you didn't break anything when you added the new FixedDiscountCalculator.  Note that I'm a fan of RhinoMocks, and leveraged it so I don't actually care about where my customer or product information comes from (there's actually no implementation of these interfaces at all - they should be loaded using your favorite repository-pattern-ORM-leveraging-code).  For the sake of keeping this post short, I've uploaded the complete source with unit tests as PricingCalculator.txt - rename to .cs, add NUnit and RhinoMocks references, and you're good to go.

In conclusion, Is SRP dangerous?  IMHO, not when properly applied.  But that's true for most SOLID principles - you need to know how to apply them appropriately.