上一篇使用了Extract Method技巧,从Statement方法中分离出了AmountFor方法,从而略微提高了Statement方法的可读性,下面我们继续跟着《重构》的脚步来进行剩余的重构。当然在继续之前,我还是会提醒你,每一步重构之后,记得运行单元测试,它是重构的基石。
一、重命名AmountFor的局部变量
首先看看我们先前抽取出来的AmountFor方法:
public double AmountFor(Rental rental)
{
double thisAmount = 0;
switch (rental.Movie.PriceCode)
{
case Movie.REGULAR:
thisAmount += 2;
if (rental.DaysRented > 2)
thisAmount += (rental.DaysRented - 2) * 1.5;
break;
case Movie.NEW_RELEASE:
thisAmount += rental.DaysRented * 3;
break;
case Movie.CHILDRENS:
thisAmount += 1.5;
if (rental.DaysRented > 3)
thisAmount += (rental.DaysRented - 3) * 1.5;
break;
}
return thisAmount;
}
方法中有个局部变量:thisAmount,实际上,该名称是在Statement方法中起的。而对于AmountFor方法而言,只可能有一个amount,对应传参进来的rental。所以,加上this的修饰反而是画蛇添足的做法。而我也不太认同《重构》中对该局部变量的新命名(result),result一词过于宽泛,也有许多其它数据提到过这个问题,例如避免使用如result,item,manager,这一类“万能”词汇。所以,改进后的代码如下:
private double AmountFor(Rental rental)
{
double amount = 0;
switch (rental.Movie.PriceCode)
{
case Movie.REGULAR:
amount += 2;
if (rental.DaysRented > 2)
amount += (rental.DaysRented - 2) * 1.5;
break;
case Movie.NEW_RELEASE:
amount += rental.DaysRented * 3;
break;
case Movie.CHILDRENS:
amount += 1.5;
if (rental.DaysRented > 3)
amount += (rental.DaysRented - 3) * 1.5;
break;
}
return amount;
}
如书上所言,不要忽视这种微小的重构,它的确能降低代码的理解难度。试想,如果一个朋友本想和你说“圣诞快乐”,却说成了“新年幸福”,这也许在西方国家并不存在太大问题,但是中国人却是难以将两件事情联系在一起的。所以,让你的命名确切地表示它的意思,这很重要。在我的编码工作中,甚至可能超过15%的时间是花在为类、方法、字段等内容的命名上,浪费吗?一点都不!
二、将AmountFor方法转移到Rental类
《重构》:目前的AmountFor没有使用任何Customer类的属性、字段、方法,它在不在Customer方法中其实并不是那么重要,而这正是暗示该方法不应该属于该类型的明确信号,Rental类才是它的归属地。下面是重构的过程:
1.在Rental类中创建一个Charge属性,并将AmountFor中的代码抄过去:
public double Charge
{
get
{
double amount = 0;
switch (Movie.PriceCode)
{
case Movie.REGULAR:
amount += 2;
if (DaysRented > 2)
amount += (DaysRented - 2) * 1.5;
break;
case Movie.NEW_RELEASE:
amount += DaysRented * 3;
break;
case Movie.CHILDRENS:
amount += 1.5;
if (DaysRented > 3)
amount += (DaysRented - 3) * 1.5;
break;
}
return amount;
}
}
2.把原Customer的AmountFor方法体清空,但仍保留声明和空的函数体。
private double AmountFor(Rental rental)
{
return 0;
}
3.更改Customer的AmountFor实现为:
private double AmountFor(Rental rental)
{
return rental.Charge;
}
4.运行测试项目,如果不通过,则检查、修改代码。直到通过为止
5.修改Statement方法,使其不调用自身类的AmountFor方法来取得Rental的Amount,而是直接问Rental要:
public string Statement()
{
double totalAmount = 0;
int frequentRenterPoints = 0;
string result = "Rental Record for " + Name + "\n";
foreach (Rental rental in Rentals)
{
double thisAmount = rental.Charge;
// add frequent renter points
frequentRenterPoints++;
// add bonus for a two day new release rental
if (rental.Movie.PriceCode == Movie.NEW_RELEASE &&
rental.DaysRented > 1) frequentRenterPoints++;
// show figures for this rental
result += "\t" + rental.Movie.Title + "\t" + thisAmount.ToString() + "\n";
totalAmount += thisAmount;
}
// add footer lines
result += "Amount owed is " + totalAmount.ToString() + "\n";
result += "You earned " + frequentRenterPoints.ToString() + " frequent renter points";
return result;
}
6.运行测试,如果不通过则检查、修改代码,直到通过为止。
7.重构完成
需要注意的是,将AmountFor方法的内容迁移到Rental中时,并没有沿用原来的名字,而是采用了Charge这个名称,主要原因是对于Customer来说,该数值是Rental的Amount,而对于Rental自己来说,用Charge来称呼它更贴切。这是一个描述角度的问题,也是很多程序员容易忽略的事情。
三、去除thisAmount局部变量
哪怕只是一个稍微复杂的算法,通常都包含了许多运算中间量/辅助量。当整个运算过程都堆在一个方法中时,这些繁杂的变量就交织在一起。如果少一个中间量该算法可以正常运行,为什么不把它去掉呢?许多程序员、数学家都有这种追求极致的癖好不是么?实际上也不能说是癖好,应该说少一个变量,意味着该算法更简单,而越简单的算法能为这些人带来越多的成就感,甚至是荣誉感。抛开精神上的收益不谈,少一个变量确实会让该方法更容易维护。对于Statement方法而言,每一个循环中,thisAmount局部变量只被赋值了一次,而使用了两次,该初始值为rental.Charge。所以,自然地,所有使用到thisAmount的地方换成rental.Charge属性的调用应该是等价的:
public string Statement()
{
double totalAmount = 0;
int frequentRenterPoints = 0;
string result = "Rental Record for " + Name + "\n";
foreach (Rental rental in Rentals)
{
// add frequent renter points
frequentRenterPoints++;
// add bonus for a two day new release rental
if (rental.Movie.PriceCode == Movie.NEW_RELEASE &&
rental.DaysRented > 1) frequentRenterPoints++;
// show figures for this rental
result += "\t" + rental.Movie.Title + "\t" + rental.Charge.ToString() + "\n";
totalAmount += rental.Charge;
}
// add footer lines
result += "Amount owed is " + totalAmount.ToString() + "\n";
result += "You earned " + frequentRenterPoints.ToString() + " frequent renter points";
return result;
}
抛开书上提到可能存在的效率问题,我更愿意从语义的角度去理解这种重构。上面总共使用了rental.Charge两次,可以分别描述为:
1.给我一个rental对象的Charge值的文本,我要将它和影片的标题连成一个行表示该影片花费的描述文本。
2.被我一个rental对象的Charge值,我要把它累加到总花费中。
两次描述中,都直接使用了“rental对象的Charge值”这样的概念,而不是“给我一个变量,该变量保存了一个rental对象的Charge值”,后面这种是程序员的典型思维方式。它无助于理解算法的本质,而是更容易使查看代码的人陷入算法的实现细节。所以,当你需要用一个概念时,直接用它,不要绕弯子。当然,真正的问题在于你能很好地总结出这种概念——重构就有这样的效果。
注:别忘了在重构后运行单元测试
四、将FrequentRenterPoint的计算抽取、迁移到Rental类中
在精简后的Statement方法中,可以清楚地看到,它还包含了求每一个Rental对象的FrequentRenterPoint的计算。这和求Rental的Charge几乎是一致的。所以,可以使用同样的方法来处理它:
1.在Rental类型中,创建新的属性FrequentRenterPoints,并将Statement方法中的计算逻辑抄到该属性中,注意要去掉rental对象的引用,因为它现在就属于Rental类:
public int FrequentRenterPoints
{
get
{
int frequentRenterPoints = 0;
// add frequent renter points
frequentRenterPoints++;
// add bonus for a two day new release rental
if (Movie.PriceCode == Movie.NEW_RELEASE &&
DaysRented > 1) frequentRenterPoints++;
return frequentRenterPoints;
}
}
2.运行单元测试,如果不通过则检查、调试、修改,直到通过为止
3.该属性的实现非常“笨”,实际想表达的就是如果符合条件则得2分,否则得1分,所以可重构为:
public int FrequentRenterPoints
{
get
{
if (Movie.PriceCode == Movie.NEW_RELEASE && DaysRented > 1)
{
return 2;
}
else
{
return 1;
}
}
}
可以使用更简单的三木操作符 :? 来替代if-else,但通常后者更容易被理解。
4.运行单元测试,如果不通过则检查、调试、修改,直到通过为止
5.替换原Statement中关于frequentRenterPoints的计算,使用Rental的FrequentRentalPoints属性来进行计算:
public string Statement()
{
double totalAmount = 0;
int frequentRenterPoints = 0;
string result = "Rental Record for " + Name + "\n";
foreach (Rental rental in Rentals)
{
frequentRenterPoints += rental.FrequentRenterPoints;
// show figures for this rental
result += "\t" + rental.Movie.Title + "\t" + rental.Charge.ToString() + "\n";
totalAmount += rental.Charge;
}
// add footer lines
result += "Amount owed is " + totalAmount.ToString() + "\n";
result += "You earned " + frequentRenterPoints.ToString() + " frequent renter points";
return result;
}
6.运行单元测试,如果不通过则检查、调试、修改,直到通过为止
7.重构完成
五、抽取花费总值的计算以及总积分的计算
现在已经较容易看出,Statement方法中,除了报告文本的生成外,主要还做了两样事情:计算总花费,计算总积分,而现在它们是混合在一起的。而我们的确可以把它们抽取为两个独立的查询过程(对C#而言可以是两个属性),分别命名为TotalCharge、TotalRentalPoints:
1.在Customer类中添加一个属性TotalCharge如下:
private double TotalCharge
{
get
{
double sum = 0;
foreach (Rental aRental in Rentals)
{
sum += aRental.Charge;
}
return sum;
}
}
2.将Statement方法中所有关于总花费的局部变量、过程都去掉,换成对TotalCharge的调用:
public string Statement()
{
int frequentRenterPoints = 0;
string result = "Rental Record for " + Name + "\n";
foreach (Rental rental in Rentals)
{
frequentRenterPoints += rental.FrequentRenterPoints;
// show figures for this rental
result += "\t" + rental.Movie.Title + "\t" + rental.Charge.ToString() + "\n";
}
// add footer lines
result += "Amount owed is " + TotalCharge.ToString() + "\n";
result += "You earned " + frequentRenterPoints.ToString() + " frequent renter points";
return result;
}
3.运行单元测试,如果不通过则检查、调试、修改,直到通过为止
4.在Customer类中添加TotalRenterPoints属性如下:
private int TotalRenterPoints
{
get
{
int points = 0;
foreach (Rental aRental in Rentals)
{
points += aRental.FrequentRenterPoints;
}
return points;
}
}
5.将Statement方法中所有关于总积分的局部变量、过程都去掉,换成对TotalRenterPoints的调用:
public string Statement()
{
string result = "Rental Record for " + Name + "\n";
foreach (Rental rental in Rentals)
{
// show figures for this rental
result += "\t" + rental.Movie.Title + "\t" + rental.Charge.ToString() + "\n";
}
// add footer lines
result += "Amount owed is " + TotalCharge.ToString() + "\n";
result += "You earned " + TotalRenterPoints.ToString() + " frequent renter points";
return result;
}
6.运行单元测试,如果不通过则检查、调试、修改,直到通过为止
7.重构完成
至此,我们重新审视一下Statement方法,比起最初的版本而言,哪一个更容易看出它所包含的逻辑?最起码我扫一眼后面这个版本就知道它的工作如下:
总的来说,它生成了一份报告文本;这个报告用用户名生成了一报告头;为每一个Rental生成了一份标题+花费的记录;报告的Footer包含两样内容,分别是该客户的总花费,以及他能得到的总积分是多少。
至于很多人会质疑,抽取的TotalCharge和TotalRenterPoints会带来两次额外的Rentals遍历,以至于存在效率降低的的问题。还是参阅《重构》书中的内容吧。