一个重构的例子

一个重构的例子

在看《程序员的呐喊》里面看到了关于《重构》这本书的一些描述

我注意到《重构》这本书是在2002年的时候，距离它出版已经好多年了。之前一直没读是因为出版它的是那帮搞UML的蠢货。我从来都不是他们的粉丝。
…
《重构》偏偏就和这样的一堆烂书挤在一起，每次看到它我都直接扫过，从不停留一秒。别在烂书上浪费时间.直到2002年冬， 我在一家书店拿起它。没有什么特别的原因，纯碎是出于好奇而已。或许我在其他什么地方听到过“重构”这个词吧。
…
接着一股恐惧袭上心头：他居然说的没错，有理有据。我最自豪的编程习惯——把中间值保存在局部变量里，作为简单的性能优化——显然是个坏习惯。中明明白白展示了这一点。它解释了我代码里某些方法为什么不断膨胀，那就是因为这些方法无法分割，这一点我之前我从未想到过。
…
这本书接着告诉我，不要写注释。又是疯话！可他说的确有道理。
…
我掏钱买它回家，反复研读。彻底震惊，太天才了。时至今日我也依然这么觉得，虽然那种震撼没有当天那么巨大。但这本书是在乃惊世之作，好似醍醐灌顶。这种事情可不常见。

看到这样的描述都没有办法不去把它买回来好好研读一番，而且一开始对这本书的感受也跟他描述的一样，看见过很多次，但总感觉是本烂书，都不想翻开。今天是新年第一天，正好适合醍醐灌顶，于是从第一个例子开始，仔细研究，这里也用Swift来重写这个例子。原书中用的Java，所以会在一些特性上有所不同，而且我对Swift的理解也很有限，还没有完整的用Swift开发过一个产品，所以难免会有用的不妥当的特性。

源代码地址在此

起点

例子非常简单，是一个影碟出租公司的例子。从这个例子就可以看得出来，这本书的历史悠久了，现在哪还有出租影碟的？

整个程序就一个功能，根据顾客租了哪些影片、租期多长，算出费用和会员的积分。影片有三种类型，普通片、儿童片和新片，而费用和积分的计算规则是因类型而异的。
简单的UML类图如下：

代码如下：

Movie

//Movie
enum MovieType {
    case Children
    case Regular
    case NewRelease
}

class Movie: NSObject {

    let title: String
    let priceCode: MovieType

    init(title: String, priceCode: MovieType) {
        self.title = title
        self.priceCode = priceCode
    }

    override var description: String {
        return "Movie<title \(title), proce code: \(priceCode)>"
    }
}

Rental

//Rental
class Rental: NSObject {

    let movie: Movie
    let daysRented: Int

    init(movie: Movie, daysRented: Int) {
        self.movie = movie
        self.daysRented = daysRented
    }

    override var description: String {
        return "Rental<movie: \(movie.description) rent days: \(daysRented)>"
    }
}

Customer

 //Customer
 class Customer: NSObject {

    let name: String
    var rentals: [Rental] = [Rental]()

    init(name: String) {
        self.name = name
    }

    func addRental(arg: Rental) {
        rentals.append(arg)
    }

    override var description: String {

        get {
            var des: String = "Cutomer name is \(name):"
            for rental in rentals {
                des += "\n"
                des += rental.description
            }

            return des;
        }
    }

    func statement() -> String {

        var totalAmount: Double = 0
        var frequentRenterPoints = 0

        var result: String = "Rental Record for \(name) \n"

        for rental in rentals {

            var thisAmount: Double = 0

            switch (rental.movie.priceCode) {
            case .Regular:
                thisAmount += 2
                if (rental.daysRented > 2) {
                    thisAmount += (Double)(rental.daysRented - 2) * 1.5
                }
            case .NewRelease:
                thisAmount += (Double)(rental.daysRented) * 3.0
            case .Children :
                thisAmount += 1.5
                if (rental.daysRented > 3) {
                    thisAmount += (Double)(rental.daysRented - 3) * 1.5
                }
            }

            frequentRenterPoints++

            if (rental.movie.priceCode == .NewRelease && rental.daysRented > 1) {
                frequentRenterPoints++
            }

            result += "\t \(rental.movie.title) \t \(thisAmount) \n"
            totalAmount += thisAmount
        }
        result += "Amount owned is \(totalAmount) \n"
        result += "You earned \(frequentRenterPoints) frequent renter points"
        return result
    }
}

该阶段的代码在此
你对这个程序的印象如何？反正我是没有耐心看完statement()这个方法的，它做的事情太多了。如果现在我们需要增加一个方法HTMLStatement()，那么我们只能编写一个全新的方法来大量重复statement()里面的代码。然后如果计费的方法或者积分的规则发生了变化，你就又不得不去修改两个地方。
所以现在很简单的规则下，这段代码是可以正确输出的，但是随着规则的变化，代码会越来越难以适应新的规则，维护会越来越困难。

重构的第一步

所有重构的第一步都是一样的——为即将修改的代码简建立一组可靠的测试环境。好的测试是重构的根本，花时间去建立优良的测试机制是完全值得的。后面会专门的讨论关于测试。

分解并重组statement()

代码提取

代码最严重的问题在于长的离谱的statement()。要明确的是，代码块越小，代码的功能就越容易管理，代码的处理和移动也越轻松。
这一节的目的是将长函数切开，把较小的块移到合适的类中。
第一步是找出代码里的逻辑泥团，然后运用Extract Method规则

你有一段代码可以组织在一起，并且独立出来

这里我们首先发现的一个泥团就是switch语句，应该把它提到单独的函数中。首先找出这段代码中的函数内的局部变量和参数。这段代码里有两个变量，rental和thisAmount，前者并未被修改，而后者会被修改。任何不会被修改的变量都可以被当成参数传入新的函数。而回被修改的变量可以被当做是返回值处理，thisAmount是个临时变量，其值会在每次循环起始处被设为0，并且在switch之前不会改变，所以可以直接把新函数的返回值赋值给它。

下面就是提取出来的新函数：

//Customer
func amountForEach(aRental: Rental) -> Double {

        var result: Double = 0

        switch (aRental.movie.priceCode) {
        case .Regular:
            result += 2
            if (aRental.daysRented > 2) {
                result += (Double)(aRental.daysRented - 2) * 1.5
            }
        case .NewRelease:
            result += (Double)(aRental.daysRented) * 3.0
        case .Children :
            result += 1.5
            if (aRental.daysRented > 3) {
                result += (Double)(aRental.daysRented - 3) * 1.5
            }
        }
        return result
    }

而对应的在statement()中的使用部分是：

//Customer
for rental in rentals {
    let thisAmount: Double = amountForEach(rental)

    frequentRenterPoints++
    if (rental.movie.priceCode == .NewRelease && rental.daysRented > 1) {
        frequentRenterPoints++
    }

    result += "\t \(rental.movie.title) \t \(thisAmount) \n"
    totalAmount += thisAmount
}

这个改动是很小，但是我们也单独拿出来说了，原因就是重构技术就是以微小的步伐去改变程序。如果犯了错，很容易就可发现。
我们可以重新编译程序，并且测试，看我们的重构过程是否影响了程序的正确性。

搬移函数

观察一下刚刚抽取的函数amountFor()，发现这个函数中使用了Rental类中的信息，但是却并没有使用来自Customer类的信息。这立刻让我怀疑它是不是放错了位置。绝大多数情况下，函数应该放在它所使用的数据的所属对象内，所以amountFor()应该被移到Rental中去。这里要运用Move Method

//Rental
var charge: Double {
    get {
        var result: Double = 0

        switch (movie.priceCode) {
        case .Regular:
            result += 2
            if (daysRented > 2) {
                result += (Double)(daysRented - 2) * 1.5
            }
        case .NewRelease:
            result += (Double)(daysRented) * 3.0
        case .Children :
            result += 1.5
            if (daysRented > 3) {
                result += (Double)(daysRented - 3) * 1.5
            }
        }
        return result
    }
}

然后对应的在Cutomer中的使用:

//Customer
func amountForEach(aRental: Rental) -> Double {

    return aRental.charge
}

然后查找使用了amountFor()的地方，这里很简单只有一处，替换它，最后删掉amountFor()：

//Customer
for rental in rentals {
            let thisAmount: Double = rental.charge

            frequentRenterPoints++
            if (rental.movie.priceCode == .NewRelease && rental.daysRented > 1) {
                frequentRenterPoints++
            }

            result += "\t \(rental.movie.title) \t \(thisAmount) \n"
            totalAmount += thisAmount
        }

然后，下一个发现的问题是thisAmount变的多余，仅仅是接受了rental.charge的结果，然后就不再变化了，所以运用Replace Temp with Query来去除thisAmount:

你的程序以一个临时变量保存某一个表达式的运算结果。将这个表达式提炼到一个独立的函数中。将这个临时变量的所有引用点替换为对新函数的调用。此后，新函数就可被其他函数调用。

替换后的代码：

//Customer
for rental in rentals {

            let thisAmount = rental.charge

            frequentRenterPoints++

            if (rental.movie.priceCode == Movie.MovieType.NewRelease && rental.daysRented > 1) {
                frequentRenterPoints++
            }

            result += "\t \(rental.movie.title) \t \(thisAmount) \n"
            totalAmount += thisAmount
        }

每次修改都保持程序的正确性，编译测试，保证没有破坏任何东西。
我比较喜欢尽量的去除临时变量，临时变量往往引发问题，它们会导致大量的参数被传来传去，而其实完全没有这种必要。当然这么做也需要付出性能的代价。例子中的费用就被计算了两次，但是这很容易在Rental类中被优化。而且如果代码有合理的组织和管理，优化就会有更好的效果。

提炼和搬移积分代码

下面我们针对积分的代码做类似的处理，首先提取函数，然后搬移到Rental中

//Rental
var frequentRenterPoints : Double {
    get {
        if movie.priceCode == .NewRelease && daysRented > 1 {
            return 2
        }
        return 1
    }
}

而对应的statement()也相应的被简化了：

//Customer
func statement() -> String {

    var totalAmount: Double = 0
    var frequentRenterPoints:Double = 0

    var result: String = "Rental Record for \(name) \n"

    for rental in rentals {

        frequentRenterPoints += rental.frequentRenterPoints
        result += "\t \(rental.movie.title) \t \(rental.charge) \n"
        totalAmount += rental.charge
    }
    result += "Amount owned is \(totalAmount) \n"
    result += "You earned \(frequentRenterPoints) frequent renter points"
    return result
}

去除临时变量

正如前面提到的，临时变量可能是个问题。这里还有两个临时变量:totalAmount和frequentRenterPoints，两者都是用来从Customer中的Rental对象中获取某个总量，不论是statement()还是HTMLStatemnet()都会使用到它们，所以继续利用Replace Temp with Query来替代这两个临时变量。

//Cunstomer
private var totalCharge: Double {
    get {
        var result: Double = 0
        for rental in rentals {
            result += rental.charge
        }
        return result
    }
}

private var totalFrequentRenterPoints: Double {
    get {
        var result: Double = 0
        for rental in rentals {
            result += rental.frequentRenterPoints
        }
        return result
    }
}

func statement() -> String {
    var result: String = "Rental Record for \(name) \n"

    for rental in rentals {

        result += "\t \(rental.movie.title) \t \(rental.charge) \n"
    }

    result += "Amount owned is \(totalCharge) \n"
    result += "You earned \(totalFrequentRenterPoints) frequent renter points"

    return result
}

到现在，我们终于可以戴上添加新功能的帽子，增加HTMLStatment()

//Customer
func HTMLStatement() -> String {
        var result: String = "<h1>Rentals for <em>\(name)</em></h1><p>\n"
        for rental in rentals {
            result += "\(rental.movie.title): \(rental.charge) <br>\n"
        }

        result += "<p>You owe <em>\(totalCharge) </em><p>\n"
        result += "On this rental you earned <em>\(totalFrequentRenterPoints)</em> frequent renter points<p>"

        return result
    }

该阶段的代码在此
到现在，我们的类图进化如下：

运用多态取代与价格相关的条件逻辑

这个问题的第一步是switch语句。最好不要在另外一个对象的属性基础上运用switch语句，如果不得不使用，也应该在自己的数据上使用，而不是别人的数据上用。这暗示着Rental中的charge应该被移到Movie中去。计算租金需要两部分数据，一是租期，这在Rental中，二是需要电影类型，在这Movie中。为什么我们选择把租期传给Movie而不是反过来呢？这是因为在本系统中可能的变化是新影片类型的加入，我们希望把这些变化封装在Movie中，而不是变化之后需要修改各个地方。所以：

//Movie

func getCharge(DaysRented daysRented: Int) -> Double{
    var result: Double = 0
    switch(priceCode) {
    case .Children:
        result += 1.5
        if (daysRented > 3) {
            result += (Double)(daysRented - 3) * 1.5
        }
    case .Regular:
        result += 2
        if (daysRented > 2) {
            result += (Double)(daysRented - 2) * 1.5;
        }
    case .NewRelease:
        result += Double(daysRented) * 3.0
    }
    return result
}

func getFrequentRenterPoints(DaysRented daysRented: Int) -> Double {
    if (priceCode == .NewRelease && daysRented > 1) {
        return 2
    }
    return 1
}

//Rental
var charge: Double {
    return movie.getCharge(DaysRented: daysRented)
}

var frequentRenterPoints : Double {
    return movie.getFrequentRenterPoints(DaysRented: daysRented)
}

终于，我们来到了继承

我们有多重影片类型，它们以不同的方式回答各自的问题。这听起来很像是子类的工作。假设我们我们建立3个Movie的子类，每个子类都有自己的计费方法，这么一来我们可以用多态来取代switch语句了。但是很遗憾的是，这里有个小问题，就是一部影片可以在生命周期内修改自己的分类方法，但是一个对象却不可以。所以这里的解决方法是：State模式：

首先要使用Replace Type Code with State/Strategy，第一步是针对类型代码使用Self Encapsulate Field来确保通过getter和setter来访问类型代码。也就是priceCode属性需要通过getter和setter来使用，这样的属性在Swift里叫做computed property，与之相对应的是stored property。在这里把priceCode作为了计算属性，而新增加一个存储属性是遵守’Price’协议的price属性：

//Movie
var priceCode: MovieType {

        get {
            return price.priceCode
        }

        set {
            switch (newValue) {
            case .Regular:
                price = RegularPrice()
            case .Children:
                price = ChildrenPrice()
            case .NewRelease:
                price = NewReleasePrice()
            }
        }
    }
    var price: Price!

    init(title: String, priceCode: MovieType) {
        self.title = title
        super.init()
        self.priceCode = priceCode
    }

新增加的Price协议如下：

//Price
protocol Price {
    var priceCode: MovieType { get }

    func getCharge(daysRented: Int) -> Double
}

遵守Price协议的三个类分别是RegularPrice, ChildrenPrice和NewReleasePrice。这里仅列出RegularPrice代码：

//RegularPrice
class RegularPrice: NSObject, Price {

    var priceCode: MovieType {
        get {
            return .Regular
        }
    }

    func getCharge(daysRented: Int) -> Double {
        var result: Double = 2
        if daysRented > 2 {
            result += Double(daysRented - 2) * 1.5
        }
        return result
    }
}

到这里，我们解决了计费的问题，同样的情况需要在积分上使用。积分和计费有一个不同是积分并不像计费的规则那样，每一种类型都是不同的，积分仅仅是新片有所不同，而其他的类型都是一样的，所以在书中在超类中去掉了abstract，在超类中定义一个函数，成为默认的行为，而只需要在NewReleasePrice重写即可，但是Swift并没有类似的概念。这里有两种解决办法，一是模拟Java中的虚函数的概念，基类用class而不是protocol,提供所有的默认方法，并且在必须在子类中实现的方法中直接抛出错误。二是，可以继续使用协议，利用协议的拓展来实现积分的计算，而不需要具体的类知道发生了什么。这里我们贴出后者的代码：

//Price
extension Price {

    func getFrequentRenterPoints(daysRented: Int) -> Double {

        if (priceCode == .NewRelease && daysRented > 1) {
            return 2
        }
        return 1
    }
}

该阶段的代码在此