Navs
Code Smells Catalog
Shotgun Surgery

Last Revision — April 19, 2022

2 Min Read


Shotgun Surgery

Similar to Divergent Change, but with a broader spectrum, the smell symptom of the Shotgun Surgery code is detected by the unnecessary requirement of changing multiple different classes to introduce a single modification. Things like that can happen with the failure to use the correct design pattern for the given system. This expansion of functionality can lead to an easy miss (and thus introduce a bug) if these small changes are all over the place and they are hard to find. Most likely, too many classes solve a simple problem.

Joshua Kerievsky noted this smell as Solution Sprawl [1]. Monteiro stated that the tiny difference between these two comes from how they are sensed. In the Divergent Change, one becomes aware of the smell while making the changes, and in the Solution Sprawl, one is aware by observing the issue. [2]

Causation

Wake says it could have happened due to an "overzealous attempt to eliminate Divergent Change" [3]. A missing class could understand the entire responsibility and handle the existing cluster of changes by itself. That scenario could also happen with cascading relationships of classes [4].

Problems

Single Responsibility Principle Violation

The codebase is non-cohesive.

Duplicated Code

The increased learning curve for new developers to effectively implement a change.

Examples

Smelly

class Minion:
    energy: int

    def attack(self):
        if self.energy < 20:
            animate('no-energy')
            skip_turn()
            return
        ...

    def cast_spell(self):
        if self.energy < 50:
            animate('no-energy')
            skip_turn()
            return
        ...

    def block(self):
        if self.energy < 10:
            animate('no-energy')
            skip_turn()
            return
        ...

    def move(self):
        if self.energy < 35:
            animate('no-energy')
            skip_turn()
            return
        ...

Solution

class Minion:
    energy: int

    def attack(self):
        if not self.has_energy(20):
            return
        ...

    def cast_spell(self):
        if not self.has_energy(50):
            return
        ...

    def block(self):
        if not self.has_energy(10):
            return
        ...

    def move(self):
        if not self.has_energy(35):
            return
        ...

    def has_energy(self, energy_required: int) -> bool:
        if self.energy < energy_required:
            self.handle_no_energy()
            return False
        return True

    def handle_no_energy(self) -> None:
        animate('no-energy')
        skip_turn()

Refactoring:

  • Extract Method
  • Combine Functions into Class
  • Combine Functions into Transform
  • Split Phase
  • Move Method and Move Field
  • Inline Function/Class
Sources
  • [1] - Joshua Kerievsky, "Refactoring to Patterns" (2005)
  • [2] - Monteiro, M.P., Fernandes, J.M., "Towards a catalog of aspect-oriented refactorings" (2005)
  • [3] - William C. Wake, "Refactoring Workbook" (2004)
  • [4] - MS Haque, J Carver, T Atkison, "Causes, impacts, and detection approaches of code smell: A survey" (2018)
  • [Origin] - Martin Fowler, "Refactoring: Improving the Design of Existing Code" (1999)