2012年11月24日 星期六

重構CH1//重構第一步

第一步:

找出程式碼的「邏輯泥團」(logical clump),運用Extract Method
本例中的logical clump就是switch(),將它拉出來成為一個獨立的method

安全做法參考書後的refactoring catalog(重構名錄)

1. 找出函式內的區域變數和參數

    找到兩個: each, thisAmount(前者不會被修改,後者會被修改)
    不會被修改的變數,都可以被傳入新的函式
    會被修改的變數,就要格外小心
        只有一個變數修改,就把它當作是返回值

        thisAmount是個暫時變數,其值在每次迴圈開始處被設為0
                                                                    在switch之前不會改變,所以我可以直接把新函式的返回值賦值給它
inline std::string Customer::statement()
{
double totalAmount = 0; //消費總金額
int frequentReterPoints = 0; //常客積點
std::vector::const_iterator rentals = _rentals.begin();
std::string result = "Rental Record for " + getName() + "\n";

while(retals.hasMoreElements())
{
double thisAmount = 0;
Rental each = (Rental)rentals.next(); //取得一筆租借記錄

//determine amounts for each line
thisAmount = amountFor(each); //程式碼被移到amountFor()改成這一行

//add frequent renter points(累加 常客積點)
frequentReterPoints++;

//add bonus for a two day new release rental
if ((each.getMovie().getPriceCode() == Movie.NEW.RELEASE) && each.getDayRented() > 1))
frequentReterPoints++;

//show figures for this rental (顯示這筆租借資料)
result += "\t" + each.getMovie().getTitle() + "\t" + std::string.valueOf(thisAmount) + "\n";
totalAmount += thisAmount;
}

//add footer lines(結尾列印)
result += "Amount owed is " + std::string.valueOf(totalAmount) + "\n";
rental += "You earned " + std::string.valueOf(frequentReterPoints) + " frequent renter points";

return result;
}

inline int Customer::amountFor(Rental each)
{
int thisAmount = 0;//新增的一行

//原本的程式碼移過來
//--------------------------------------------------
switch(each.getMovie().getPriceCode()) //取得影片出租價格
{
case Movie.REGULAR:
thisAmount += 2;
if (each.getDayRented() > 2)
thisAmount += (each.getDayRented()-2)*1.5;
break;

case Movie.NEW_RELEASE:
thisAmount += each.getDayRented()*3;
break;

case Movie.CHILDRENS:
thisAmount += 1.5;
if (each.getDayRented() > 3)
thisAmount += (each.getDayRented()-3)*1.5;
break;
}
//--------------------------------------------------
return thisAmount;
}

此次修改出錯:
返回值的型態和賦值型態不合
int = double

重構精神:
修改的幅度小,很好的測試,容易發現任何錯誤,不用花費時間除錯



2. 修改Extract Method之後新函式裡的變數名稱

變數名稱,是程式碼清晰的關鍵。

each → aRental
thisAmount → result
inline double Customer::amountFor(Rental aRental)
{
double result = 0;

switch(aRental.getMovie().getPriceCode()) //取得影片出租價格
{
case Movie.REGULAR:
result += 2;
if (aRental.getDayRented() > 2)
result += (aRental.getDayRented()-2)*1.5;
break;

case Movie.NEW_RELEASE:
result += aRental.getDayRented()*3;
break;

case Movie.CHILDRENS:
result += 1.5;
if (aRental.getDayRented() > 3)
result += (aRental.getDayRented()-3)*1.5;
break;
}

return result;
}




3.搬移amountFor()(金額計算)程式碼

amountFor()大量的使用了Rental的變數,沒有使用Customer的變數
使用Move Method,將此函數,換一個更適合的class
(意味著去掉參數)

amountFor()從class Customer→搬家到→class Rental
(aRental.就去掉了直接呼叫getMovie()等Rental的函數)

並且修改名稱
amountFor() → getCharge()
inline double Rental::getCharge()
{
double result = 0;

switch(getMovie().getPriceCode()) //取得影片出租價格
{
case Movie.REGULAR:
result += 2;
if (getDayRented() > 2)
result += (getDayRented()-2)*1.5;
break;

case Movie.NEW_RELEASE:
result += getDayRented()*3;
break;

case Movie.CHILDRENS:
result += 1.5;
if (getDayRented() > 3)
result += (getDayRented()-3)*1.5;
break;
}

return result;
}
原本的amountFor()修改如下:
inline int Customer::amountFor(Rental aRental)
{
return aRental.getCharge();
}




4. 回到Customer::statemetn()

找出所有的reference(引用)點,並修改它們,讓它們改用新函式
要盡量的放棄掉Customer中,因為重構而產生的Method,而直接呼叫已經移到Rental.getCharge()

在本例中,只有一個地方,一般來說所有的classes都要找一遍。

thisAmount = amountFor(each);  →  thisAmount = each.getCharge();

這一步自己做過一次,真的很有體會。(each從舊用法到新用法)

對於Customer::amountFor(Rental aRental)的去留,也是很有意思
若它是public: 那也許留下呼叫新函式,可以省下修改更多程式碼。(不想修改其它class介面時)
若它是private: 那就刪掉吧。


5. 去除thisAmount這個暫時變數

因為thisAmount在接收each.getCharge()的執行結果之後就沒有再變過。
運用Replace Temp with Query

inline std::string Customer::statement()
{
double totalAmount = 0; //消費總金額
int frequentReterPoints = 0; //常客積點
std::vector::const_iterator rentals = _rentals.begin();
std::string result = "Rental Record for " + getName() + "\n";

while(retals.hasMoreElements())
{
//double thisAmount = 0;
Rental each = (Rental)rentals.next(); //取得一筆租借記錄

//determine amounts for each line
//thisAmount = each.getCharge();

//add frequent renter points(累加 常客積點)
frequentReterPoints++;

//add bonus for a two day new release rental
if ((each.getMovie().getPriceCode() == Movie.NEW.RELEASE) && each.getDayRented() > 1))
frequentReterPoints++;

//show figures for this rental (顯示這筆租借資料)
result += "\t" + each.getMovie().getTitle() + "\t" + std::string.valueOf(each.getCharge()) + "\n";
totalAmount += each.getCharge();
}

//add footer lines(結尾列印)
result += "Amount owed is " + std::string.valueOf(totalAmount) + "\n";
rental += "You earned " + std::string.valueOf(frequentReterPoints) + " frequent renter points";

return result;
}

除去暫時變數,它往往形成問題
導致大量參數被傳來傳去,而其實完全沒有這種必要。
很容易失去他們的縱跡,尤其在長長的函式之中更是如此。

雖然這會付出效率的代價,但是這很容易在Rental class中被最佳化
先讓程式碼有合的組識和管理,再來最佳化,才是正確的順序。

接下來進行第二步

沒有留言:

張貼留言