第一步:
找出程式碼的「邏輯泥團」(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中被最佳化
先讓程式碼有合的組識和管理,再來最佳化,才是正確的順序。
接下來進行第二步
沒有留言:
張貼留言
(什麼是留言欄訊息?)