25 Common Code Smells and Refactoring Techniques

Introduction

What makes code good? Good code should have consistent naming, strong readability, extensibility, robustness, etc. This article highlights 25 typical code smells that developers need to watch out for.

1. Duplicated Code

Duplicated code appears when the same program structure exists in different places, often caused by copy‑paste during fast iteration. It is hard to maintain because a change must be applied in multiple locations.

Three typical situations are discussed:

Two methods in the same class contain the same statements.

class A {
    public void method1() {
        doSomething1
        doSomething2
        doSomething3
    }
    public void method2() {
        doSomething1
        doSomething2
        doSomething4
    }
}

Solution: use Extract Method to pull the common logic into a shared method.

class A {
    public void method1() {
        commonMethod();
        doSomething3
    }
    public void method2() {
        commonMethod();
        doSomething4
    }
    public void commonMethod(){
       doSomething1
       doSomething2
    }
}

Two sibling subclasses contain the same statements.

class A extend C {
    public void method1(){
        doSomething1
        doSomething2
        doSomething3
    }
}

class B extend C {
    public void method1(){
        doSomething1
        doSomething2
        doSomething4
    }
}

Solution: extract the common method to the parent class.

class C {
    public void commonMethod(){
      doSomething1
      doSomething2
   }
}
class A extend C {
    public void method1(){
        commonMethod();
        doSomething3
    }
}

class B extend C {
    public void method1(){
        commonMethod();
        doSomething4
    }
}

Two unrelated classes contain duplicated code.

Solution: create a new utility class (Extract Class) and move the duplicated logic there.

2. Long Method

A long method spans hundreds of lines, reducing readability and making the code hard to understand. The article shows a bad example and a refactored version that splits the method into smaller, well‑named functions using Extract Method.

public class Test {
    private String name;
    private Vector<Order> orders = new Vector<Order>();
    public void printOwing(){
        // print banner
        System.out.println("****************");
        System.out.println("*****customer Owes *****");
        System.out.println("****************");
        // calculate totalAmount
        Enumeration env = orders.elements();
        double totalAmount = 0.0;
        while (env.hasMoreElements()) {
            Order order = (Order) env.nextElement();
            totalAmount += order.getAmout();
        }
        // print details
        System.out.println("name:" + name);
        System.out.println("amount:" + totalAmount);
        ......
    }
}

Refactored version using Extract Method:

public class Test {
    private String name;
    private Vector<Order> orders = new Vector<Order>();
    public void printOwing(){
        printBanner();
        double totalAmount = getTotalAmount();
        printDetail(totalAmount);
    }
    void printBanner(){
        System.out.println("****************");
        System.out.println("*****customer Owes *****");
        System.out.println("****************");
    }
    double getTotalAmount(){
        Enumeration env = orders.elements();
        double totalAmount = 0.0;
        while (env.hasMoreElements()) {
            Order order = (Order) env.nextElement();
            totalAmount += order.getAmout();
        }
        return totalAmount;
    }
    void printDetail(double totalAmount){
        System.out.println("name:" + name);
        System.out.println("amount:" + totalAmount);
    }
}

3. Large Class

A class that does too many things becomes hard to understand and maintain. The article demonstrates a class that mixes order, goods, and points logic and refactors it into three single‑responsibility classes.

Class A{
  public void printOrder(){
   System.out.println("订单");
  }
  public void printGoods(){
   System.out.println("商品");
  }
  public void printPoints(){
   System.out.println("积分");
  }
}

Refactored version:

Class Order{
  public void printOrder(){
   System.out.println("订单");
  }
}
Class Goods{
  public void printGoods(){
   System.out.println("商品");
  }
}
Class Points{
  public void printPoints(){
   System.out.println("积分");
  }
}

4. Long Parameter List

Methods with many parameters are hard to read and maintain. The article shows how to wrap parameters into a DTO.

public void getUserInfo(String name,String age,String sex,String mobile){
  // do something ...
}

After refactoring:

public void getUserInfo(UserInfoParamDTO userInfoParamDTO){
  // do something ...
}
class UserInfoParamDTO{
  private String name;
  private String age; 
  private String sex;
  private String mobile;
}

5. Divergent Change

When adding a new feature requires changing many methods in the same class, the code suffers from Divergent Change. The article uses a car example and suggests extracting interfaces and abstract classes.

public class Car {
    void start(Engine engine) { ... }
    void drive(Engine engine,Car car){ ... }
    String getBrand(Car car){ ... }
}

Refactored version introduces IEngine , AbstractCar , and concrete car classes.

public interface IEngine { void start(); }
public abstract class AbstractCar { protected IEngine engine; public AbstractCar(IEngine engine){ this.engine = engine; } public abstract void drive(); }
public class BenzCar extends AbstractCar { ... }
public class BaoMaCar extends AbstractCar { ... }

6. Shotgun Surgery

Implementing a small feature that requires changes in many classes leads to Shotgun Surgery. The article recommends moving related fields and methods into a single class.

public class DbAUtils { @Value("${db.mysql.url}") private String mysqlDbUrl; }
public class DbBUtils { @Value("${db.mysql.url}") private String mysqlDbUrl; }

After consolidation:

public class DbUtils { @Value("${db.mysql.url}") private String mysqlDbUrl; }

7. Feature Envy

A method that uses many members of another class should be moved to that class. The article shows moving getFullPhoneNumber from User to Phone .

public class User{ public void getFullPhoneNumber(Phone phone){ ... } }

8. Data Clumps

When several data items always appear together, they should be encapsulated into a value object.

public class User{ private String firstName; private String lastName; private String province; private String city; private String area; private String street; }

Refactored version:

public class User{ private UserName username; private Address address; }
class UserName{ private String firstName; private String lastName; }
class Address{ private String province; private String city; private String area; private String street; }

9. Primitive Obsession

Using many primitive types together is a smell; they should be wrapped in objects.

public class Order{ private String customName; private String address; private Integer orderId; private Integer price; }

Refactored version:

public class Order{ private Custom custom; private Integer orderId; private Integer price; }
public class Custom{ private String name; private String address; }

10. Switch Statements

Long switch or if‑else chains are hard to extend. The article suggests replacing them with polymorphism.

String medalType = "guest";
if ("guest".equals(medalType)) { System.out.println("嘉宾勋章"); }
else if ("vip".equals(medalType)) { System.out.println("会员勋章"); }
else if ("guard".equals(medalType)) { System.out.println("守护勋章"); }

Polymorphic solution using IMedalService and a factory.

public interface IMedalService { void showMedal(); }
public class GuardMedalServiceImpl implements IMedalService { public void showMedal(){ System.out.println("展示守护勋章"); } }
public class MedalServicesFactory { private static final Map
map = new HashMap<>(); static { map.put("guard", new GuardMedalServiceImpl()); ... } public static IMedalService getMedalService(String type){ return map.get(type); } }

11. Parallel Inheritance Hierarchies

When a change in one hierarchy forces a parallel change in another, the solution is to remove the unnecessary inheritance relationship.

Use a common abstraction or composition to avoid parallel hierarchies.

12. Lazy Class

If a class does very little, its responsibilities should be merged into a more relevant class and the empty class removed.

13. Speculative Generality

Avoid over‑designing code for future scenarios that may never be needed. Remove unused abstractions, collapse hierarchies, and eliminate unnecessary parameters.

14. Temporary Field

A field that is used only in a specific situation makes the class harder to understand. The article shows excessMinutesCharge as an example and suggests using a local variable instead.

public class PhoneAccount {
    private double excessMinutesCharge;
    private static final double RATE = 8.0;
    public double computeBill(int minutesUsed, int includedMinutes){
        excessMinutesCharge = 0.0;
        int excessMinutes = minutesUsed - includedMinutes;
        if (excessMinutes >= 1) {
            excessMinutesCharge = excessMinutes * RATE;
        }
        return excessMinutesCharge;
    }
    public double chargeForExcessMinutes(int minutesUsed, int includedMinutes){
        computeBill(minutesUsed, includedMinutes);
        return excessMinutesCharge;
    }
}

15. Message Chains

Long chains of method calls (e.g., A.getB().getC().getD() ) violate encapsulation. Break the chain by introducing a method that returns the needed data directly.

16. Middle Man

When a class delegates most of its work to another class, it becomes a middle man. Remove the middle man by letting clients use the real service directly.

17. Inappropriate Intimacy

Two classes that use each other's private members are tightly coupled. Extract the shared behavior into a common class to reduce intimacy.

18. Alternative Classes with Different Interfaces

When two classes provide similar functionality with different interfaces, unify them by renaming, moving methods, or introducing an abstract superclass.

19. Incomplete Library Class

If a third‑party library class lacks needed functionality, wrap it or create a helper class that adds the missing behavior.

20. Data Class

Classes that only hold data and expose fields should encapsulate related operations, hide collections, and remove unnecessary setters.

21. Refused Bequest

Subclasses that inherit methods they do not use indicate a flawed hierarchy. Push down unused methods/fields or replace inheritance with delegation.

22. Comments

Excessive or redundant comments are discouraged. Prefer self‑describing names; use comments only for complex business logic.

23. Bad Naming

Names should be clear and expressive. Avoid vague names like test ; use isParamPass instead.

24. Magic Numbers

Hard‑coded literals such as 1 and 2 should be replaced with named constants or enums.

25. Chaotic Layer Calls

Controllers should call services, which call DAOs. Direct controller‑to‑DAO calls break the layered architecture and should be refactored.

@RestController("user")
public class UserController {
    @Autowired
    private UserDao userDao;
    @RequestMapping("/queryUserInfo")
    public String queryUserInfo(String userName) {
        return userDao.selectByUserName(userName);
    }
}

References and Thanks

Software Engineering Lab: Common Code Smells and Refactoring Examples

22 Code Smells Summarized in One Sentence

Refactoring Bad Smells

Code Smell literature

"Refactoring: Improving the Design of Existing Code"