重构:改善既有代码的设计(1)

文章说明:

  1.这是一部经典的著作,下面的大多数文字都是翻译的原文,部分难懂的句子也参考了本书的中文版《重构:改善既有代码的设计》--侯捷 熊节 译

  2.本文跟原文相比精简了许多,如果你有什么不明白的地方,建议你阅读原著作,英文不好可以阅读注释版(我所阅读的版本)或者中文版

  3.也请大家不要喷饭,我并没有糟蹋原著作的意思,只相当于自己的读书笔记而已

  4.由于原文介绍第一个例子的代码较多,本文也是尽量注意代码的排版,如果仍有不懂地方,可以留言交流,可以对照原文

 

   该如何介绍重构呢?按照传统的作法,应该首先讲讲它的历史、主要原理等等,但是每当有人讲起这些时,我们也总是昏昏欲睡,思绪也开始游荡,直到他拿出实例,我们才会打起精神。

   所以这里以一个实例作为起点,为你介绍重构的原理,让你对重构有一点感觉。

   但是这里又陷入一个两难的境地,如果选择一个大型的程序,对于程序自身和重构的描述都太过复杂,任何人都不能掌握。选择一个小的容易理解的程序来讲解,可能又看不出重构的价值。所以,选择的例子坦白的说,根本就不值得我们去重构,但是请将这个例子看做是大系统的一部分,重构技术也就变得重要起来了,所以,你一定要设身处地的想象。

   这是一个录像带出租店使用的程序,计算每一位顾客的消费金额和打印报表。操作者告诉程序:顾客租了哪些影片、租期多长,以便程序根据影片的类型和租赁时间计算出租费用,影片分为3类:普通片,儿童片和新片。除了计算费用,还要为常客计算点数,点数会因为是否租赁新片而不同,以下为代码:

//Movie.java
public class Movie {
	public static final int CHILDRENS = 2;
	public static final int REGULAR = 0;
	public static final int NEW_RELEASE = 1;
	
	private String title; //片名
	private int priceCode; //价格代号
	
	public Movie(String title, int priceCode) {
		this.title = title;
		this.priceCode = priceCode;
	}
	
	public String getTitle() {
		return title;
	}
	public int getPriceCode() {
		return priceCode;
	}
	public void setPriceCode(int priceCode) {
		this.priceCode = priceCode;
	}}
//Rental.java
public class Rental {
	private Movie movie;
	private int daysRented;
	
	public Rental(Movie movie, int daysRented) {
		this.movie = movie;
		this.daysRented = daysRented; //租期
	}
	public Movie getMovie() {
		return movie;
	}
	public int getDaysRented() {
		return daysRented;
	}	
}
//Customer.java
//Customer类表示顾客,它还提供一个制造报表的方法
public class Customer {
	private String name;
	private Vector<Rental> rentals = new Vector<Rental>();
	
	public Customer(String name) {
		this.name = name;
	}

	public String getName() {
		return name;
	}
	
	public void addRental(Rental r) {
		rentals.addElement(r);
	}
	
	public String statement() {
		double totalAmout = 0; //消费金额
		int frequentRenterPoints = 0;//常客积分
		Enumeration<Rental> rs = rentals.elements();
		String result = "Rental Record for" + this.getName() +"\n";
		while(rs.hasMoreElements()) {
			double thisAmout = 0;
		 	Rental each = (Rental)rs.nextElement();
			
		switch(each.getMovie().getPriceCode()){//各种影片的价格不同
				case Movie.REGULAR:
					thisAmout += 2;
					if(each.getDaysRented()>2)
						thisAmout += (each.getDaysRented()-2)*1.5;
					break;
				case Movie.NEW_RELEASE:
					thisAmout += each.getDaysRented()*3;
					break;
				case Movie.CHILDRENS:
					thisAmout += 1.5;
					if(each.getDaysRented()>3)
						thisAmout += (each.getDaysRented()-3)*1.5;
					break;
			}
			//累计积分
			frequentRenterPoints ++;
			if((each.getMovie().getPriceCode()==Movie.NEW_RELEASE)&&
					each.getDaysRented()>1) frequentRenterPoints ++;
			//显示此次租赁的数据
			result += "\t" +each.getMovie().getTitle() + "\t" + String.valueOf(thisAmout) + "\n";
			totalAmout += thisAmout;
		}
		result += "Amout owed is" + String.valueOf(totalAmout) + "\n";
		result += "You earned" + String.valueOf(frequentRenterPoints) + "frequent renter points";
		return result;
	}
}

  这样的设计给你的印象怎么样?诚然,对于这样一个小的程序,简单而又随性的设计并没有什么过错。但是,我会认为它设计得不好,明显不符合面向对象的特征。如果一个大的系统中有这样的代码,那么我对程序的信心动摇了,因为Customer类里那个statement()做了太多不应该它做的事。

  即便如此,这个程序还是能够正常运行,因为编译器是不在意你的代码写得丑陋与否。那么,这个程序是否就真的这样丑陋呢?如果你不打算修改这段代码,那么确实没什么关系。但是如果你要修改代码的时候,这样差劲的代码是很难修改的,因为我们很难找到修改的地方,这样我们很快就会犯错,进而引入bug。

  现在假设,用户希望以HTML格式打印报表,这样就可以直接在网络上显示。看看上面的代码,你就会发现,为了实现这个功能,你唯有重新编写一个htmlStatement(),当中大量重复statement()的行为。当然,这个并不是很费力,因为你只需要将statement()中的代码拷贝一份,然后修改就可以了。

  又如果,现在用户要修改计费标准,那么你需要修改statement(),同时你还要修改htmlStatement(),并且要保持它们修改的地方一致。再如果,用户想要修改影片的分类规则,这直接影响着计费和积分点数的计算。诸如此类,你会发现当用户的需求不断的改变,上面的代码将越来越难以修改。

  当需要为自己的程序添加新功能时,你发现代码的结构不能使你很方便的那么做,那就先进行重构,让代码的结构容易添加新功能,然后再添加新特性。

The First Step in Refactoring(重构第一步)

  当你在重构时,无论如何,你首先要干的事就是构建一组可靠的测试环境。尽管遵循重构准则可以避免绝大多数的错误,但是只要是人就可能犯错,我们需要可靠的测试环境。

  由于statement()返回一个字符串,我首先假设一些顾客租了不同的影片,并产生报表字符串。然后,我就可以拿这些字符串与我手上已经检查过的字符串比较,然后得出结果。

  对于测试的结构的设计也很重要。它们要么说"OK",表示测试的字符串与参照的字符串完全一样;要么显示一系列错误的信息,显示问题字符串的行号。这样的测试都属于自我检验。你必须让你的测试有自我检验的功能,不然你就得花大把的时间来回的对比,降低开发效率。

  所有的重构都依赖与测试。所以,在你重构之前,首先检查一下你是否有这样一套具备自我检验功能的测试机制。

Decomposing and Redistributing the Statement Method(分解并重组statement方法)

  首先引起你注意的应该是长的掉渣的statement(),每当我们看到这样长的方法时,就应该把它分解成小的代码块,因为代码块越小就容易管理,也更容易处理和迁移。

  在第一阶段,我将说明怎样将长长的代码切开,并把它放到合适的类中。我的目标是降低代码重复量,让写htmlStatement()更容易。

  第一步是找出代码中的逻辑代码团并运用Exatract Method(提取方法,后文介绍)。这里很明显的就是switch代码块了,把它放到单独的方法中似乎更好。

  首先我得在方法中找出局部变量和参数,这里很明显的就是each和thisAmout,each一直没有被修改,所以可以作为参数直接传递进去,而thisAmout一直被修改,并且这里只有这样一个一直在改变的变量,可以把它当作返回值。thisAmout是一个临时变量,它的值在每次循环的开始都被设置为0,且在switch之前不会改变,所以可以将新方法返回的值直接赋给它。

  下面就是经过第一次分解后的代码:

public class Customer {
	//此处省略代码
	public String statement() {
		//此处省略代码
		while(rs.hasMoreElements()) {
			double thisAmout = 0;
			Rental each = (Rental)rs.nextElement();
			thisAmout = amoutFor(each);
			//此处省略代码
		}
		return result;
	}
	
	public double amoutFor(Rental each) {
		double thisAmout = 0;
		switch(each.getMovie().getPriceCode()) { //各种影片的价格不同
			case Movie.REGULAR:
				thisAmout += 2;
				if(each.getDaysRented()>2)
					thisAmout += (each.getDaysRented()-2)*1.5;
				break;
			case Movie.NEW_RELEASE:
				thisAmout += each.getDaysRented()*3;
				break;
			case Movie.CHILDRENS:
				thisAmout += 1.5;
				if(each.getDaysRented()>3)
					thisAmout += (each.getDaysRented()-3)*1.5;
				break;
	}
		return thisAmout;
	}
}

  好了,第一步完成,但是我不喜欢新方法的变量名(修改变量名也是一种重构),所以我把它做如下修改(each->rental,thisAmout->result):

public double amoutFor(Rental rental) {
		double result = 0;
		switch(rental.getMovie().getPriceCode()) { //各种影片的价格不同
			case Movie.REGULAR:
				result += 2;
				if(rental.getDaysRented()>2)
					result += (rental.getDaysRented()-2)*1.5;
				break;
			case Movie.NEW_RELEASE:
				result += rental.getDaysRented()*3;
				break;
			case Movie.CHILDRENS:
				result += 1.5;
				if(rental.getDaysRented()>3)
					result += (rental.getDaysRented()-3)*1.5;
				break;
		}
		return result;
	}

  这里说说,重命名是否值得呢?当然。好的代码应该清楚的表达出自己的功能,变量名就是关键。为了提高代码的清晰度,不要怕为重命名而付出努力。而且好的编译工具是重命名变得很简单,类型检查和测试也能够检查出你的纰漏。所以记住:

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

  作者经验:代码应该表现自己的目的,这一点非常重要。阅读代码的时候,我经常进行重构。这样,随着对程序的理解逐渐加深,我也就不断地把这些理解嵌入到代码中,这么一来才不会遗忘我曾经理解的东西。

  我们重新审视amoutFor(),我发现它所用的信息全部来自Rental类,没有使用Customer类中的一丁点信息。所以有理由怀疑这个方法放在了一个错误的位置。绝大多数情况下,方法应该放到它所使用的数据所属的类当中去,所以amoutFor()应该放到Rental类中去。为了实现这个目的,我使用Move Method(移动方法,后文介绍)。注意,在移动的过程中修改了方法名。

	public class Rental {
	//此处省略代码
		public 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类的代码:

public class Customer {
	//其它代码省略
	public String statement() {
			thisAmout = each.getCharge();
	}
}

  接下来发现thisAmout变成了多余的了,所以这里我要删除这个临时变量(即把thisAmout用each.getCharge()替代)。statement()就变成了如下模样:

public String statement() {
	//此处省略部分代码
		while(rs.hasMoreElements()) {
			Rental each = (Rental)rs.nextElement();
			//累计积分
			frequentRenterPoints ++;
			if((each.getMovie().getPriceCode()==Movie.NEW_RELEASE)&&
					each.getDaysRented()>1) frequentRenterPoints ++;
			//显示此次租赁的数据
			result += "\t" +each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "\n";
			totalAmout += each.getCharge();
		}
		result += "Amout owed is" + String.valueOf(totalAmout) + "\n";
		result += "You earned" + String.valueOf(frequentRenterPoints) + "frequent renter points";
		return result;
	}

  在编写代码过程中尽量删除临时变量,因为它们往往导致大量的参数被传来传去,这完全没有必要。特别是在比较长的方法中,你可能很容易就失去它们的踪迹。当然这也损失一些性能,比如上面的代码中charge(指:each.getCharge())被计算了两次。但是这在Rental类中很容易优化。而且如果你的代码有合理的结构和有效的管理,优化会有更好的效果。

  现在我用同样的方法来重构计算积分的过程:在statement方法中计算积分的代码如下:

//累计积分
frequentRenterPoints ++;
if((each.getMovie().getPriceCode()==Movie.NEW_RELEASE)&&
			each.getDaysRented()>1)frequentRenterPoints ++;

  我们把它放在了Rental类中(顾客租赁所有影片积分先加1,如果是新片积分再增加1),所以在Rental就应该这样:

public class Rental {
	//此处省略部分代码
	public int getFrequentRenterPoints() {
		if((getMovie().getPriceCode()==Movie.NEW_RELEASE)&& getDaysRented()>1) 
			return 2;
		else
			return 1;
		}

  那么计算积分的两句代码就应该变成如下一句:

frequentRenterPoints += each.getFrequentRenterPoints();

  接下来我们在删除代码中临时变量,现在的statement()方法如下:

public String statement() {
		double totalAmout = 0; //消费金额
		int frequentRenterPoints = 0;//常客积分
		Enumeration<Rental> rs = rentals.elements();
		String result = "Rental Record for" + this.getName() +"\n";
		while(rs.hasMoreElements()) {
			Rental each = (Rental)rs.nextElement();
			//累计积分
			frequentRenterPoints += each.getFrequentRenterPoints();
			//显示此次租赁的数据
			result += "\t" +each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "\n";
			totalAmout += each.getCharge();
		}
		result += "Amout owed is" + String.valueOf(totalAmout) + "\n";
		result += "You earned" + String.valueOf(frequentRenterPoints) + "frequent renter points";
		return result;
}

  正如前面提到的,临时变量可能是个问题,它们只在自己的方法中有效,所以它们使方法复杂而且冗长。这里有两个临时变量totalAmout和frequentRentalPoints,两者都是用来从Customer对象相关的Rental对象中获得某个总量,不论是ASCII版还是HTML版本的statement都需要这些总量,所以这里运用Raplace Temp With Query中的query Method来取代它们,由于两个临时变量在while循环中,所以下面的两个方法不能不重复的写while循环。

public String statement() {
		Enumeration<Rental> rs = rentals.elements();
		String result = "Rental Record for" + this.getName() +"\n";
		while(rs.hasMoreElements()) {
			Rental each = (Rental)rs.nextElement();
			//显示此次租赁的数据
			result += "\t" +each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "\n";
		}
		result += "Amout owed is" + String.valueOf(getTotalCharge()) + "\n";
		result += "You earned" + String.valueOf(getTotalFrequentRenterPoints()) + "frequent renter points";
		return result;
	}
	//取得总金额
	private double getTotalCharge() {
		double result = 0;
		Enumeration<Rental> rs = rentals.elements();
		while(rs.hasMoreElements()) {
			Rental each = (Rental)rs.nextElement();
			result += each.getCharge();
		}
		return result;
	}
	//取得总积分
	private int getTotalFrequentRenterPoints() {
		int result = 0;
		Enumeration<Rental> rs = rentals.elements();
		while(rs.hasMoreElements()) {
			Rental each = (Rental)rs.nextElement();
			result += each.getFrequentRenterPoints();
		}
		return result;
		}

  现在有必要停下来思考一下,大多数重构都是减少代码,这次却是增加代码。另一个问题就是原来只执行一次循环,这次却执行了3次,如果每次循环执行的时间很多,那么就可能大大的降低效率,所以很多程序员但从这方面就不愿意进行这次重构。但是你要思考这样做是否值得,这里也只是可能降低效率,除非进行很严格的测试,否则你不知道每次循环到底会用掉多少时间,更无法确定多执行的这两次循环是否会影响这个系统的性能。所以重构时,你无须担心这些,优化的时候才是你担心这个的时候,到那个时候,你已经处于有利的位置了,你可能会有更多的选择来完成有效的优化。

  现在我们可以暂时不要去想重构了,我们可以想想增加功能了,我们现在可以向下面这样写htmlStatement方法了:

public String htmlStatement() {
		Enumeration<Rental> rs = rentals.elements();
		String result = "<h1>Rental Record for<em>" + this.getName() +"</em></h1><p>\n";
		while(rs.hasMoreElements()) {
			Rental each = (Rental)rs.nextElement();
			//显示此次租赁的数据
			result += "\t" +each.getMovie().getTitle() + "\t" + String.valueOf(each.getCharge()) + "<br>\n";
		}
		result += "<p>Amout owed is<em>" + String.valueOf(getTotalCharge()) + "</em></p>\n";
		result += "You earned<em>" + String.valueOf(getTotalFrequentRenterPoints()) + "</em>frequent renter points<p>";
		return result;
}

  我们复用原来的版本得到了新的htmlStatement,如果计算规则发生改变,我们也不用再复制粘贴了,我们只需要需改程序中的一处。若还需要其它的报表,也是很容易实现的事情了。这个重构并没有花很多时间,大部分时间都是用来分析代码所完成的功能,如果要增加新功能,这些事情都是我们必须要做的。

                                                                                                                                                                                                              第一篇:完

posted on 2011-09-09 20:37 luecsc 阅读( ...) 评论( ...) 编辑 收藏

转载于:https://www.cnblogs.com/luecsc/archive/2011/09/09/2172813.html

  • 0
    点赞
  • 0
    收藏
    觉得还不错? 一键收藏
  • 0
    评论
评论
添加红包

请填写红包祝福语或标题

红包个数最小为10个

红包金额最低5元

当前余额3.43前往充值 >
需支付:10.00
成就一亿技术人!
领取后你会自动成为博主和红包主的粉丝 规则
hope_wisdom
发出的红包
实付
使用余额支付
点击重新获取
扫码支付
钱包余额 0

抵扣说明:

1.余额是钱包充值的虚拟货币,按照1:1的比例进行支付金额的抵扣。
2.余额无法直接购买下载,可以购买VIP、付费专栏及课程。

余额充值