《重构》学习（1）拆分 statement 函数-CFANZ编程社区

1.1 起点

假设我们是一个剧团，我们剧团会被邀请去各种剧场表演。
一般由客户（custom）来指定剧目，剧团就根据观众数量和剧目类型来向客户收费，我们需要设计一个系统来实现，需要有下面这些功能点：

该剧团目前只有两种类型的戏剧：喜剧（comedy）、悲剧（tragedy）
需要根据观众（audience）和剧目类型（type）来算出账单
给客户发出账单时，剧团需要根据到场观众数量，给出“观众量积分”（volume credit），下次客户再来看表演时，可以通过积分获得折扣

首先我们把剧团的剧目信息存储在一个 map 中：

data class PlayInfo(val name: String, val type: String)
val playInfos: MutableMap<String, PlayInfo> = mutableMapOf(
    Pair("西红市首富", PlayInfo("西红市首富", "comedy")),
    Pair("夏洛的烦恼", PlayInfo("夏洛的烦恼", "comedy")),
    Pair("新宿事件", PlayInfo("新宿事件", "tragedy"))
)

其次，客户请剧团演出后的相关信息如下：

data class PerformancesData(val playId: String, val audience: Int)
data class Invoices(val customer: String, val performances: List<PerformancesData>)
val invoices = Invoices(
    "中船戏院", listOf(
        PerformancesData("西红市首富", 55),
        PerformancesData("夏洛的烦恼", 35),
        PerformancesData("新宿事件", 48)
    )
)

最后，我们剧团要打印出这个客户的账单，代码如下：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    var totalAmount = 0
    var volumeCredits = 0
    var result = "这里您的账单: ${invoice.customer}

    for (perf in invoice.performances) {
        val playInfo = plays[perf.playId] ?: continue
        var thisAmount = 0

        when (playInfo.type) {
            "tragedy" -> {
                thisAmount = 40000
                if (perf.audience > 30) {
                    thisAmount += 1000 * (perf.audience - 30)
                }
            }
            "comedy" -> {
                thisAmount = 30000
                if (perf.audience > 20) {
                    thisAmount += 10000 + 500 * (perf.audience - 20)
                }
                thisAmount += 300 * perf.audience
            }
        }
        // 加积分
        volumeCredits += (perf.audience - 30).coerceAtLeast(0)
        if (playInfo.type == "comedy") volumeCredits += floor(perf.audience / 5f).toInt()
        // 打印当前戏剧的演出费用
        result += " ${playInfo.name}: ${thisAmount / 100f} (${perf.audience}
        totalAmount += thisAmount
    }
    result += "所欠金额：￥${totalAmount / 100f}\n"
    result += "你获取了 $volumeCredits
    return result
}

打印如下：

这里您的账单: 中船戏院 
  西红市首富: 740.0 (55 观众) 
  夏洛的烦恼: 580.0 (35 观众) 
  新宿事件: 580.0 (48 观众) 
所欠金额：￥1900.0
你获取了 66

1.2 函数评价

这个程序的设计怎么样呢？我自己认为是能看得懂，但是也有一些可以改进的地方，比如归纳其计算策略算法。

代码组织不清晰，但这也在可以接受的范围内。这样小的程序，不做任何深入的设计，也不会太难理解，但这是因为这个例子非常小，假如把这段代码有上百行，那么把这些代码放到一个函数里面就很难理解的。

差劲的系统是很难修改的，因为难以找到修改点，难以了解做出的修改与现有代码如何协作实现我想要的行为。如果很难找到修改点，就很有可能犯错而引入Bug。

所以，如果我们需要修改一个上百行的程序，我们会希望其有良好的结构，即被分解成一系列函数和其他程序要素。这能帮我们更易于清楚了解这段代码在做什么。

如果你要给程序添加特性，但发现代码因缺乏良好的结构而不易于进行更改，那就先重构那个程序，使其容易添加该特性，然后再添加该特性

上面这段代码无法应对需求的变化，例如：

输出希望以表单的形式，而不是以 String 的形式，这会导致 result 变量要加很多分支逻辑
或许还有更多类型的戏剧，那计费方式、积分计算都会被影响

需求的变化使得重构变得必要，如果一段代码能正常工作且不需要被修改，那么它就完全没有重构的必要，但是如果有人想要理解其工作原理，那最好还是改进一下代码

1.3 重构第一步

当需要进行重构的时候， 第一步永远是：保证被修改的代码有一组可靠的测试用例，这能帮助我们在修改代码时不会引入Bug。放到本人的开发来说，就是有一套单元测试来测试被修改类。

写测试系统的好处：

写单元测试其实就是对代码进行二次检查，降低犯错概率
虽然写单元测试需要花费时间，但其实却节省了后期debug调试时间

statement 函数返回的就是一个字符串，重构前后，我们可以通过输入几个 invoices、plays 来借助测试框架检查输出，即期望输出和实际输出，来判断我们的修改是否会产生问题。

1.4 分解 Statement 函数

每当看到这样长长的函数，就要下意识的从整个函数中分离出不同的关注点。

1.4.1 分离 when 语句

首先，先着眼的是中间那段 when 的语句：

....
        when (playInfo.type) {
            "tragedy" -> {
                thisAmount = 40000
                if (perf.audience > 30) {
                    thisAmount += 1000 * (perf.audience - 30)
                }
            }
            "comedy" -> {
                thisAmount = 30000
                if (perf.audience > 20) {
                    thisAmount += 10000 + 500 * (perf.audience - 20)
                }
                thisAmount += 300 * perf.audience
            }
        }
        ...

这段代码的作用是用来计算每种类型戏剧的演出费。当然如果没有细看代码的话，我们并不会清楚每个类型的戏剧是如何计算的，因为它里面做了一些运算。

为了理解这段代码，我们得先把它抽成一个独立的函数，按照其作用给这个函数命名，比如命名为 amountFor()。
注：每次想要将一块代码抽取成一个函数时，都会遵循一个标准流程，最大程度减少犯错的可能，所以我们需要把这个流程记录了下来，并命名为提炼函数（001），以便日后可以方便的引用

首先需要检查一下，如果提炼到一个独立的函数里，有哪些变量会离开本函数作用域。
在这个示例中，是 playInfo、perf、thisAmout 这个三个变量，前两个变量会被提炼后的函数使用，但是不会被修改，所以独立函数可以将他们以参数的方式传递进来，我们更关系那些会被修改的变量，这里的就是 thisAmout，因此可以将它从函数中直接返回，我们还可以将其初始化放到提炼后的函数中去，代码如下：

fun amountFor(perf: PerformancesData, play: PlayInfo): Int {
    var thisAmount = 0
    when (play.type) {
        "tragedy" -> {
            thisAmount = 40000
            if (perf.audience > 30) {
                thisAmount += 1000 * (perf.audience - 30)
            }
        }
        "comedy" -> {
            thisAmount = 30000
            if (perf.audience > 20) {
                thisAmount += 10000 + 500 * (perf.audience - 20)
            }
            thisAmount += 300 * perf.audience
        }
        else -> throw Exception("unknown type:${play.type}")
    }
    return thisAmount
}

现在 statement() 就可以调用这个函数了：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    var totalAmount = 0
    var volumeCredits = 0
    var result = "这里您的账单: ${invoice.customer}

    for (perf in invoice.performances) {
        val playInfo = plays[perf.playId] ?: continue
        // 这里使用提炼后的函数
        val thisAmount = amountFor(perf, playInfo)
        
        volumeCredits += (perf.audience - 30).coerceAtLeast(0)
        if (playInfo.type == "comedy") volumeCredits += floor(perf.audience / 5f).toInt()
        result += " ${playInfo.name}: ${thisAmount / 100f} (${perf.audience}
        totalAmount += thisAmount
    }
    result += "所需缴费：￥${totalAmount / 100f}\n"
    result += "你获取了 $volumeCredits
    return result
}

在书中，每当进行了类似于这样的修改，需要重新编译并执行测试，保证修改不会出错，然后提交 commit，最后push时，将这些零散的commit通过合成一个有意义的commit再push到远程仓库上。

1.4.2 命名讲究

这里书中将提炼函数 amountFor() 的 thisAmount 命名为 result，永远将返回值命名为 result，这样就能一眼知道这个函数的作用。

1.4.3 移除 playInfo 变量

我们继续观察这个 amountFor() ，我们看看它的参数都从哪里来， perf 是从循环变量中来的，所以自然每次循环都会改变，但 playInfo 变量是由 PerformancesData 计算得到的，因此没有必要将它作为参数传入，我可以在 amountFor() 函数中重新计算得到它。 当我们分解一个长函数时，需要将 play 这样的变量移除掉，因为它们创建了很多具有局部作用域的临时变量，这会导致提炼函数更加复杂。这里我们要使用重构手法来取代临时变量。

就是将复制表达式的右边提炼出一个函数：

fun playFor(perf: PerformancesData, plays: MutableMap<String, PlayInfo>): PlayInfo = plays[perf.playId]?: PlayInfo()

然后通过内敛函数的形式在顶层作用域调用：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    ...
    for (perf in invoice.performances) {
        val thisAmount = amountFor(perf, playFor(perf, plays))  // 1
        volumeCredits += (perf.audience - 30).coerceAtLeast(0)
        if (playFor(perf, plays).type == "comedy") volumeCredits += floor(perf.audience / 5f).toInt()  // 2
        result += " ${playFor(perf, plays).name}: ${thisAmount / 100f} (${perf.audience}  // 3
        totalAmount += thisAmount
    }
    ...
}

看到这里，可能有些同学会感到疑问：重构之前，获取 play变量的代码，每次循环只执行了一次，但是重构之后，却执行了三次，这不是看起来变得更加麻烦了吗？？

我们会在后面探讨重构与性能之间的关系，但是现在，我认为这次改动还不太可能对性能有严重影响，即使真的有所影响，后续再对一段i建二狗良好的代码继续进行性能调优，也容易得多。

接下来我们继续移除掉 amountFor() 的 play 变量，使其：

fun amountFor(perf: PerformancesData, plays: MutableMap<String, PlayInfo>): Int {
    var result: Int
    when (playFor(perf, plays).type) {
        ...
        else -> throw Exception("unknown type:${playFor(perf, plays).type}")
    }
    return result
}

顶层调用：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    ...
    for (perf in invoice.performances) {
        val thisAmount = amountFor(perf, plays)
        volumeCredits += (perf.audience - 30).coerceAtLeast(0)
        if (playFor(perf, plays).type == "comedy") volumeCredits += floor(perf.audience / 5f).toInt()
        result += " ${playFor(perf, plays).name}: ${thisAmount / 100f} (${perf.audience}
        totalAmount += thisAmount
    }
    ...
}

移除局部变量的好处就是做提炼时会简单得多，因为需要操心的局部作用域减少了。
实际上，在做任何提炼时，一般都会先去移除局部变量。

之后我们看下顶层作用域的 amountFor 计算出来的 thisAmount，它之后不会再被修改了，所以我们又可以采用内敛的手法内敛它：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    ...
    for (perf in invoice.performances) {
        volumeCredits += (perf.audience - 30).coerceAtLeast(0)
        if (playFor(perf, plays).type == "comedy") volumeCredits += floor(perf.audience / 5f).toInt()
        result += " ${playFor(perf, plays).name}: ${amountFor(perf, plays) / 100f} (${perf.audience}
        totalAmount += amountFor(perf, plays)
    }
    ...
}

1.4.4 提炼计算观众量积分逻辑

我们移除了 play 变量后，这使得我们提炼观众积分的计算逻辑又简单了一些。

我们仍需要处理其他两个局部变量， perf 同样可以做为参数传入，但 volumeCredits 变量则有些棘手，它是一个累加变量，每次循环迭代都会更新它的值。所以最简单的方式，就是将整块逻辑提炼到新函数中，然后在新函数中直接返回 volumeCredits，我们命名为 volumeCreditsFor，如下：

fun volumeCreditsFor(perf: PerformancesData, plays: MutableMap<String, PlayInfo>): Int {
    var volumeCredits = 0
    volumeCredits += (perf.audience - 30).coerceAtLeast(0)
    if (playFor(perf, plays).type == "comedy") volumeCredits += floor(perf.audience / 5f).toInt()
    return volumeCredits
}

提炼使用后，顶层作用域就是这样的：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    ...
    for (perf in invoice.performances) {
        volumeCredits += volumeCreditsFor(perf, plays)
        result += " ${playFor(perf, plays).name}: ${amountFor(perf, plays) / 100f} (${perf.audience}
        totalAmount += amountFor(perf, plays)
    }
    ...
}

之后，我们将 volumeCreditsFor 中重新命名 volumeCredits => result 就可以了

1.4.5 移除观众量积分总和

临时变量往往会带来麻烦，它们指在对其进行处理的代码块中有用，因此临时变量实质上是鼓励你写长而复杂的函数。因此下一步需要替换掉一些临时变量，从 volumeCredits 变量入手，这个变量比较微妙，因为它是在迭代过程中累加得到的，第一步就是拆分循环，将 volumeCredits 的累加过程分离出来：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    var totalAmount = 0
    var result = "这里您的账单: ${invoice.customer}
    for (perf in invoice.performances) {
        result += " ${playFor(perf, plays).name}: ${amountFor(perf, plays) / 100f} (${perf.audience}
        totalAmount += amountFor(perf, plays)
    }

    // 拆出这个循环
    var volumeCredits = 0
    for (perf in invoice.performances) {
        volumeCredits += volumeCreditsFor(perf, plays)
    }

    result += "所需缴费：￥${totalAmount / 100f}\n"
    result += "你获取了 $volumeCredits
    return result
}

然后抽出循环后如下所示：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    var totalAmount = 0
    var result = "这里您的账单: ${invoice.customer}
    for (perf in invoice.performances) {
        result += " ${playFor(perf, plays).name}: ${amountFor(perf, plays) / 100f} (${perf.audience}
        totalAmount += amountFor(perf, plays)
    }

    result += "所需缴费：￥${totalAmount / 100f}\n"
    result += "你获取了 ${totalVolumeCredits(invoice, plays)}
    return result
}

fun totalVolumeCredits(invoice: Invoices, plays: MutableMap<String, PlayInfo>) {
    var volumeCredits = 0
    for (perf in invoice.performances) {
        volumeCredits += volumeCreditsFor(perf, plays)
    }
}

到这里，我们先斯多普一下。我们在这里修改了循环，肯定会有人警惕（包括我），因为我们对循环重复了一次。
大多数时，重复一下这样的循环性能的影响都可忽略不计，如果你在重构前后进行计时，很可能甚至都注意不到运行速度的变化—通常也确实没有什么变化。
许多程序员对代码实际运行路径都所知不足，在聪明的编译器、现代的缓存技术面前，我们很多直觉都是不准确的，软件的性能通常只与代码的一小部分有关，改变其他的部分往往对总体性能贡献甚为。

当然，不能以偏概全，一些重构手法也会影响到性能，但即使如此，我们通常也不需要去管它，二十继续重构，因为有了一份结构良好的代码，回头调优其性能也容易得多，如果我们在重构时引入了明显的性能损耗，后面再花时间进行性能调优。

OK，继续往下，用同样的步骤来移除 totalAmount，得到如下代码：

fun statement(invoice: Invoices, plays: MutableMap<String, PlayInfo>): String {
    var result = "这里您的账单: ${invoice.customer}
    for (perf in invoice.performances) {
        result += " ${playFor(perf, plays).name}: ${amountFor(perf, plays) / 100f} (${perf.audience}
    }
    ...
}

fun totalAmount(invoice: Invoices, plays: MutableMap<String, PlayInfo>): Int {
    var result = 0
    for (perf in invoice.performances) {
        result += amountFor(perf, plays)
    }
    return result
}

1.4.6 里程碑

接下来将它写进一个类里面，并且将修改一些函数的名称，将入参设置为全局变量，现在来看下我们重构的代码：

class Statement(private val invoice: Invoices, private val plays: MutableMap<String, PlayInfo>) {

    fun getStatement(): String {
        var result = "这里您的账单: ${invoice.customer}
        for (perf in invoice.performances) {
            result += " ${playFor(perf).name}: ${amountFor(perf) / 100f} (${perf.audience}
        }

        result += "所需缴费：￥${totalAmount() / 100f}\n"
        result += "你获取了 ${totalVolumeCredits()}
        return result
    }

    private fun totalAmount(): Int {
        var result = 0
        for (perf in invoice.performances) {
            result += amountFor(perf)
        }
        return result
    }

    private fun totalVolumeCredits(): Int {
        var result = 0
        for (perf in invoice.performances) {
            result += volumeCreditsFor(perf)
        }
        return result
    }

    private fun volumeCreditsFor(perf: PerformancesData): Int {
        var result = 0
        result += (perf.audience - 30).coerceAtLeast(0)
        if (playFor(perf).type == "comedy") result += floor(perf.audience / 5f).toInt()
        return result
    }

    private fun amountFor(perf: PerformancesData): Int {
        var result: Int
        when (playFor(perf).type) {
            "tragedy" -> {
                result = 40000
                if (perf.audience > 30) {
                    result += 1000 * (perf.audience - 30)
                }
            }
            "comedy" -> {
                result = 30000
                if (perf.audience > 20) {
                    result += 10000 + 500 * (perf.audience - 20)
                }
                result += 300 * perf.audience
            }
            else -> throw Exception("unknown type:${playFor(perf).type}")
        }
        return result
    }

    private fun playFor(perf: PerformancesData): PlayInfo =
        plays[perf.playId] ?: PlayInfo("", "")
}