本文共 21703 字,大约阅读时间需要 72 分钟。
本节书摘来自异步社区《重构:改善既有代码的设计》一书中的第1章,第1.3节分解并重组statement(),作者【美】Martin Fowler,更多章节内容可以访问云栖社区“异步社区”公众号查看。
1.3 分解并重组statement()
第一个明显引起我注意的就是长得离谱的statement()。每当看到这样长长的函数,我就想把它大卸八块。要知道,代码块越小,代码的功能就愈容易管理,代码的处理和移动也就越轻松。本章重构过程的第一阶段中,我将说明如何把长长的函数切开,并把较小块的代码移至更合适的类。我希望降低代码重复量,从而使新的(打印HTML详单用的)函数更容易撰写。
第一个步骤是找出代码的逻辑泥团并运用Extract Method (110)。本例一个明显的逻辑泥团就是switch语句,把它提炼到独立函数中似乎比较好。
和任何重构手法一样,当我提炼一个函数时,我必须知道可能出什么错。如果提炼得不好,就可能给程序引入bug。所以重构之前我需要先想出安全做法。由于先前我已经进行过数次这类重构,所以我已经把安全步骤记录于后面的重构列表中了。
首先我得在这段代码里找出函数内的局部变量和参数。我找到了两个,each和thisAmount,前者并未被修改,后者会被修改。任何不会被修改的变量都可以被我当成参数传入新的函数,至于会被修改的变量就需格外小心。如果只有一个变量会被修改,我可以把它当作返回值。thisAmount是个临时变量,其值在每次循环起始处被设为0,并且在switch语句之前不会改变,所以我可以直接把新函数的返回值赋给它。
下面两页展示了重构前后的代码。重构前的代码在左页,重构后的代码在右页。凡是从函数提炼出来的代码,以及新代码所做的任何修改,只要我觉得不是明显到可以一眼看出,就以粗体字标示出来特别提醒你。本章剩余部分将延续这种左右比对形式。
public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement(); //determine amounts for each line switch (each.getMovie().getPriceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.getDaysRented() > 2) thisAmount += (each.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.getDaysRented() * 3; break; case Movie.CHILDRENS: thisAmount += 1.5; if (each.getDaysRented() > 3) thisAmount += (each.getDaysRented() - 3) * 1.5; break; } // add frequent renter points frequentRenterPoints ++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints ++; //show figures for this rental result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(thisAmount) + "\n"; totalAmount += thisAmount; } //add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result;}public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement(); thisAmount = amountFor(each); // add frequent renter points frequentRenterPoints ++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints ++; //show figures for this rental result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(thisAmount) + "\n"; totalAmount += thisAmount; } //add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; }}private int amountFor(Rental each) { int thisAmount = 0; switch (each.getMovie().getPriceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.getDaysRented() > 2) thisAmount += (each.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.getDaysRented() * 3; break; case Movie.CHILDRENS: thisAmount += 1.5; if (each.getDaysRented() > 3) thisAmount += (each.getDaysRented() - 3) * 1.5; break; } return thisAmount;}
每次做完这样的修改,我都要编译并测试。这一次起头不算太好——测试失败了,有两条测试数据告诉我发生了错误。一阵迷惑之后,我明白了自己犯的错误。我愚蠢地将amountFor()的返回值类型声明为int,而不是double。
private double amountFor(Rental each) { double thisAmount = 0; switch (each.getMovie().getPriceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.getDaysRented() > 2) thisAmount += (each.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.getDaysRented() * 3; break; case Movie.CHILDRENS: thisAmount += 1.5; if (each.getDaysRented() > 3) thisAmount += (each.getDaysRented() - 3) * 1.5; break; } return thisAmount;}
我经常犯这种愚蠢可笑的错误,而这种错误往往很难发现。在这里,Java无怨无尤地把double类型转换为int类型,而且还愉快地做了取整动作[Java Spec]。还好此处这个问题很容易发现,因为我做的修改很小,而且我有很好的测试。借着这个意外疏忽,我要阐述重构步骤的本质:由于每次修改的幅度都很小,所以任何错误都很容易发现。你不必耗费大把时间调试,哪怕你和我一样粗心。
现在,我已经把原来的函数分为两块,可以分别处理它们。我不喜欢amountFor()内的某些变量名称,现在正是修改它们的时候。
下面是原本的代码:
private double amountFor(Rental each) { double thisAmount = 0; switch (each.getMovie().getPriceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.getDaysRented() > 2) thisAmount += (each.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.getDaysRented() * 3; break; case Movie.CHILDRENS: thisAmount += 1.5; if (each.getDaysRented() > 3) thisAmount += (each.getDaysRented() - 3) * 1.5; break; } return thisAmount;}
下面是改名后的代码:
private double amountFor(Rental aRental) { double result = 0; switch (aRental.getMovie().getPriceCode()) { case Movie.REGULAR: result += 2; if (aRental.getDaysRented() > 2) result += (aRental.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: result += aRental.getDaysRented() * 3; break; case Movie.CHILDRENS: result += 1.5; if (aRental.getDaysRented() > 3) result += (aRental.getDaysRented() - 3) * 1.5; break; } return result;}
改名之后,我需要重新编译并测试,确保没有破坏任何东西。
更改变量名称是值得的行为吗?绝对值得。好的代码应该清楚表达出自己的功能,变量名称是代码清晰的关键。如果为了提高代码的清晰度,需要修改某些东西的名字,那么就大胆去做吧。只要有良好的查找/替换工具,更改名称并不困难。语言所提供的强类型检查以及你自己的测试机制会指出任何你遗漏的东西。记住:
搬移“金额计算”代码
观察amountFor()时,我发现这个函数使用了来自Rental类的信息,却没有使用来自Customer类的信息。class Customer... private double amountFor(Rental aRental) { double result = 0; switch (aRental.getMovie().getPriceCode()) { case Movie.REGULAR: result += 2; if (aRental.getDaysRented() > 2) result += (aRental.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: result += aRental.getDaysRented() * 3; break; case Movie.CHILDRENS: result += 1.5; if (aRental.getDaysRented() > 3) result += (aRental.getDaysRented() - 3) * 1.5; break; } return result;}
这立刻使我怀疑它是否被放错了位置。绝大多数情况下,函数应该放在它所使用的数据的所属对象内,所以amountFor()应该移到Rental类去。为了这么做,我要运用Move Method (142)。首先把代码复制到Rental类,调整代码使之适应新家,然后重新编译。像下面这样:
class Rental... double getCharge() { double result = 0; switch (getMovie().getPriceCode()) { case Movie.REGULAR: result += 2; if (getDaysRented() > 2) result += (getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: result += getDaysRented() * 3; break; case Movie.CHILDRENS: result += 1.5; if (getDaysRented() > 3) result += (getDaysRented() - 3) * 1.5; break; } return result;}
在这个例子里,“适应新家”意味着要去掉参数。此外,我还要在搬移的同时变更函数名称。
现在我可以测试新函数是否正常工作。只要改变Customer.amountFor()函数内容,使它委托调用新函数即可:
class Customer... private double amountFor(Rental aRental) { return aRental.getCharge(); }
现在我可以编译并测试,看看有没有破坏什么东西。
下一个步骤是找出程序中对于旧函数的所有引用点,并修改它们,让它们改用新函数。下面是原本的程序:
class Customer... public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement(); thisAmount = amountFor(each); // add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints++; // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf(thisAmount) + "\n"; totalAmount += thisAmount; } // add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; }
本例之中,这个步骤很简单,因为我才刚刚产生新函数,只有一个地方使用了它。一般情况下,你得在可能运用该函数的所有类中查找一遍。
class Customer public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement(); thisAmount = each.getCharge(); // add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints++; // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf(thisAmount) + "\n"; totalAmount += thisAmount; } // add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; }
做完这些修改之后(图1-3),下一件事就是去掉旧函数。编译器会告诉我是否我漏掉了什么。然后我进行测试,看看有没有破坏什么东西。
当然我还想对Rental.getCharge()做些修改,不过暂时到此为止,让我们回到Customer.statement()函数。
public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement(); thisAmount = each.getCharge(); // add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints++; // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf(thisAmount) + "\n"; totalAmount += thisAmount; } // add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result;}
下一件引我注意的事是:thisAmount如今变得多余了。它接受each.getCharge()的执行结果,然后就不再有任何改变。所以我可以运用Replace Temp with Query (120)把thisAmount除去:
public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); // add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints++; // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf (each.getCharge()) + "\n"; totalAmount += each.getCharge(); } // add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; } }
做完这份修改,我立刻编译并测试,保证自己没有破坏任何东西。
我喜欢尽量除去这一类临时变量。临时变量往往引发问题,它们会导致大量参数被传来传去,而其实完全没有这种必要。你很容易跟丢它们,尤其在长长的函数之中更是如此。当然我这么做也需付出性能上的代价,例如本例的费用就被计算了两次。但是这很容易在Rental类中被优化。而且如果代码有合理的组织和管理,优化就会有很好的效果。我将在第69页的“重构与性能”一节详谈这个问题。
提炼“常客积分计算”代码
下一步要对“常客积分计算”做类似处理。积分的计算视影片种类而有不同,不过不像收费规则有那么多变化。看来似乎有理由把积分计算责任放在Rental类身上。首先需要针对“常客积分计算”这部分代码(粗体部分)运用Extract Method(110)重构手法:public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); // add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints++; // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "\n"; totalAmount += each.getCharge(); } // add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; }}
我们再来看局部变量。这里再一次用到了each,而它可以被当作参数传入新函数中。另一个临时变量是frequentRenterPoints。本例中,它在被使用之前已经先有初值,但提炼出来的函数并没有读取该值,所以我们不需要将它当作参数传进去,只需把新函数的返回值累加上去就行了。
我完成了函数的提炼,重新编译并测试,然后做一次搬移,再编译、再测试。重构时最好小步前进,如此一来犯错的几率最小。
class Customer... public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += each.getFrequentRenterPoints(); // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "\n"; totalAmount += each.getCharge(); } // add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; } class Rental... int getFrequentRenterPoints() { if ((getMovie().getPriceCode() == Movie.NEW_RELEASE) && getDaysRented() > 1) return 2; else return 1; }
我利用重构前后的UML图(图1-4~图1-7)来总结刚才所做的修改。和先前一样,左页是修改前的图,右页是修改后的图。
class Customer... public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += each.getFrequentRenterPoints(); // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "\n"; totalAmount += each.getCharge(); } // add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; }首先我用Customer类的getTotalCharge()取代totalAmount: class Customer... public String statement() { int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += each.getFrequentRenterPoints(); // show figures for this rental result += "\t" + each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "\n"; } // add footer lines result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; } private double getTotalCharge() { double result = 0; Enumeration rentals = _rentals.elements(); while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); result += each.getCharge(); } return result;}
这并不是Replace Temp with Query (120)的最简单情况。由于totalAmount在循环内部被赋值,我不得不把循环复制到查询函数中。
重构之后,重新编译并测试,然后以同样手法处理frequentRenterPoints:
class Customer... public String statement() { int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"', while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += each.getFrequentRenterPoints(); //show figures for this rental result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(each.getCharge()) + "\n"; } //add footer lines result += "Amount owed is "+ String.valueOf(getTotalCharge()) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; }public String statement() { Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); //show figures for this rental result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(each.getCharge()) + "\n"; } //add footer lines result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n"; result += "You earned "+ String.valueOf(getTotaIFrequentRenterPoints()) + " frequent renter points"; return result;}private int getTotaIFrequentRenterPoints(){ int result = 0; Enumeration rentals = _rentals.elements(); while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); result += each.getFrequentRenterPoints(); } return result;}
图1-8~图1-11分别以UML类图和交互图展示statement()重构前后的变化。
这次重构存在另一个问题,那就是性能。原本代码只执行while循环一次,新版本要执行三次。如果while循环耗时很多,就可能大大降低程序的性能。单单为了这个原因,许多程序员就不愿进行这个重构动作。但是请注意我的用词:“如果”和“可能”。除非我进行评测,否则我无法确定循环的执行时间,也无法知道这个循环是否被经常使用以至于影响系统的整体性能。重构时你不必担心这些,优化时你才需要担心它们,但那时候你已处于一个比较有利的位置,有更多选择可以完成有效优化(见第69页的讨论)。
现在,Customer类内的任何代码都可以调用这些查询函数了。如果系统其他部分需要这些信息,也可以轻松地将查询函数加入Customer类接口。如果没有这些查询函数,其他函数就必须了解Rental类,并自行建立循环。在一个复杂系统中,这将使程序的编写难度和维护难度大大增加。
你可以很明显看出来,htmlStatement()和statement()是不同的。现在,我应该脱下“重构”的帽子,戴上“添加功能”的帽子。我可以像下面这样编写htmlStatement(),并添加相应测试:
public String htmlStatement() { Enumeration rentals = _rentals.elements(); String result = "Rentals for " + getName() + "
\n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); // show figures for each rental result += each.getMovie().getTitle() + ": "+ String.valueOf(each.getCharge()) + "
\n"; } // add footer lines result += "You owe " + String.valueOf(getTotalCharge())+ "
\n"; result += "On this rental you earned "+ String.valueOf(getTotalFrequentRenterPoints()) + " frequent renter points
"; return result; }
通过计算逻辑的提炼,我可以完成一个htmlStatement(),并复用原本statement()内的所有计算。我不必剪剪贴贴,所以如果计算规则发生改变,我只需在程序中做一处修改。完成其他任何类型的详单也都很快而且很容易。这次重构并没有花很多时间,其中大半时间我用来弄清楚代码所做的事,而这是我无论如何都得做的。
前面有些代码是从ASCII版本中复制过来的——主要是循环设置部分。更深入的重构动作可以清除这些重复代码。我可以把处理表头(header)、表尾(footer)和详单细目的代码都分别提炼出来。在Form Template Method (345)实例中,你可以看到如何做这些动作。但是,现在用户又开始嘀咕了,他们准备修改影片分类规则。我们尚未清楚他们想怎么做,但似乎新分类法很快就要引入,现有的分类法马上就要变更。与之相应的费用计算方式和常客积分计算方式都还有待决定,现在就对程序做修改,肯定是愚蠢的。我必须进入费用计算和常客积分计算中,把因条件而异的代码[3]替换掉,这样才能为将来的改变镀上一层保护膜。现在,请重新戴回“重构”这顶帽子。
转载地址:http://ajkda.baihongyu.com/