25種代碼壞味道總結+優化示例

前言

什么樣的代碼是好代碼呢？好的代碼應該命名規范、可讀性強、擴展性強、健壯性......而不好的代碼又有哪些典型特征呢？這25種代碼壞味道大家要注意啦

1. Duplicated Code （重復代碼）

重復代碼就是不同地點，有着相同的程序結構。一般是因為需求迭代比較快，開發小伙伴擔心影響已有功能，就復制粘貼造成的。重復代碼很難維護的，如果你要修改其中一段的代碼邏輯，就需要修改多次，很可能出現遺漏的情況。

如何優化重復代碼呢？分三種情況討論：

1. 同一個類的兩個函數含有相同的表達式

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

優化手段：可以使用Extract Method(提取公共函數) 抽出重復的代碼邏輯，組成一個公用的方法。

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

2. 兩個互為兄弟的子類內含相同的表達式

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

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

優化手段：對兩個類都使用Extract Method(提取公共函數)，然后把抽取出來的函數放到父類中。

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
    }
}

3. 兩個毫不相關的類出現重復代碼

如果是兩個毫不相關的類出現重復代碼，可以使用Extract Class將重復代碼提煉到一個類中。這個新類可以是一個普通類，也可以是一個工具類，看具體業務怎么划分吧。

2 .Long 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);
        ......
    }
}

可以使用Extract Method，抽取功能單一的代碼段，組成命名清晰的小函數，去解決長函數問題，正例如下:

public class Test {
    private String name;
    private Vector<Order> orders = new Vector<Order>();

    public void printOwing() {

        //print banner
        printBanner();
        //calculate totalAmount
        double totalAmount = getTotalAmount();
        //print details
        printDetail(totalAmount);
    }

    void printBanner(){
        System.out.println("****************");
        System.out.println("*****customer Owes *****");
        System.out.println("****************");
    }

    double getTotalAmount(){
        Enumeration env = orders.elements();
        double totalAmount = .;
        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里面，商品庫存相關的也放在類A里面，積分相關的還放在類A里面...反例如下：

Class A{
  public void printOrder(){
   System.out.println("訂單");
  }
  
  public void printGoods(){
   System.out.println("商品");
  }
  
  public void printPoints(){
   System.out.println("積分");
  }
}

試想一下，亂七八糟的代碼塊都往一個類里面塞，還談啥可讀性。應該按單一職責，使用Extract Class把代碼划分開，正例如下：

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 (過長參數列)

方法參數數量過多的話，可讀性很差。如果有多個重載方法，參數很多的話，有時候你都不知道調哪個呢。並且，如果參數很多，做新老接口兼容處理也比較麻煩。

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

如何解決過長參數列問題呢？將參數封裝成結構或者類，比如我們將參數封裝成一個DTO類，如下：

public void getUserInfo（UserInfoParamDTO userInfoParamDTO){
  // do something ...
}

class UserInfoParamDTO{
  private String name;
  private String age; 
  private String sex;
  private String mobile;
}

5. Divergent Change （發散式變化）

對程序進行維護時, 如果添加修改組件, 要同時修改一個類中的多個方法, 那么這就是 Divergent Change。舉個汽車的例子，某個汽車廠商生產三種品牌的汽車：BMW、Benz和LaoSiLaiSi，每種品牌又可以選擇燃油、純電和混合動力。反例如下：

/**
 *  公眾號：撿田螺的小男孩
 */
public class Car {

    private String name;

    void start(Engine engine) {
        if ("HybridEngine".equals(engine.getName())) {
            System.out.println("Start Hybrid Engine...");
        } else if ("GasolineEngine".equals(engine.getName())) {
            System.out.println("Start Gasoline Engine...");
        } else if ("ElectricEngine".equals(engine.getName())) {
            System.out.println("Start Electric Engine");
        }
    }

    void drive(Engine engine,Car car) {
        this.start(engine);
        System.out.println("Drive " + getBrand(car) + " car...");
    }

    String getBrand(Car car) {
        if ("Baoma".equals(car.getName())) {
            return "BMW";
        } else if ("BenChi".equals(car.getName())) {
            return "Benz";
        } else if ("LaoSiLaiSi".equals(car.getName())) {
            return "LaoSiLaiSi";
        }
        return null;
    }
 }

如果新增一種品牌新能源電車，然后它的啟動引擎是核動力呢，那么就需要修改Car類的start和getBrand方法啦，這就是代碼壞味道：Divergent Change （發散式變化）。

如何優化呢？一句話總結：拆分類，將總是一起變化的東西放到一塊。

★

• 運用提煉類(Extract Class) 拆分類的行為。
• 如果不同的類有相同的行為，提煉超類(Extract Superclass) 和 提煉子類(Extract Subclass)。

”

正例如下：

因為Engine是獨立變化的，所以提取游戲出售平台地圖一個Engine接口，如果新加一個啟動引擎，多一個實現類即可。如下：

//IEngine
public interface IEngine {
    void start();
}

public class HybridEngineImpl implements IEngine { 
    @Override
    public void start() {
        System.out.println("Start Hybrid Engine...");
    }
}

因為drive方法依賴於Car，IEngine，getBand方法;getBand方法是變化的，也跟Car是有關聯的，所以可以搞個抽象Car的類，每個品牌汽車繼承於它即可，如下

public abstract class AbstractCar {

    protected IEngine engine;

    public AbstractCar(IEngine engine) {
        this.engine = engine;
    }

    public abstract void drive();
}

//奔馳汽車
public class BenzCar extends AbstractCar {

    public BenzCar(IEngine engine) {
        super(engine);
    }

    @Override
    public void drive() {
      this.engine.start();
      System.out.println("Drive " + getBrand() + " car...");
    }

    private String getBrand() {
        return "Benz";
    }
}

//寶馬汽車
public class BaoMaCar extends AbstractCar {

    public BaoMaCar(IEngine engine) {
        super(engine);
    }

    @Override
    public void drive() {
        this.engine.start();
        System.out.println("Drive " + getBrand() + " car...");
    }

    private String getBrand() {
        return "BMW";
    }
}

細心的小伙伴，可以發現不同子類BaoMaCar和BenzCar的drive方法，還是有相同代碼，所以我們可以再擴展一個抽象子類，把drive方法推進去，如下:

public abstract class AbstractRefinedCar extends AbstractCar {

    public AbstractRefinedCar(IEngine engine) {
        super(engine);
    }

    @Override
    public void drive() {
        this.engine.start();
        System.out.println("Drive " + getBrand() + " car...");
    }

    abstract String getBrand();
}

//寶馬
public class BaoMaRefinedCar extends AbstractRefinedCar {

    public BaoMaRefinedCar(IEngine engine) {
        super(engine);
    }

    @Override
    String getBrand() {
        return  "BMW";
    }
}

如果再添加一個新品牌，搞個子類，繼承AbstractRefinedCar即可，如果新增一種啟動引擎，也是搞個類實現IEngine接口即可

6. Shotgun Surgery（散彈式修改）

當你實現某個小功能時，你需要在很多不同的類做出小修改。這就是Shotgun Surgery（散彈式修改）。它跟發散式變化(Divergent Change) 的區別就是，它指的是同時對多個類進行單一的修改，發散式變化指在一個類中修改多處。反例如下：

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

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

多個類使用了db.mysql.url這個變量，如果將來需要切換mysql到別的數據庫，如Oracle，那就需要修改多個類的這個變量！

如何優化呢？將各個修改點，集中到一起，抽象成一個新類。

★

可以使用 Move Method （搬移函數）和 Move Field （搬移字段）把所有需要修改的代碼放進同一個類，如果沒有合適的類，就去new一個。

”

正例如下：

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

7. Feature Envy (依戀情節)

某個函數為了計算某個值，從另一個對象那里調用幾乎半打的取值函數。通俗點講，就是一個函數使用了大量其他類的成員，有人稱之為紅杏出牆的函數。反例如下：

public class User{
 private Phone phone;
  public User(Phone phone){
        this.phone = phone;
    }
    public void getFullPhoneNumber(Phone phone){
        System.out.println("areaCode:" + phone.getAreaCode());
        System.out.println("prefix：" + phone.getPrefix());
        System.out.println("number：" + phone.getNumber());
    }
}

如何解決呢？在這種情況下，你可以考慮將這個方法移動到它使用的那個類中。例如，要將 getFullPhoneNumber()從 User 類移動到Phone類中，因為它調用了Phone類的很多方法。

8. Data Clumps（數據泥團）

數據項就像小孩子，喜歡成群結隊地呆在一塊。如果一些數據項總是一起出現的，並且一起出現更有意義的，就可以考慮，按數據的業務含義來封裝成數據對象。反例如下：

public class User {

    private String firstName;
    private String lastName;

    private String province;
    private String city;
    private String area;
    private String street;
}

正例：

public class User {

    private UserName username;
    private Adress adress;
}

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 （基本類型偏執)

多數編程環境都有兩種數據類型，結構類型和基本類型。這里的基本類型，如果指Java語言的話，不僅僅包括那八大基本類型哈，也包括String等。如果是經常一起出現的基本類型，可以考慮把它們封裝成對象。我個人覺得它有點像Data Clumps（數據泥團） 舉個反例如下：

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

正例：

// 訂單類
public class Order {
    private Custom custom;
    private Integer orderId;
    private Integer price;
}
// 把custom相關字段封裝起來，在Order中引用Custom對象
public class Custom {
    private String name;
    private String address;
}

當然，這里不是所有的基本類型，都建議封裝成對象，有關聯或者一起出現的，才這么建議哈。

10. Switch Statements （Switch 語句）

這里的Switch語句，不僅包括Switch相關的語句，也包括多層if...else的語句哈。很多時候，switch語句的問題就在於重復，如果你為它添加一個新的case語句，就必須找到所有的switch語句並且修改它們。

示例代碼如下：

    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("守護勛章");
    }
    ...

這種場景可以考慮使用多態優化：

//勛章接口
public interface IMedalService {
    void showMedal();
}

//守護勛章策略實現類
public class GuardMedalServiceImpl implements IMedalService {
    @Override
    public void showMedal() {
        System.out.println("展示守護勛章");
    }
}
//嘉賓勛章策略實現類
public class GuestMedalServiceImpl implements IMedalService {
    @Override
    public void showMedal() {
        System.out.println("嘉賓勛章");
    }
}

//勛章服務工廠類
public class MedalServicesFactory {

    private static final Map<String, IMedalService> map = new HashMap<>();
    static {
        map.put("guard", new GuardMedalServiceImpl());
        map.put("vip", new VipMedalServiceImpl());
        map.put("guest", new GuestMedalServiceImpl());
    }
    public static IMedalService getMedalService(String medalType) {
        return map.get(medalType);
    }
}

當然，多態只是優化的一個方案，一個方向。如果只是單一函數有些簡單選擇示例，並不建議動不動就使用動態，因為顯得有點殺雞使用牛刀了。

11.Parallel Inheritance Hierarchies（ 平行繼承體系）

平行繼承體系 其實算是Shotgun Surgery的特殊情況啦。當你為A類的一個子類Ax，也必須為另一個類B相應的增加一個子類Bx。

解決方法：遇到這種情況，就要消除兩個繼承體系之間的引用，有一個類是可以去掉繼承關系的。

12. Lazy Class (冗贅類)

把這些不再重要的類里面的邏輯，合並到相關類，刪掉舊的。一個比較常見的場景就是，假設系統已經有日期工具類DateUtils，有些小伙伴在開發中，需要用到日期轉化等，不管三七二十一，又自己實現一個新的日期工具類。

13. Speculative Generality(誇誇其談未來性)

盡量避免過度設計的代碼。例如：

• 只有一個if else，那就不需要班門弄斧使用多態;
• 如果某個抽象類沒有什么太大的作用，就運用Collapse Hierarchy（折疊繼承體系）
• 如果函數的某些參數沒用上，就移除。

14. Temporary Field(令人迷惑的臨時字段)

某個實例變量僅為某種特定情況而定而設，這樣的代碼就讓人不易理解，我們稱之為 Temporary Field(令人迷惑的臨時字段)。反例如下:

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;
    }
}

思考一下，臨時字段excessMinutesCharge是否多余呢？

15. Message Chains (過度耦合的消息鏈)

當你看到用戶向一個對象請求另一個對象，然后再向后者請求另一個對象，然后再請求另一個對象...這就是消息鏈。實際代碼中，你看到的可能是一長串getThis（）或一長串臨時變量。反例如下：

A.getB().getC().getD().getTianLuoBoy().getData();

A想要獲取需要的數據時，必須要知道B，又必須知道C,又必須知道D...其實A需要知道得太多啦，回頭想下封裝性，嘻嘻。其實可以通過拆函數或者移動函數解決，比如由B作為代理，搞個函數直接返回A需要數據。

16. Middle Man （中間人）

對象的基本特征之一就是封裝，即對外部世界隱藏其內部細節。封裝往往伴隨委托，過度運用委托就不好：某個類接口有一半的函數都委托給其他類。可以使用Remove Middle Man優化。反例如下：

A.B.getC(){
   return C.getC();
}

其實，A可以直接通過C去獲取C，而不需要通過B去獲取。

17. Inappropriate Intimacy（狎昵關系）

如果兩個類過於親密，過分狎昵，你中有我，我中有你，兩個類彼此使用對方的私有的東西，就是一種壞代碼味道。我們稱之為Inappropriate Intimacy（狎昵關系）

建議盡量把有關聯的方法或屬性抽離出來，放到公共類，以減少關聯。

18. Alternative Classes with Different Interfaces （異曲同工的類）

A類的接口a，和B類的接口b，做的的是相同一件事，或者類似的事情。我們就把A和B叫做異曲同工的類。

可以通過重命名，移動函數，或抽象子類等方式優化

19. Incomplete Library Class (不完美的類庫)

大多數對象只要夠用就好，如果類庫構造得不夠好，我們不可能修改其中的類使它完成我們希望完成的工作。可以醬紫：包一層函數或包成新的類。

20. Data Class （純數據類）

什么是Data Class? 它們擁有一些字段，以及用於訪問(讀寫)這些字段的函數。這些類很簡單，僅有公共成員變量，或簡單操作的函數。

如何優化呢？將相關操作封裝進去，減少public成員變量。比如：

• 如果擁有public字段-> Encapsulate Field
• 如果這些類內含容器類的字段，應該檢查它們是不是得到了恰當地封裝-> Encapsulate Collection封裝起來
• 對於不該被其他類修改的字段-> Remove Setting Method->找出取值/設置函數被其他類運用的地點-> Move Method 把這些調用行為搬移到Data Class來。如果無法搬移整個函數，就運用Extract Method產生一個可被搬移的函數->Hide Method把這些取值/設置函數隱藏起來。

21. Refused Bequest （被拒絕的饋贈）

子類應該繼承父類的數據和函數。子類繼承得到所有函數和數據，卻只使用了幾個，那就是繼承體系設計錯誤，需要優化。

• 需要為這個子類新建一個兄弟類->Push Down Method和Push Down Field把所有用不到的函數下推給兄弟類，這樣一來，超類就只持有所有子類共享的東西。所有超類都應該是抽象的。
• 如果子類復用了超類的實現，又不願意支持超類的接口，可以不以為然。但是不能胡亂修改繼承體系->Replace Inheritance with Delegation(用委派替換繼承).

22. Comments (過多的注釋)

這個點不是說代碼不建議寫注釋哦，而是，建議大家避免用注釋解釋代碼，避免過多的注釋。這些都是常見注釋的壞味道：

• 多余的解釋
• 日志式注釋
• 用注釋解釋變量等
• ...

如何優化呢？

• 方法函數、變量的命名要規范、淺顯易懂、避免用注釋解釋代碼。
• 關鍵、復雜的業務，使用清晰、簡明的注釋

23. 神奇命名

方法函數、變量、類名、模塊等，都需要簡單明了，淺顯易懂。避免靠自己主觀意識瞎起名字。

反例：

boolean test = chenkParamResult(req);

正例：

boolean isParamPass = chenkParamResult(req);

24. 神奇魔法數

日常開發中，經常會遇到這種代碼：

if(userType==1){
   //doSth1
}else If( userType ==2){
   //doSth2
}
...

代碼中的這個1和2都表示什么意思呢？再比如setStatus(1)中的1又表示什么意思呢？看到類似壞代碼，可以這兩種方式優化：

• 新建個常量類，把一些常量放進去，統一管理，並且寫好注釋；
• 建一個枚舉類，把相關的魔法數字放到一起管理。

25. 混亂的代碼層次調用

我們代碼一般會分dao層、service層和controller層。

• dao層主要做數據持久層的工作，與數據庫打交道。
• service層主要負責業務邏輯處理。
• controller層負責具體的業務模塊流程的控制。

所以一般就是controller調用service，service調dao。如果你在代碼看到controller直接調用dao，那可以考慮是否優化啦。反例如下：

@RestController("user")
public class UserController {

    Autowired
    private UserDao userDao;

    @RequestMapping("/queryUserInfo")
    public String queryUserInfo(String userName) {
        return userDao.selectByUserName(userName);
    }
}