The Danger of Single Responsibility in Programming Continued

October 16th 2009 | David Cooksey

The Dangers of Single Responsibility in Programming Continued

The Dangers of Single Responsibility, Cont.

Doug Rohrer responded to my initial post on this topic with a good refactoring of the classes involved in a manner similar to the Strategy pattern. I agree with many of his points—the hypothetical developer certainly chose the wrong responsibilities; misunderstood the Single Responsibility Principle; and generally made the code a mess. That said, I believe that SRP is most definitely dangerous, not because of what happens when it is used correctly, but because of how easy it is to get it wrong. Misapplying the SRP can result in code that makes God objects look easy to maintain.

For clarity’s sake, I’ll go one step further—it is easy to misunderstand the sentence “A class should have only one reason to change” as a literal commandment to be applied at the line or method level. This results in disaster. One common example of how the SRP is misunderstood can be seen in this thread, where the poster asks if the SRP means that each class can have only one method. Luckily the poster received a good informative answer, but that is not the case for all developers learning about the SRP.

Here is an example of code modifications I have seen motivated by a desire to apply the SRP.

BEFORE

public class Check
    {
        private readonly IDataProvider _dataProvider;

        public Check(IDataProvider dataProvider)
        {
            _dataProvider = dataProvider;
        }

        public bool Run()
        {
            IBusinessObject data = _dataProvider.Get();

            if (data.Condition1 && data.Condition2)
            {
                string message = string.Format("Check failed, {0} {1}", data.Property1, data.Property2);
                throw new Exception(message);
            }
            return data.Property3 != data.Property4;
        }
    }

AFTER

public class Class
    {
        private readonly IDataProvider _dataProvider;
        private readonly ICheckErrorMessageProvider _checkErrorMessageProvider;

        public Check(IDataProvider dataProvider, ICheckErrorMessageProvider checkErrorMessageProvider)
        {
            _dataProvider = dataProvider;
            _checkErrorMessageProvider = checkErrorMessageProvider;
        }

        public bool Run()
        {
            IBusinessObject data = _dataProvider.Get();

            if (data.Condition1 && data.Condition2)
            {
                string message = _checkErrorMessageProvider.GetErrorMessage(data);
                throw new Exception(message);
            }
            return data.Property3 != data.Property4;
        }
    }
public class CheckErrorMessageProvider : ICheckErrorMessageProvider
    {
        public string GetErrorMessage(IBusinessObject data)
        {
            return string.Format("Check failed, {0} {1}", data.Property1, data.Property2);
        }
    }

Here, the developer asked the SRP question “Does this class have only one reason to change?” and got the answer “No, it could change because the formatted text could change, or because the logic could change”, and refactored the String.format out into its own provider. While harmless on the surface, this artificial separation of concerns does not add any value. The new class is so specific that it cannot be used anywhere else. In addition, the developer is likely to forget the CheckErrorMessageProvider name almost immediately, so if a text change is required he will most likely go to the Check class first, and then go the extra level down into the string provider in order to make the text change. In other words, the complexity of the code was increased for no benefit.

I believe that after correctness, simplicity is the most important programming principle. Simpler code is easier to understand when first read; easier to remember; easier to test; easier to refactor; and easier to add features to. Anything that adds complexity makes all of these tasks harder, especially on larger projects with many non-trivial sub-systems. Applying single responsibility at the line or method level diffuses business logic into a cloud of tiny classes that do next-to-nothing individually, and thoroughly obscure the logic they represent.

In conclusion, yes, the SRP is not dangerous when applied correctly. But then, most things are dangerous because of what happens when they are misused, and the Single Responsibility Principle is no exception. Handle with care!

David Cooksey is a Senior .NET Consultant at Thycotic Software, an agile software services and product development company based in Washington DC. Secret Server is our flagship password management software product.

Comments

• Other Developer Blogs

  • The Morning Brew
  • Alvin Ashcraft’s Morning Dew

• Recent Posts

  • The Benefits of Pair Programming
  • Managed code isnt always the best solution
  • Debugger Friend or Foe
  • Dealing with Nullable Types and IIf in VBNET
  • Cool Things to Do with Dynamics in dotNET 4
  • Are Extension Methods a Code Smell
  • An Overview of C# 4.0
  • Some Love for VB.NET 10 Too
  • The Danger of Single Responsibility in Programming Continued
  • Want to join a team of top notch developers in the DC Metro area?
  • The AJAX UpdatePanel Is it your worst enemy?
  • Thycotic brings Password Management to SuperComputing 2009
  • Top 10 Takeaways from DevConnections Day Four
  • Top 10 Takeaways from DevConnections Day Three
  • Top 10 Takeaways from DevConnections Day Two