loading...

Code Smell: Shotgun Surgery

gothinkster profile image Thinkster Originally published at Medium ・2 min read

If nothing else, the name of this code smell is one of the more entertaining names. The shotgun surgery code smell is one of the code smells that often overlaps with other code smells, particularly duplicate code. But it can still pop up and knowing about it and looking for it in your code will help you to keep your code more easily maintained. This is ESPECIALLY true of code bases that suffer from classes and functions that are too large.

Remember the part of your system everyone is afraid to touch? You can almost guarantee it has issues with shotgun surgery. In fact, the shotgun surgery code smell is at the root of that issue of “you change something here and it breaks something over there” which has possibly caused more keyboard-related violence than just about anything else.

To put it simply, shotgun surgery is when you have to go to multiple places in your codebase and make the same change. Let’s look at a simplified example:

image

This savings class doesn’t look too bad at first glance, but the issue here is the proliferation of very similar code. Specifically, that minimum balance check.

this.balance < MIN_BALANCE

We’re doing that in three places. If something about that ever needs to change, then we’ll have to make changes in three places. Notice that all the code is not identical in each case, but the core logic is identical.

So let’s look at one possible refactoring (you may come up with even better ones).

image

Here we have extracted the core logic of the check, and moved it to its own method. Personally, this is one of my favorite refactorings, to extract the boolean condition in an if into its own method, because then I can give that ugly boolean logic a nice name. Since we have gathered the duplicate logic together, we now have just one place to make changes if something about the minimum balance calculation needs to change.

We could certainly go farther and look for even more similar logic extraction. But we’ll leave our example like this.

Of course this is simple when the similar logic is gathered all together like this and obviously duplicated. In a real code base, this may be scattered around a much larger class, or it may even be in multiple classes, and even in different parts of the code base. But the lesson still stands. When we are dealing with checking the minimum balance, that should be done in ONE place, not three, or five, or ten. That way changes are easy to implement.

Signup for my newsletter at here.
Visit Us: thinkster.io | Facebook: @gothinkster | Twitter: @gothinkster

Discussion

pic
Editor guide