重构，第一个案例（C++版）——分解并重组Statement()

在上一篇文章——重构，第一个案例（C++版）——最初的程序，我们已经实现了一个影片出租程序的最初版本。我们也分析了，这个版本的程序虽然能跑起来，没有bug。但是，明显的，程序中有一些“代码的坏味道”。为了重构它，我们首先写出了一段测试代码，方便我们重构的时候进行测试。

下面我们的重点就在于将已经看得很不顺眼的长函数Statement进行大卸八块了。记住，代码块越小，代码的功能就越容易管理，代码的处理和移动也就越轻松。

解决逻辑泥团

首先，在Statement()中我们一眼能看到的就是包含switch语句的这团逻辑泥团。我们将用Extrate Method（提炼函数）的方法将这团代码提炼成一个函数。经过重构的部分代码如下, Statement函数中没有改动的部分我用...省略表示了（后面代码中的...都表示省略的相同代码，不赘述）。

    string Statement() {
        ...
        this_amount = AmountFor(each);
        ...
    }

private:
    double AmountFor(Rental each) {
        double this_amount = 0;
        switch (each.GetMovie().GetPriceCode()) {
        case PriceCode::REGULAR:
            this_amount += 2;
            if (each.GetDaysRented() > 2)
                this_amount += (each.GetDaysRented() - 2) * 1.5;
            break;
        case PriceCode::NEW_RELEASE:
            this_amount += each.GetDaysRented() * 3;
            break;
        case PriceCode::CHILDREN:
            this_amount += 1.5;
            if (each.GetDaysRented() > 3)
                this_amount += (each.GetDaysRented() - 3) * 1.5;
            break;
        }
        return this_amount;                          
    }

书中，作者还展示了他在重构的一开始，错误的将提炼后的函数返回值定义成int，然后运行测试代码后就马上发现了错误。显示了测试在代码重构过程中的重要性。作者总结道：

重构技术就是以微小的步伐修改程序。如果你犯下错误，很容易便可发现它。

然后，我们发现新提炼出来的AmountFor函数中有些变量名是让人不好理解的。这里可以更改一下这些变量名，将入参改为由each改为rental，将计算后的返回值由this_amount改为result，更名后代码如下。

private:
    double AmountFor(Rental rental) {
        double result = 0;
        switch (rental.GetMovie().GetPriceCode()) {
        case PriceCode::REGULAR:
            result += 2;
            if (rental.GetDaysRented() > 2)
                result += (rental.GetDaysRented() - 2) * 1.5;
            break;
        case PriceCode::NEW_RELEASE:
            result += rental.GetDaysRented() * 3;
            break;
        case PriceCode::CHILDREN:
            result += 1.5;
            if (rental.GetDaysRented() > 3)
                result += (rental.GetDaysRented() - 3) * 1.5;
            break;
        }
        return result;                          
    }

为什么要做这样看似无用的操作呢？因为，变量名称是代码清晰的关键。而使代码清晰，方便其他人阅读、理解是每个程序员应有的觉悟。目前，无论是编辑器还是ide都给我们提供了很多很方便更改名称的重构功能，那么为什么不用起来呢。没有注释，就是最好的注释，因为你的代码已经很清晰的表达了你的想法。

任何一个傻瓜都能写出计算机可以理解的代码。唯有写出人类容易理解的代码，才是优秀的程序员。

搬移“金额计算”代码

下面我们将注意力放在刚刚提炼出来的AmountFor函数中，我们可以发现该函数中使用了Rental类的函数，而没有使用它所在的对象Customer类中的函数或者变量。这里我们就应该感觉到有问题了。

绝大多数情况下，函数应该放在它所使用的数据的所属对象内。

我们应该使用Move Method（搬移函数）的方法，将AmountFor()搬移到Rental类中。首先，我们要把代码复制到Rental类中。然后，让它“适应新家”——要做的工作包括，去掉参数rental，更改函数名称为GetCharge。最后在AmountFor函数中调用我们刚刚搬移的新函数。修改完成后编译运行下测试，看看有没有问题。修改的代码如下。

class Rental
{
    ...
    double GetCharge() {
        double result = 0;
        switch (GetMovie().GetPriceCode()) {
        case PriceCode::REGULAR:
            result += 2;
            if (GetDaysRented() > 2)
                result += (GetDaysRented() - 2) * 1.5;
            break;
        case PriceCode::NEW_RELEASE:
            result += GetDaysRented() * 3;
            break;
        case PriceCode::CHILDREN:
            result += 1.5;
            if (GetDaysRented() > 3)
                result += (GetDaysRented() - 3) * 1.5;
            break;
        }
        return result;                          
    }
     ...
}

class Customer
{
    ...
private:
    double AmountFor(Rental each) {
        return each.GetCharge();                          
    }
    ...
}

搬移完函数后，我们就需要做对旧函数的善后工作了。首先找出所有使用到旧函数AmountFor()的位置，将其替换为我们的新函数。所幸，在我们这个程序中只有Statement函数中这一处使用了它，修改起来比较简单。一般情况下，我们可以使用查找工具找出旧函数的所有引用的地方。替换工作完成后，我们就可以删除掉旧函数。最后，按照惯例，我们需要运行下测试代码，看看有没有改出什么新问题。这里我们修改的代码就比较简单了（修改的代码省略了删除的说明，下同）。

class Customer
{
    ...
    this_amount = each.GetCharge();
    ...
}

接下来我们将会注意到刚刚修改的地方，有一个变量this_amount显得有点多余了。它被each.GetCharge()赋值后就没有任何变化了。所以，我们可以使用Replace Temp with Query（以查询取代临时变量）。这里的查询实际上是指的一个函数表达式。在我们的程序中就是删除掉this_amount，将使用这个临时变量的位置直接用each.GetCharge()来替换。

class Customer
{
    ...
    result += "\t" + each.GetMovie().GetTitle() + "\t" +
        to_string(each.GetCharge()) + "\n";
    total_amount += each.GetCharge();
    ...
}

这里对临时变量的修改会付出性能上的代价，因为去掉了临时变量，each.GetCharge()会被调用两次。但作者觉得这种代价是值得的，这样的改动更有利于今后的优化。为此作者专门用了一节——“重构与性能”来谈这个问题，有兴趣的同学可以找书来看看。后文中我也会提到一点。

经过这次修改后，我们程序的UML类图变为了下面这样。

提炼“常客积分计算”代码

下一步我们就要对“常客积分计算”这部分的代码做类似上面的处理了。简单分析下，积分的计算一般只会因为影片种类而不同，且其计算方式一般变动不大。那么这部分的代码像“金额计算”一样搬移在Rental类中应该是合适的。这里我们使用的重构方法也和上面类似，使用Extrate Method（提炼函数）提炼出新的“常客积分计算”的函数。然后使用Move Method（搬移函数）将新函数搬移到Rental类中使它适应新家。最后常规操作，运行测试代码。下面是这次修改后的代码。

class Rental
{
    ...
    int GetFrequentRenterPoints() {
        if (GetMovie().GetPriceCode() == PriceCode::NEW_RELEASE &&
            GetDaysRented() > 1)
            return 2;
        else
            return 1;    
    }
     ...
}

class Customer
{
    ...
    string Statement() {
    ...
            // add frequent renter points
            frequent_renter_points += each.GetFrequentRenterPoints();
    ...
    }
    ...
}

去除临时变量

最后，我们要针对Statement函数中两个临时变量进行重构。在这个函数的开始部分我们定义了两个总量的临时变量——total_amount和frequent_renter_points。这两个变量都是用来从Customer对象相关的Rental对象中获取某个总量的。现在我们Statement函数的输出是ASCII版的，如果以后我们要增加其他输出版本的（而这是很有可能发生的），比如需要HTML版的，那么获取价格总量和积分总量的函数就是这两个版本都需要的。这就是促使我们进行这个重构的理由之一。重构方式也很简单，在删除掉这两个临时变量后，我们需要提供两个private的函数来替代者两个临时变量。当然在这两个新函数中不可避免的会有包含有循环语句的代码，这里就可能会影响程序的性能。关于这个问题，我们后面再来讨论。经过这轮重构后，我们也可以看到Statement函数已经变得比之前“清爽”太多了。可以来看看目前的代码。

class Customer
{
    ...
        string Statement() {
        string result = "Rental Record for " + GetName() + "\n";
        for (auto& each : rentals_) {
            // show figures for this rental
            result += "\t" + each.GetMovie().GetTitle() + "\t" +
                to_string(each.GetCharge()) + "\n";
        }

        // add footer lines
        result += "Amount owed is " + to_string(GetTotalCharge()) + "\n";
        result += "You earned " + to_string(GetTotalFrequentRenterPoints()) +
            " frequent renter points";
        
        return result;
    }

private:
    double GetTotalCharge() {
        double result = 0;
        for (auto& each : rentals_) {
            result += each.GetCharge();
        }
        return result;
    }
    double GetTotalFrequentRenterPoints() {
        double result = 0;
        for (auto& each : rentals_) {
            result += each.GetFrequentRenterPoints();
        }
        return result;
    }
    ...
}

经过重构后我们的UML类图和序列图如下。

这轮重构后，我们来做个小小的总结。先来说说经过这轮重构后我们得到了什么。首先，我们得到了一个“清爽的”、扩展性更强的Statement函数。并且我们让类的结构更合理了。那么我们引来了什么问题呢？通过运行测试代码，我们可以肯定，经过重构我们没有引入bug，这是非常重要的。但是，我们还是引入了两个问题。第一个是我们增加了代码总量，虽然重构通常会减少代码总量，这次我们增加了代码总量，但我觉得在这次重构中这个问题影响不大。第二个就是之前说得性能问题，我们在去掉Statement函数中的两个临时变量时，增加了两个新的函数，而这两个新函数中都包含了循环语句。也就是说经过重构我们的Statement函数会执行3次同样的循环过程。如果这里循环耗时过多的话，就很可能会大大的降低程序的性能。因为这个原因，包括我在内很多程序员一开始会不愿意进行这个重构。但是，这里作者给出了他的两个理由。而这两个理由我是接受的。

• 第一个理由实际上是作者专门讨论这个问题的章节“重构与性能”中所提及的核心观点，也就是目前我们不确定这个循环的执行时间是否会构成我们的性能瓶颈（在不大部分情况下，实际影响不大）。所以在重构时，我们可以先忽略这个问题，而在优化程序性能的阶段，通过专门的工具去找到程序的性能瓶颈，在针对瓶颈进行优化。这时候的优化因为重构后结构更清晰，会使得优化效果更好，也更容易。
• 而第二个理由则是，通过新函数的提炼，实际上我们在Customer类中增加了两个新功能。作为private函数，我们在Customer类中要新增新版本的Statement函数时，就可以使用者两个新函数。而如果外面有地方需要计算这两个总量时，我们也可以开放这两个函数，将其改为public的。这样外面使用时，也只用调用Customer类的接口，而不用去了解Rental类。

扩展打印输入

接下来我们可以来简单的检验下我们重构的效果。比如前面提到的情况发生了，现在我们的客户提出了输出需要变成HTML格式的。那么为了添加这个功能，我们只用添加一个HTML版的Statement函数即可。其代码如下。

    string HtmlStatement() {
        string result = "<H1>Rental for <EM>" + GetName() + "</EM></H1><p>\n";
        for (auto& each : rentals_) {
            // show figures for this rental
            result +=  each.GetMovie().GetTitle() + ":" +
                to_string(each.GetCharge()) + "<BR>\n";
        }

        // add footer lines
        result += "<P> You owe <EM>" + to_string(GetTotalCharge()) + "</EM><P>\n";
        result += "On this rental you earned <EM>" + to_string(GetTotalFrequentRenterPoints()) +
            "<EM> frequent renter points<P>";
        
        return result;
    }

可以看到这个HTML版的Statement写起来很方便，基本逻辑都是复制的已经重构好的Statement()，当然这里还存在着一些重复的代码，我们可以通过进一步的重构消除它们，但这个不在目前的讨论范围。目前这样的代码的好处也很明显，如果客户想改变金额的计算方式，或者是积分的计算方式，那么我们只需要在程序的一处做修改（Rental类中相应的函数），从而避免了散弹式修改。
最后我们还是来运行下测试代码，检验下效果。

思路总结

• 找出程序中的逻辑泥团。
• 分析代码结构，提炼、搬移函数，让它们“各回各家”。
• 去除临时变量，让结构更清晰。
• 每一小步重构都需要运行测试代码以避免引入bug。

本系列文章

找出那些代码里的坏味道吧——《重构》笔记（一）
重构，第一个案例（C++版）——最初的程序
重构，第一个案例（C++版）——分解并重组Statement()
重构，第一个案例（C++版）——运用多态取代与价格相关的条件逻辑