此文為譯文,原文地址請點選。
本文通過重構一個垃圾程式碼,闡述瞭如何寫出優秀的程式碼。開發人員及程式碼稽核人員需按照此規範開發和稽核程式碼。此規範以C#為例,JAVA的童鞋一併參考,C++的童鞋自行腦補吧。
簡介
這篇文章的目的是展示如何將一段垃圾程式碼重構成一個乾淨的、可擴充套件性和可維護的程式碼。我將解釋如何通過最佳實踐和更好的設計模式來改寫它。
閱讀本文你需要有以下基礎:
- c# 基礎
- 依賴注入,工廠模式,策略模式
此文中的例子源於實際專案,這裡不會有什麼使用裝飾模式構建的披薩,也不會使用策略模式的計算器,這些例子是非常好的說明,但是它們很難被在實際專案中使用。
一段垃圾程式碼
在我們真實的產品中有這麼一個類:
public class Class1
{
public decimal Calculate(decimal amount, int type, int years)
{
decimal result = 0;
decimal disc = (years > 5) ? (decimal)5/100 : (decimal)years/100;
if (type == 1)
{
result = amount;
}
else if (type == 2)
{
result = (amount - (0.1m * amount)) - disc * (amount - (0.1m * amount));
}
else if (type == 3)
{
result = (0.7m * amount) - disc * (0.7m * amount);
}
else if (type == 4)
{
result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount));
}
return result;
}
}
這是一段非常糟糕的程式碼(二胡:如果你沒覺得這段程式碼很糟糕,那你目前狀態可能很糟糕了),我們不太清楚這個類的目的是什麼。它可能是一個網上商城的折扣管理類,負責為客戶計算折扣。
這個類完全具備了不可讀、不可維護、不可擴充套件的特點,它使用了很多壞的實踐和反模式的設計。
下面我們逐步分析這裡到底有多少問題?
命名問題 - 我們只能通過猜測這個類到底是為了計算什麼。這實在是在浪費時間。
如果我們沒有相關文件或者重構這段程式碼,那我們下一次恐怕需要花大量的時間才能知道這段程式碼的具體含義。魔數 - 在這個例子中,你能猜到變數type是指客戶賬戶的狀態嗎。通過if-else來選擇計算優惠後的產品價格。
現在,我們壓根不知道賬戶狀態1,2,3,4分別是什麼意思。
此外,你知道0.1,0.7,0.5都是什麼意思嗎?
讓我們想象一下,如果你要修改下面這行程式碼:
result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount));
隱藏的BUG - 因為程式碼非常糟糕,我們可能會錯過非常重要的事情。試想一下,如果我們的系統中新增了一類賬戶狀態,而新的賬戶等級不滿足任何一個if-else條件。這時,返回值會固定為0。
不可讀 - 我們不得不承認,這是一段不可讀的程式碼。不可讀=更多的理解時間+增加產生錯誤的風險
DRY – 不要產生重複的程式碼
我們可能不能一眼就找出它們,但它們確實存在。
例如:disc *(amount - (0.1m * amount));
同樣的邏輯:
disc *(amount - (0.5m * amount));
這裡只有一個數值不一樣。如果我們無法擺脫重複的程式碼,我們會遇到很多問題。比如某段程式碼在五個地方有重複,當我們需要修改這部分邏輯時,你很可能只修改到了2至3處。單一職責原則
這個類至少具有三個職責:
1 選擇計算演算法
2 根據賬戶狀態計算折扣
3 根據賬戶網齡計算折扣
它違反了單一職責原則。這會帶來什麼風險?如果我們將要修改第三個功能的話,會影響到另外第二個功能。這就意味著,我們每次修改都會改動我們本不想修改的程式碼。因此,我們不得不對整個類進行測試,這實在很浪費時間。
重構
通過以下9布,我會告訴你們如何避免上述風險並實現一個乾淨的、可維護的、可測試的程式碼。
命名,命名,命名
這是良好程式碼的最重要方面之一。我們只需要改變方法,引數和變數的命名。現在,我們可以確切的知道下面的類是負責什麼的了。public decimal ApplyDiscount(decimal price, int accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100; if (accountStatus == 1) { priceAfterDiscount = price; } else if (accountStatus == 2) { priceAfterDiscount = (price - (0.1m * price)) - (discountForLoyaltyInPercentage * (price - (0.1m * price))); } else if (accountStatus == 3) { priceAfterDiscount = (0.7m * price) - (discountForLoyaltyInPercentage * (0.7m * price)); } else if (accountStatus == 4) { priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price))); } return priceAfterDiscount; }
但是我們任然不知道賬戶狀態1,2,3到底是什麼意思。
魔數
在C#中避免魔數我們一般採用列舉來替換它們。這裡使用AccountStatus 列舉來替換if-else中的魔數。
public enum AccountStatus { NotRegistered = 1, SimpleCustomer = 2, ValuableCustomer = 3, MostValuableCustomer = 4 }
現在我們來看看重構後的類,我們可以很容易的說出哪一個賬戶狀態應該用什麼演算法來計算折扣。混合賬戶狀態的風險迅速的下降了。public class DiscountManager { public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100; if (accountStatus == AccountStatus.NotRegistered) { priceAfterDiscount = price; } else if (accountStatus == AccountStatus.SimpleCustomer) { priceAfterDiscount = (price - (0.1m * price)) - (discountForLoyaltyInPercentage * (price - (0.1m * price))); } else if (accountStatus == AccountStatus.ValuableCustomer) { priceAfterDiscount = (0.7m * price) - (discountForLoyaltyInPercentage * (0.7m * price)); } else if (accountStatus == AccountStatus.MostValuableCustomer) { priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price))); } return priceAfterDiscount; } }
更多的程式碼可讀性
在這一步中,我們使用switch-case 來替換 if-else if來增強程式碼可讀性。
同時,我還將某些長度很長的語句才分成兩行。比如:**priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price)));**
被修改為:
**priceAfterDiscount = (price - (0.5m * price));priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);**
以下是完整的修改:public class DiscountManager { public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100; switch (accountStatus) { case AccountStatus.NotRegistered: priceAfterDiscount = price; break; case AccountStatus.SimpleCustomer: priceAfterDiscount = (price - (0.1m * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; case AccountStatus.ValuableCustomer: priceAfterDiscount = (0.7m * price); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; case AccountStatus.MostValuableCustomer: priceAfterDiscount = (price - (0.5m * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; } return priceAfterDiscount; } }
消除隱藏的BUG
正如我們之前提到的,我們的ApplyDiscount方法可能將為新的客戶狀態返回0。
我們怎樣才能解決這個問題?答案就是丟擲NotImplementedException。
當我們的方法獲取賬戶狀態作為輸入引數,但是引數值可能包含我們未設計到的未知情況。這時,我們不能什麼也不做,丟擲異常是這時候最好的做法。public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100; switch (accountStatus) { case AccountStatus.NotRegistered: priceAfterDiscount = price; break; case AccountStatus.SimpleCustomer: priceAfterDiscount = (price - (0.1m * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; case AccountStatus.ValuableCustomer: priceAfterDiscount = (0.7m * price); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; case AccountStatus.MostValuableCustomer: priceAfterDiscount = (price - (0.5m * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; default: throw new NotImplementedException(); } return priceAfterDiscount; }
- 分析演算法
在這個例子中,我們通過兩個標準來計算客戶折扣:
- 賬戶狀態
賬戶網齡
通過網齡計算的演算法都類似這樣:
(discountForLoyaltyInPercentage * priceAfterDiscount)
但是對於賬戶狀態為ValuableCustomer的演算法卻是:
0.7m * price
我們把它修改成和其他賬戶狀態一樣的演算法:
price - (0.3m * price)
public class DiscountManager { public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100; switch (accountStatus) { case AccountStatus.NotRegistered: priceAfterDiscount = price; break; case AccountStatus.SimpleCustomer: priceAfterDiscount = (price - (0.1m * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; case AccountStatus.ValuableCustomer: priceAfterDiscount = (price - (0.3m * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; case AccountStatus.MostValuableCustomer: priceAfterDiscount = (price - (0.5m * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; default: throw new NotImplementedException(); } return priceAfterDiscount; } }
消除魔數的另一種方法
使用靜態常量來替換魔數。0.1m,0.2m,0.3m,我m,我們並不知道它們是什麼意思。
此外decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5/100 : (decimal)timeOfHavingAccountInYears/100;中,數字5也非常神祕。
我們必須讓它們更具有描述性,這時使用常量會是一個比較好的辦法。
我們來定義一個靜態類:public static class Constants { public const int MAXIMUM_DISCOUNT_FOR_LOYALTY = 5; public const decimal DISCOUNT_FOR_SIMPLE_CUSTOMERS = 0.1m; public const decimal DISCOUNT_FOR_VALUABLE_CUSTOMERS = 0.3m; public const decimal DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS = 0.5m; }
接著修改DiscountManager類:
public class DiscountManager { public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY / 100 : (decimal)timeOfHavingAccountInYears / 100; switch (accountStatus) { case AccountStatus.NotRegistered: priceAfterDiscount = price; break; case AccountStatus.SimpleCustomer: priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; case AccountStatus.ValuableCustomer: priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; case AccountStatus.MostValuableCustomer: priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS * price)); priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount); break; default: throw new NotImplementedException(); } return priceAfterDiscount; } }
消除重複的程式碼
為了消除重複的程式碼,這裡將部分演算法提取出來。首先,我們建立兩個擴充套件方法:public static class PriceExtensions { public static decimal ApplyDiscountForAccountStatus(this decimal price, decimal discountSize) { return price - (discountSize * price); } public static decimal ApplyDiscountForTimeOfHavingAccount(this decimal price, int timeOfHavingAccountInYears) { decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY / 100 : (decimal)timeOfHavingAccountInYears / 100; return price - (discountForLoyaltyInPercentage * price); } }
通過方法名稱,我們就可以知道它的職責是什麼,現在修改我們的例子:
public class DiscountManager { public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; switch (accountStatus) { case AccountStatus.NotRegistered: priceAfterDiscount = price; break; case AccountStatus.SimpleCustomer: priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS) .ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears); break; case AccountStatus.ValuableCustomer: priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS) .ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears); break; case AccountStatus.MostValuableCustomer: priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS) .ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears); break; default: throw new NotImplementedException(); } return priceAfterDiscount; } }
刪除沒用的程式碼
我們應該寫出簡短的程式碼,因為簡短的程式碼=減少BUG發生的機率,並且也讓我們縮短理解業務邏輯的時間。
我們發現,這裡三種狀態的客戶呼叫了相同的方法:
.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
這裡可以合併程式碼:public class DiscountManager { public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; switch (accountStatus) { case AccountStatus.NotRegistered: priceAfterDiscount = price; break; case AccountStatus.SimpleCustomer: priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS); break; case AccountStatus.ValuableCustomer: priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS); break; case AccountStatus.MostValuableCustomer: priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS); break; default: throw new NotImplementedException(); } priceAfterDiscount = priceAfterDiscount.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears); return priceAfterDiscount; } }
9.最後,得到乾淨的程式碼
最後,讓我們通過引入依賴注入和工廠方法模式來得到最終的版本吧。
先來看卡最終結果:
首先,我們擺脫了擴充套件方法(靜態類),如果我們想對ApplyDiscount方法進行單元測試是比較困難的,廢除我們對PriceExtensions擴充套件類也進行測試。public class DiscountManager { private readonly IAccountDiscountCalculatorFactory _factory; private readonly ILoyaltyDiscountCalculator _loyaltyDiscountCalculator; public DiscountManager(IAccountDiscountCalculatorFactory factory, ILoyaltyDiscountCalculator loyaltyDiscountCalculator) { _factory = factory; _loyaltyDiscountCalculator = loyaltyDiscountCalculator; } public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price); priceAfterDiscount = _loyaltyDiscountCalculator.ApplyDiscount(priceAfterDiscount, timeOfHavingAccountInYears); return priceAfterDiscount; } } public interface ILoyaltyDiscountCalculator { decimal ApplyDiscount(decimal price, int timeOfHavingAccountInYears); } public class DefaultLoyaltyDiscountCalculator : ILoyaltyDiscountCalculator { public decimal ApplyDiscount(decimal price, int timeOfHavingAccountInYears) { decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY / 100 : (decimal)timeOfHavingAccountInYears / 100; return price - (discountForLoyaltyInPercentage * price); } } public interface IAccountDiscountCalculatorFactory { IAccountDiscountCalculator GetAccountDiscountCalculator(AccountStatus accountStatus); } public class DefaultAccountDiscountCalculatorFactory : IAccountDiscountCalculatorFactory { public IAccountDiscountCalculator GetAccountDiscountCalculator(AccountStatus accountStatus) { IAccountDiscountCalculator calculator; switch (accountStatus) { case AccountStatus.NotRegistered: calculator = new NotRegisteredDiscountCalculator(); break; case AccountStatus.SimpleCustomer: calculator = new SimpleCustomerDiscountCalculator(); break; case AccountStatus.ValuableCustomer: calculator = new ValuableCustomerDiscountCalculator(); break; case AccountStatus.MostValuableCustomer: calculator = new MostValuableCustomerDiscountCalculator(); break; default: throw new NotImplementedException(); } return calculator; } } public interface IAccountDiscountCalculator { decimal ApplyDiscount(decimal price); } public class NotRegisteredDiscountCalculator : IAccountDiscountCalculator { public decimal ApplyDiscount(decimal price) { return price; } } public class SimpleCustomerDiscountCalculator : IAccountDiscountCalculator { public decimal ApplyDiscount(decimal price) { return price - (Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS * price); } } public class ValuableCustomerDiscountCalculator : IAccountDiscountCalculator { public decimal ApplyDiscount(decimal price) { return price - (Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS * price); } } public class MostValuableCustomerDiscountCalculator : IAccountDiscountCalculator { public decimal ApplyDiscount(decimal price) { return price - (Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS * price); } }
為了避免這個問題,我們建立了DefaultLoyaltyDiscountCalculator 類來替換ApplyDiscountForTimeOfHavingAccount這個擴充套件方法,此類還實現了ILoyaltyDiscountCalculator介面。現在,當我們要測試DiscountManager類時,我們只建構函式注入ILoyaltyDiscountCalculator介面的實現即可。這裡使用了依賴注入。
通過這樣做,我們將網齡折扣的演算法遷移到類似DefaultLoyaltyDiscountCalculator 的不同類中,這樣當我們修改某一個演算法不會覆蓋到其他業務。
對於根據賬戶狀態來計算折扣的業務,我們需要在DiscountManager中刪除兩個職責:
- 根據賬戶狀態選擇計算的演算法
- 實現計算演算法
這裡我們通過DefaultAccountDiscountCalculatorFactory工廠類來解決這個問題,DefaultAccountDiscountCalculatorFactory工廠類實現了IAccountDiscountCalculatorFactory介面。
我們的工廠將決定選擇哪一個折扣演算法。接著,工廠類被通過建構函式注入到DiscountManager類中。
下面我只需要在DiscountManager 中使用工廠:
priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price);
以上,我們解決了第一個問題,下面我們需要實現計算演算法。根據賬戶狀態,提供不同的演算法,這正好符合策略模式。我們需要構建三個策略:NotRegisteredDiscountCalculator,SimpleCustomerDiscountCalculator,MostValuableCustomerDiscountCalculator已經抽象出來的介面IAccountDiscountCalculator。
好了,現在我們有可一段乾淨可讀的程式碼了,這段程式碼中所有的類都只有一個職責: - DiscountManager - 管理
- DefaultLoyaltyDiscountCalculator - 網齡計算折扣
- DefaultAccountDiscountCalculatorFactory - 根據賬戶狀態選擇折扣策略
NotRegisteredDiscountCalculator,SimpleCustomerDiscountCalculator,MostValuableCustomerDiscountCalculator-計算折扣演算法
我們來比較一下修改前後的程式碼:public class Class1 { public decimal Calculate(decimal amount, int type, int years) { decimal result = 0; decimal disc = (years > 5) ? (decimal)5 / 100 : (decimal)years / 100; if (type == 1) { result = amount; } else if (type == 2) { result = (amount - (0.1m * amount)) - disc * (amount - (0.1m * amount)); } else if (type == 3) { result = (0.7m * amount) - disc * (0.7m * amount); } else if (type == 4) { result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount)); } return result; } }
修改後:
public class DiscountManager { private readonly IAccountDiscountCalculatorFactory _factory; private readonly ILoyaltyDiscountCalculator _loyaltyDiscountCalculator; public DiscountManager(IAccountDiscountCalculatorFactory factory, ILoyaltyDiscountCalculator loyaltyDiscountCalculator) { _factory = factory; _loyaltyDiscountCalculator = loyaltyDiscountCalculator; } public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears) { decimal priceAfterDiscount = 0; priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price); priceAfterDiscount = _loyaltyDiscountCalculator.ApplyDiscount(priceAfterDiscount, timeOfHavingAccountInYears); return priceAfterDiscount; } }
總結
本文通過簡單易懂的方法重構了一段問題程式碼,它顯示瞭如何在實際情況中使用最佳實踐和設計模式來幫助我們寫出乾淨的程式碼。
就我的工作經驗來說,本文中出現的不良做法是經常發生的。編寫這種程式碼的人總是認為他們能夠保持這種規則,但不幸的是系統和業務往往都會越來越複雜,每次修改這類程式碼時都會帶來巨大的風險。