Monday, March 26, 2018

Ways to Spot Single Responsibility Violations

Classes should only have one reason to change. We should attempt to increase cohesion between things that change for the same reason, and decrease the coupling between things that change for different reasons. Classes should do one thing and one thing well.

Violations of the Single Responsibility Principle can cause code to be difficult to test and maintain, and can make it easier for bugs to manifest.

There are a few symptoms of SRP violations which can give warning signs of these issues.

Large classes/methods

Obviously large classes and methods do not always point to SRP violations as there may be a requirement for a significant amount of logic. However, it is usually a good sign.

Large methods are more likely to point to an SRP violation than large classes, or at least too much cyclomatic complexity. You'll become aware of this when you attempt to cover the method with test cases, and realise there are excessive test combinations required.

Too many injected dependencies

While this may depend a lot on how you set up your services, lots of injected dependencies may suggest that the class is doing too much, and may be violating SRP.

Too many tests per class

If a class has too many tests, or the tests need to change often, this is an indication that there may be too much complexity, too much setup, and this may point to an SRP violation.

Described with and/or, or encompassing words such as "manager"

This is a sure sign that your class or method is doing more than one thing. For example: a method called CreateOrEditRecord() or a class called "InvoiceHandler" are red flags, and should be investigated.

Logic in Views

Logic in your front end almost always means that you are violating SRP, because a view's responsibility is to render an output. As soon as you have two pieces of logic in a view, it's unlikely they'll be for the same reason, because they'll be for different elements on the page. This immediately makes it impossible to unit test the view.

Also, because of the standalone nature of views, there's a good chance you'll have to violate DRY in another view.

Move that logic up to a model, a controller, a helper, or a service where it can be properly unit tested.