Clean Code

How to identify Shotgun Surgery using NDepend

In the previous articles in this series we’ve seen:

In this article we’ll see how to identify an afferent (incoming) coupling code smell: Shotgun Surgery.

Shotgun Surgery Detection Strategy

A method suffers from Shotgun Surgery if it is called many times from many other classes. Object-Oriented Metrics in Practice, by Michele Lanza and Radu Marinescu, proposes the following detection strategy for Shotgun Surgery:

(CM > Short Memory Cap) AND (CC > Many)

This detection strategy uses two metrics:

  • CM – Changing Methods – the number of methods that call the measured method
  • CC – Changing Classes – the number of classes in which the changing methods are defined

This detection strategy uses Generally-Accepted Meaning Thresholds. I used 7 as the value for Short Memory Cap and 10 for Many.

Metrics Definitions

Let’s go over the definitions for the used metrics and how to implement them with NDepend. For a more detailed definition, be sure to check Appendix A.2 of Object-Oriented Metrics in Practice. If you’re not familiar with CQLinq, check out the NDepend documentation or my blog post on how to query your code base.

CM – Changing Methods

This metric counts the number of distinct methods that call the measured method. We only care about our own code, for both caller methods and measured method. But, since framework code doesn’t call our code, we can rely on the NbMethodsCallingMe NDepend metric.

// <Name>CM</Name>
from m in JustMyCode.Methods
let cm = m.NbMethodsCallingMe
orderby cm descending
select new { m, cm }

CC – Changing Classes

This metric counts the the number of classes in which the changing methods are defined

// <Name>CC</Name>
from m in JustMyCode.Methods
let changingClasses = m.MethodsCallingMe.Select(method => method.ParentType).ToHashSet()
let cc = changingClasses.Count()
orderby cc descending
select new { m, cc, changingClasses }

Putting it all together

Now that we know how to compute each of the required metrics, let’s see how the detection strategy looks like:

// <Name>Shotgun Surgery</Name>
warnif count > 0
// ** Thresholds **
let ShortMemoryCap = 7
let Many = 10

// ** Detection Strategy **
from m in JustMyCode.Methods
let methodsCallingMe = m.MethodsCallingMe
let changingClasses = methodsCallingMe.Select(method => method.ParentType).ToHashSet()
let cm = m.NbMethodsCallingMe
let cc = changingClasses.Count()

 // Operation is called by too many other methods
 cm > ShortMemoryCap && 
 // Incoming calls are from many classes
 cc > Many
select new { m, cm, cc, changingClasses, methodsCallingMe }