The Shotgun Surgery Code Smell

Code Smells are similar in concept to development-level anti-patterns. Sometimes in our code, we introduce a code smell unintentionally those makes our design fragile.

Definition of Code Smell

Code smell, also known as a bad smell in computer programming code, refers to any symptom in the source code of a program that possibly indicates a deeper problem.

Martin Fowler Says...

"A code smell is a surface indication that usually corresponds to a deeper problem in the system"

Code smell creates a lot of problems while introducing new feature or maintains the codebase.

Often a developer has to write repeatable code, breaking encapsulation, breaking abstraction, etc. if code smells are not corrected, so always refactor your code smells while developing.

In this article, we discuss one of the popular code smells: “shotgun surgery". Shotgun surgery says, to introduce a small new change, a developer has to change many classes and methods, and most of the time has to write duplicated code, which violates the “Don’t Repeat Yourself” principle.

Causes of Shotgun Surgery

1. Poor separation of concerns.

2. Failure to understand responsibilies, often due to misunderstanding (single responsibility principle).

3. Not identifying the common behavior or behaviors with a slight change.

4. Failure to introduce proper design patterns.

Consequences of Shotgun Surgery

1. Lots of duplicate code

2. Taking more time to develop small features

3. Unmaintainable code base

Refactoring Strategy

We can do it by using the “Move Method”, “Move Field”, or “Inline class.”

We will discuss the above strategies in another article.

Let's see an example where the “Shotgun Surgery” smell is present:

package com.example.codesmell;

public class Account {

       private String type;
       private String accountNumber;
       private int amount;

       public Account(String type,String accountNumber,int amount)
       {
              this.amount=amount;
              this.type=type;
              this.accountNumber=accountNumber;
       }


       public void debit(int debit) throws Exception
       {
              if(amount <= 500)
              {
                     throw new Exception("Mininum balance shuold be over 500");
              }

              amount = amount-debit;
              System.out.println("Now amount is" + amount);

       }

       public void transfer(Account from,Account to,int cerditAmount) throws Exception
       {
              if(from.amount <= 500)
              {
                     throw new Exception("Mininum balance shuold be over 500");
              }

              to.amount = amount+cerditAmount;

       }

       public void sendWarningMessage()
       {
              if(amount <= 500)
              {
                     System.out.println("amount should be over 500");
              }
       }



}

package com.example.codesmell;

public class ShotgunSurgery {

       public static void main(String[] args) throws Exception {
              Account acc = new Account("Personal","AC1234",1000);
              acc.debit(500);
              acc.sendWarningMessage();
              //acc.debit(500);
       }

}

If we pay attention to Account.java file, we can see every operation — debit(), transfer(), and sendWarningMessage() — has one validation: Account balance should be more than 500. We copied the same validation in every method because we are not able to identify the common validation, so we introduce a “Shotgun Surgery” code smell.

The real problem occurs when we add another criterion in the validation logic: if the account type is personal and the balance is over 500, then we can perform the above operations. In this scenario, we have to make changes to all methods, which is not what we want to do, so let’s see how we can solve it.

Create a common method call isAccountUnderflow() that will solve the problem, all validation related stuff will go there.

Refactored Code

package com.example.codesmell;

public class AcountRefactored {

       private String type;
       private String accountNumber;
       private int amount;



       public AcountRefactored(String type,String accountNumber,int amount)
       {
              this.amount=amount;
              this.type=type;
              this.accountNumber=accountNumber;
       }

       private boolean isAccountUnderflow()
       {
             return amount<=500;

       }


       public void debit(int debit) throws Exception
       {
              if(isAccountUnderflow())
              {
                     throw new Exception("Mininum balance shuold be over 500");
              }

              amount = amount-debit;
              System.out.println("Now amount is" + amount);

       }

       public void transfer(AcountRefactored from,AcountRefactored to,int cerditAmount) throws Exception
       {
              if(isAccountUnderflow())
              {
                     throw new Exception("Mininum balance shuold be over 500");
              }

              to.amount = amount+cerditAmount;

       }

       public void sendWarningMessage()
       {
              if(isAccountUnderflow())
              {
                     System.out.println("amount should be over 500");
              }
       }

}

Diagram