重构,第一个案例
下面的实例是一个影片出租店用的程序,计算每一个顾客的消费金额并打印报表。操作者告诉程序员:顾客组了哪些影片,租期多长,程序便根据租赁时间和影片类型计算出费用。影片分成三种类型:普通片,儿童片和新片。除了计算费用以外,还要为常客计算点数;点数随着【租片种类是否为新片】而有不同。
Movie(影片)
movie只是一个简单的data class(纯数据类)
1 public class Movie { 2 public static final int CHILDRENS = 2; 3 public static final int REGULAR = 0; 4 public static final int NEW_RELEASE = 1; 5 6 private String _title; //名称 7 private int _priceCode; //价格(代号) 8 9 public Movie(String title, int priceCode) { 10 _title = title; 11 _priceCode = priceCode; 12 } 13 14 public int getPriceCode() { 15 return _priceCode; 16 } 17 18 public void setPriceCode(int priceCode) { 19 _priceCode = priceCode; 20 } 21 22 public String getTitle() { 23 return _title; 24 } 25 }
Rental (租赁)
Rental class 表示某个顾客租了一部影片
1 public class Rental { 2 private Movie _movie; 3 private int _daysRented; 4 5 public Rental(Movie movie, int daysRented) { 6 _movie = movie; 7 _daysRented = daysRented; 8 } 9 10 public int getDaysRented() { 11 return _daysRented; 12 } 13 14 public Movie getMovie() { 15 return _movie; 16 } 17 }
Customer(顾客)
Customer class 用来表示顾客,就像其他classes一样,它也拥有数据和相应的访问函数
1 import java.util.Enumeration; 2 import java.util.Vector; 3 4 public class Customer { 5 private String _name; 6 private Vector _rentals = new Vector(); 7 8 public void addRental(Rental arg) { 9 _rentals.addElement(arg); 10 } 11 12 public String getName() { 13 return _name; 14 } 15 16 public String statement() { 17 double totalAmount = 0; 18 int frequentRenterPoints = 0; 19 Enumeration rentals = _rentals.elements(); 20 String result = "Rental Record for " + getName() + "\n"; 21 22 while(rentals.hasMoreElements()) { 23 double thisAmount = 0; 24 Rental each = (Rental)rentals.nextElement(); 25 26 switch(each.getMovie().getPriceCode()) { 27 case Movie.REGULAR: 28 thisAmount += 2; 29 if (each.getDaysRented() > 2) 30 thisAmount += (each.getDaysRented() - 2)*1.5; 31 break; 32 case Movie.NEW_RELEASE: 33 thisAmount += each.getDaysRented() * 3; 34 break; 35 case Movie.CHILDRENS: 36 thisAmount += 1.5; 37 if (each.getDaysRented() > 3) 38 thisAmount += (each.getDaysRented() - 3) * 1.5; 39 break; 40 } 41 42 frequentRenterPoints++; 43 44 if (each.getMovie().getPriceCode() == Movie.NEW_RELEASE && 45 each.getDaysRented() > 1) { 46 frequentRenterPoints++; 47 } 48 49 result += "\t" + each.getMovie().getTitle() + "\t" + 50 String.valueOf(thisAmount) + "\n"; 51 totalAmount += thisAmount; 52 } 53 54 result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; 55 result += "You earned " + String.valueOf(frequentRenterPoints) + 56 " frequent renter points"; 57 return result; 58 59 } 60 }
以上就是所有的代码了!从customer我们可以看见,它拥有一个statement方法,同时它的这个方法也是我们现在看到的当中最长的代码!那么我们说这个方法好吗?其实在刚开始学习java的时候,向我就写过这样的代码!如果是面对简单的程序,这种方式或者说写法并不存在太多的不妥!但是,当我们的代码因着需求的变化而发生变化的时候,这样的代码最终会应为变得不断的庞大,复杂而导致难以维护的局面。
所以我们要对这个代码进行重构,首先我们应该要考虑的是statement当中是否存在我们可以复用的代码。
复用是什么意思呢?比如我们又要写statementHtml方法来做打印页面的功能。结果当中我们可能有一部分代码其实是一样的,而如果我们把这个相同的代码提取出来,放在另外的类或者函数里面,当我们需要它的时候将它调用来使用,这种过程就叫做复用。而复用是为了更好的维护代码,比如被复用的是一个函数,而这个函数内部某个逻辑需要修改,那么我就只要修改这个函数的逻辑改了就好了,但是如果我们原来并没有将这份代码抽取出来,那么可能这个相同的代码可能分布各个角落,那个你就要一个个找出来并修改掉,我们说这样的代码机器能够识别,但是这样的代码对我们人来说,却是不友好的,所以我们应该要保证代码的复用性。
首先我们看见statement当中的switch我们会看见,这个逻辑是根据我们传入的租赁影片对应的类型计算对应的消费金额,而这里我们看到当中的代码thisAmount只在当中做过一次修改,而后就没有什么变化了,所以我们可以以此为标志,当某个参数只需要一次的定值,就不再有任何修改的时候,我们只需要将计算这个定值的算法提取出来,另外见一个函数,并通过返回值的方式返回计算的结果。
1 import java.util.Enumeration; 2 import java.util.Vector; 3 4 public class Customer { 5 private String _name; 6 private Vector _rentals = new Vector(); 7 8 public void addRental(Rental arg) { 9 _rentals.addElement(arg); 10 } 11 12 public String getName() { 13 return _name; 14 } 15 16 public String statement() { 17 double totalAmount = 0; 18 int frequentRenterPoints = 0; 19 Enumeration rentals = _rentals.elements(); 20 String result = "Rental Record for " + getName() + "\n"; 21 22 while(rentals.hasMoreElements()) { 23 Rental each = (Rental)rentals.nextElement(); 24 25 double thisAmount = amountFor(each); 26 27 frequentRenterPoints++; 28 29 if (each.getMovie().getPriceCode() == Movie.NEW_RELEASE && 30 each.getDaysRented() > 1) { 31 frequentRenterPoints++; 32 } 33 34 result += "\t" + each.getMovie().getTitle() + "\t" + 35 String.valueOf(thisAmount) + "\n"; 36 totalAmount += thisAmount; 37 } 38 39 result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; 40 result += "You earned " + String.valueOf(frequentRenterPoints) + 41 " frequent renter points"; 42 return result; 43 44 } 45 46 public double amountFor(Rental each) { 47 double result = 0; 48 switch(each.getMovie().getPriceCode()) { 49 case Movie.REGULAR: 50 result += 2; 51 if (each.getDaysRented() > 2) 52 result += (each.getDaysRented() - 2)*1.5; 53 break; 54 case Movie.NEW_RELEASE: 55 result += each.getDaysRented() * 3; 56 break; 57 case Movie.CHILDRENS: 58 result += 1.5; 59 if (each.getDaysRented() > 3) 60 result += (each.getDaysRented() - 3) * 1.5; 61 break; 62 } 63 return result; 64 } 65 }
接下来我们在来看一下代码,我们会发现,修改后的代码通过调用amountFor方法获取值,并将值付给了thisAmount,而这个thisAmount我们看到它在statement中只是做答应和加到totalAmount 中的变量,而我们重构中存在一个原则,就是一个函数里面的局部变量尽量少一点,因为越多的局部变量反而会让代码慢慢变得无法拆解,或者容易使得代码变得更长!所以我们接下来去掉这个变量。
1 import java.util.Enumeration; 2 import java.util.Vector; 3 4 public class Customer { 5 private String _name; 6 private Vector _rentals = new Vector(); 7 8 public void addRental(Rental arg) { 9 _rentals.addElement(arg); 10 } 11 12 public String getName() { 13 return _name; 14 } 15 16 public String statement() { 17 double totalAmount = 0; 18 int frequentRenterPoints = 0; 19 Enumeration rentals = _rentals.elements(); 20 String result = "Rental Record for " + getName() + "\n"; 21 22 while(rentals.hasMoreElements()) { 23 Rental each = (Rental)rentals.nextElement(); 24 frequentRenterPoints++; 25 26 if (each.getMovie().getPriceCode() == Movie.NEW_RELEASE && 27 each.getDaysRented() > 1) { 28 frequentRenterPoints++; 29 } 30 31 result += "\t" + each.getMovie().getTitle() + "\t" + 32 String.valueOf(amountFor(each)) + "\n"; 33 totalAmount += amountFor(each); 34 } 35 36 result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; 37 result += "You earned " + String.valueOf(frequentRenterPoints) + 38 " frequent renter points"; 39 return result; 40 41 } 42 43 public double amountFor(Rental each) { 44 double result = 0; 45 switch(each.getMovie().getPriceCode()) { 46 case Movie.REGULAR: 47 result += 2; 48 if (each.getDaysRented() > 2) 49 result += (each.getDaysRented() - 2)*1.5; 50 break; 51 case Movie.NEW_RELEASE: 52 result += each.getDaysRented() * 3; 53 break; 54 case Movie.CHILDRENS: 55 result += 1.5; 56 if (each.getDaysRented() > 3) 57 result += (each.getDaysRented() - 3) * 1.5; 58 break; 59 } 60 return result; 61 } 62 }
现在我在说一个原则:一个函数应该与所在类有关联性。所以我们可以看到,amountFor函数除了用到Rental以外,并没有与Customer存在关联性,所以我们可以怀疑这个逻辑应该被迁移到他应该存在的位置,也就是Rental中。
1 //Custormer 2 import java.util.Enumeration; 3 import java.util.Vector; 4 5 public class Customer { 6 private String _name; 7 private Vector _rentals = new Vector(); 8 9 public void addRental(Rental arg) { 10 _rentals.addElement(arg); 11 } 12 13 public String getName() { 14 return _name; 15 } 16 17 public String statement() { 18 double totalAmount = 0; 19 int frequentRenterPoints = 0; 20 Enumeration rentals = _rentals.elements(); 21 String result = "Rental Record for " + getName() + "\n"; 22 23 while(rentals.hasMoreElements()) { 24 Rental each = (Rental)rentals.nextElement(); 25 frequentRenterPoints++; 26 27 if (each.getMovie().getPriceCode() == Movie.NEW_RELEASE && 28 each.getDaysRented() > 1) { 29 frequentRenterPoints++; 30 } 31 32 result += "\t" + each.getMovie().getTitle() + "\t" + 33 String.valueOf(amountFor(each)) + "\n"; 34 totalAmount += amountFor(each); 35 } 36 37 result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; 38 result += "You earned " + String.valueOf(frequentRenterPoints) + 39 " frequent renter points"; 40 return result; 41 42 } 43 44 public double amountFor(Rental each) { 45 return each.getCharge(); 46 } 47 } 48 49 //Rental 50 public class Rental { 51 private Movie _movie; 52 private int _daysRented; 53 54 public Rental(Movie movie, int daysRented) { 55 _movie = movie; 56 _daysRented = daysRented; 57 } 58 59 public int getDaysRented() { 60 return _daysRented; 61 } 62 63 public Movie getMovie() { 64 return _movie; 65 } 66 67 public double getCharge() { 68 double result = 0; 69 switch(getMovie().getPriceCode()) { 70 case Movie.REGULAR: 71 result += 2; 72 if (getDaysRented() > 2) 73 result += (getDaysRented() - 2)*1.5; 74 break; 75 case Movie.NEW_RELEASE: 76 result += getDaysRented() * 3; 77 break; 78 case Movie.CHILDRENS: 79 result += 1.5; 80 if (getDaysRented() > 3) 81 result += (getDaysRented() - 3) * 1.5; 82 break; 83 } 84 return result; 85 } 86 }
好了,直到现在,我们就看到了这个代码从最开始到现在,基本上已经修改完成了,但是我们说这个代码已经合格了吗?其实并没有,而具体是哪里没有合格呢?不知道你有没有你的想法,可以写下你的想法在评论区,还有要改的代码将在下次的一节中补上。