此文為譯文，原文地址請點擊。
本文通過重構一個垃圾代碼，闡述了如何寫出優秀的代碼。開發人員及代碼審核人員需按照此規范開發和審核代碼。此規范以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.最后，得到干凈的代碼
最后，讓我們通過引入依賴注入和工廠方法模式來得到最終的版本吧。
先來看卡最終結果：
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);
}
}
首先，我們擺脫了擴展方法（靜態類），如果我們想對ApplyDiscount方法進行單元測試是比較困難的，廢除我們對PriceExtensions擴展類也進行測試。
為了避免這個問題，我們創建了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;
}
}
總結
本文通過簡單易懂的方法重構了一段問題代碼，它顯示了如何在實際情況中使用最佳實踐和設計模式來幫助我們寫出干凈的代碼。
就我的工作經驗來說，本文中出現的不良做法是經常發生的。編寫這種代碼的人總是認為他們能夠保持這種規則，但不幸的是系統和業務往往都會越來越復雜，每次修改這類代碼時都會帶來巨大的風險。