2011/8/2 23:28 |
閱讀數 : 3318
2 人推薦
我要推薦
|
| 文章分類 :
Tips
重構之路系列
訂閱
前言
這次的目標一樣是用抽象地角度來看我們的程式碼，用人類的話來解釋程式碼的目的與行為，並且避免重複的程式碼出現。
需求說明
我們有一個Bus的class，上面有一個Charge的方法，會根據乘客的年齡與性別，來決定要收費多少。
Spec是這樣寫的：
-
乘客是女生的話，低於60歲，收費為原價的8折；超過60歲則不收費。
-
乘客是男生的話，低於50歲，收費為原價的9折；50～60歲的，收費為原價的95折；超過60歲則不收費。
不先思考，直接動手寫程式，就會長出這樣『合理』的程式碼：
public class Bus
{
public double Charge(Person customer)
{
const double ticketCost = 100;
double result = 0;
if (customer.IsFemale)
{
if (customer.Age <= 60)
{
result = ticketCost * 0.8;
}
else
{
return 0;
}
}
else
{
if (customer.Age <= 50)
{
result = ticketCost * 0.9;
}
else if (customer.Age <= 60)
{
result = ticketCost * 0.85;
}
else
{
return 0;
}
}
return result;
}
}
這一段code問題在哪？我們繼續看下去。
重構步驟
步驟一：
首先，先看重複的code在哪裡，不管從需求或是程式碼，都可以看到：『超過60歲，則不收費』=> 這代表了老人的票價與性別無關。既然與性別無關，這一段程式碼就不該放在判斷性別的if判斷式裡面。
調整之後如下：
public double Charge(Person customer)
{
////超過60歲 => 老人，票價為0
if (customer.Age > 60)
{
return 0;
}
const double ticketCost = 100;
double result = 0;
if (customer.IsFemale)
{
if (customer.Age <= 60)
{
result = ticketCost * 0.8;
}
}
else
{
if (customer.Age <= 50)
{
result = ticketCost * 0.9;
}
else if (customer.Age <= 60)
{
result = ticketCost * 0.85;
}
}
return result;
}
步驟二：
還有哪邊是一樣的，乍看之下沒有，但倘若抽象一點來看（大家可以把眼睛瞇起來一點 XD），我們會看到，實際判斷式影響的內容是『折扣』。這個時候，千萬不要直接很高興、很帥氣的就把重複的東西都抽出來。要思考的是，是否計價的商業邏輯，就是『原價*折扣』。
建議此時可以問一下RA, SA或domain expert，詢問一下，計價方式=原價*折扣，『基本上』是否穩定不變。
我們假設專家的回答是：「基本上就是原價*折扣，但偶爾會有例外，而且折扣的值可能會變」。
基於這樣的domain know-how，我們可以再將計價公式抽象化。
步驟三：
接著，我們要消滅magic number，基本上除了0跟1以外，邏輯的code裡面應該是不會出現其他數字的。（甚至應該說，只有0是被允許的）
如果這樣還是不容易懂，簡單的說，我們應該賦予那些數字意義，用意義來寫code。程式碼才好懂，也才好維護。我們這邊用最簡單的方法，將這些數字定義成const常數。
public class Bus
{
private const double olderPrice = 0;
private const double youngLadyDiscount = 0.8;
private const double youngManDiscount = 0.9;
private const double strongManDiscount = 0.85;
private const int olderAge = 60;
private const int strongAge = 50;
private const double ticketCost = 100;
public double Charge(Person customer)
{
////超過60歲 => 老人，票價為0
if (customer.Age > olderAge)
{
return olderPrice;
}
////折扣
double discount = 0;
if (customer.IsFemale)
{
if (customer.Age <= olderAge)
{
discount = youngLadyDiscount;
}
}
else
{
if (customer.Age <= strongAge)
{
discount = youngManDiscount;
}
else if (customer.Age <= olderAge)
{
discount = strongManDiscount;
}
}
////計價方式=票價*折扣
double result = ticketCost * discount;
return result;
}
}
未來，倘若只是折扣改變，我們可以直接改const的值就可以。如果折扣是透過其他business logic來決定，那麼我們可以把const封裝成property，或其他function，這樣我們的Charge方法還是不需要改變。
如果，連計價方式都會有許多種，且未來可能有更多種，那麼計價方式可能就可以以其他的pattern來進行重構，例如strategy pattern。
結論
這一篇的重點在於，在條件式的分支中，有著相同的程式碼，代表著這一段程式碼並不會被這個條件式所影響，這時候放在條件式的分支中，就會顯得無法清楚解釋出邏輯。條件式分支中的程式碼，應該就只是說明，根據這樣的條件所影響不同的變化。
把原本順著spec寫的程式碼，重新的整理了一下，相信很多人會覺得原本的程式，並沒有太大問題。有沒有需要重構，基本上還算是見仁見智。不過，比較一下前後的程式碼，會不會覺得後面的程式碼比較乾淨一點，比較好懂一點，以後比較容易維護一點了呢？
最後，步驟三的程式碼，還是有重構的空間，不過這篇文章要表達的意思已經到了，我就不繼續往下重構了。『重構-改善既有程式的設計』這本書中的第二章『重構原則』中有提到，何時該重構，以及何時不該重構。建議大家可以看看。
希望每個工程師，都可以對的起自己寫出來的code，也都可以對他們負責 :)
↑ Grab this Headline Animator
關連文章
[測試]自動化測試經驗分享- MS TechDays 2011 BoF內容
[Memo]使用Trim(Char[])要注意的地方
[ASP.NET]重構之路系列v7 –簡化判斷式
[ASP.NET]重構之路系列v6 –抽象來看程式是否符合DRY原則