NDepend Blog

Improve your .NET code quality with NDepend


Shotgun Surgery: What It Is and How to Stop It

October 2, 2018 7 minutes read

I really love the name “shotgun surgery” for describing a code smell.  It’s sort of an interesting mix of aggressive and comical, and so it paints a memorable picture.  I kind of picture Elmer Fudd blasting away at a codebase and then declaring “ship it” and doing some kind of one-click production push.

But misappropriating Looney Tunes aside, what actually is shotgun surgery?

Well, it’s a specific code smell in your codebase.  Shotgun surgery happens when you have to make many changes in your codebase to achieve seemingly simple tasks.  Often, you’ll find yourself making changes to code that seems pretty similar, either copy-pasted directly, or else of similar intent.

Why Is It Called Shotgun Surgery?

Before diving into specific examples, let’s talk about why it has the name that it does.  As I’ve pointed out, this is a memorable name, which is intentional.  That makes for a great mnemonic device for remembering it as you build software.

Imagine a normal surgical procedure.  When you think of a surgeon performing an operation, you think of precision and of minimal invasiveness.  They go in with scalpels and do no more than they must.

But replace that surgery with a shotgun, and picture the results.  Shotguns operate by spraying projectiles that disperse at a relatively short distance, effectively blowing a scattering of holes in anything nearby.  So a “shotgun surgery” is maximally invasivekind of the definition overkill when it comes to a surgery.

So the name, shotgun surgery, aims to convey a codebase that forces a clumsy, error-prone approach, rather than a precise one.

Examples of the Problem

Understanding the origins of the term should help you picture the problem.  But to really drive the point home, let’s consider some hypothetical examples.

1. Concerns Spread Everywhere

First, have you ever seen a codebase that really separated things to the extreme?  Perhaps it contains a class for all constants, and another one for static variables.  And, maybe it also has a place where you define all menu items and another for their subsequent actions.

The result?  Every time you want to add a menu item, you have to open up Constants.cs to define the menu item text, then another class where you store definitions for all of the delegates that handle the menu item clicks.  You have to make changes in a third place to actually add the menu item to the GUI, and a fourth place where you assign the delegate to some action.  That’s a lot of changes for a single conceptual change to your codebase.

2. Speculative Over Layering

Another common example arises from speculative over-architecting.  Have you ever seen a codebase to handle a simple CRUD app, but that defined multiple layers, complete with data transfer objects, data access objects, domain objects, and so on?  And so every time you want to add a table to the database, you now have to add scaffolding across all four layers in addition to the various property bag objects within those layers?  This is another shotgun surgery situation.

3. Copy-Paste Coding

And then, there is the most common and straightforward example: changing a codebase with copy-paste code everywhere.  This means that changes to some of the copy-paste code require you to make those same changes to each additional incarnation.

What Causes Codebases to Require Shotgun Surgery?

So now that you understand what shotgun surgery is and what some examples are, it’s worth asking what causes it.  How do codebases get this way?

Well, the case of copy-paste programming is an easy one to understand.  People copy and paste code when they’re in a hurry or when they don’t know any better.  This causes knowledge duplication all over the codebase, and creates a maintenance burden.

But let’s zoom out a little and generalize.  Copy and pasting is an egregious example of a concept called lack of cohesion in your codebase.  Cohesion describes both a specific metric that NDepend computes as well as a general concept.  For our purposes, let’s consider the general concept: do things that belong together occur together?

Highly cohesive codebases conform to the single responsibility principle, which suggests that units of code should have exactly one reason to change, and to change for only that reason.  But, beyond that, they localize changes as much as possible.

So shotgun surgery occurs when, for any reason, your codebase becomes non-cohesive.

Why Should You Care? What Problems Does It Cause You?

Is this really a source of worry, though?  Will it cause you problems?

Well, obviously the answer to that depends on the degree, but the short version is, yes, it will.  Here is a list of pains that you’ll have as maintainers of a codebase.

  • Changes become more time-consuming, since you have to edit your codebase in more places.
  • Merge conflicts become more likely, since more people are touching the code in more places.  This generally makes collaborative projects more dicey.
  • You’re more likely to introduce bugs because of the cognitive load of remembering to change the code in more places.  If a simple feature requires you to make changes in 7 files, there’s a decent chance you won’t remember all 7.  This can lead to more labor-intensive QA efforts and even to production defects.
  • You wind up with more code, due to the fact that you’ll have knowledge duplication and more constructs required to glue the various disparate pieces together.
  • The learning curve is higher for new team members, since development is something of a treasure hunt.

All of this adds up to lower code quality, higher maintenance costs, longer development times, and lower morale.

The First Step to Fixing the Problem Is Recognition

Convinced yet about the perils of shotgun surgery codebases?  Hopefully you are.  And if you are, you’re probably wondering how to move away from the problem.

The first step is just to take inventory of the change sets to your codebase.  Audit your source control tool, or else just mentally pay attention to how many files you’re touching while making conceptually simple changes.  If it’s a lot, then you’ve got an issue.

I can’t give you an exact number, of course, but I can tell you that a healthy codebase is one in which you’re implementing small features with only a few touched source files.  For larger changes, you may touch many, but it shouldn’t be the whole codebase or even a significant chunk of it.  Localizing changes should be a first class code maintenance goal.

And, if you’ve got copy-paste code, you definitely have a problem.  You can find this by looking for it, or by using a duplicate code detection tool.  For instance, NDepend has a duplicate code detection power tool.

Putting Down the Shotgun: How Do You Move Away From It?

So how do you remedy the situation after recognizing it?  The very first step is to refactor duplicate code to a single, canonical implementation.  Knowledge should only exist in a single place in your codebase.

From there things get more subtle, but still relatively straightforward.  When you find yourself changing the same four files or classes for each simple change, such as the addition of a menu item, reconsider your design.  Would merging some of those things create a scenario where you could touch the code in fewer places?  Are there classic design patterns out there that handle what you’re doing more elegantly?

You’re never going to come up with the perfect design.  Separating all of those things out might have seemed perfectly logical when you did it, and it may even have been the right play at the time.  But if you find yourself in a shotgun surgery situation, the world around you has changed and evolved, and you need to roll with it.  Take the friction around making changes as feedback on your design, and optimize accordingly.

And, of course, use any tools at your disposal that you can, whether it’s the aforementioned duplicate detector, or metrics around cohesion, as NDepend offers.  If you start seeing things like lack of cohesion go through the roof, you can use the tooling to identify and mitigate those trouble spots.

But perhaps the most powerful thing you can do is simply be aware of this smell as you work.  Constantly ask yourself whether maintenance programmers will need a shotgun or a scalpel to make changes likely to be necessary down the road.  And if the answer is “shotgun,” then reconsider your approach.