重構CH1//重構第一步

重構CH1//重構第一步
1. 第一步：

¶重構CH1//重構第一步

原文連結: https://darkblack01.blogspot.com/2012/11/ch1.html
移植時的最後更新日期: 2012-12-01T11:12:19.635+08:00

第一步：

找出程式碼的「邏輯泥團」（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中被最佳化
先讓程式碼有合的組識和管理，再來最佳化，才是正確的順序。

接下來進行第二步

發表於 2012-11-24

tags:

{ 重構 }