重构—改善既有代码的设计001：重构，第一个案例
一：案例
class Movie
{
public const int CHILDRENS = 2;
public const int NEW_RELEASE = 1;
public const int REGULAR = 0;
private string _title;
private int _priceCode;
public Movie(string title, int priceCode)
{
_title = title;
_priceCode = priceCode;
}
public int getPriceCode()
{
return _priceCode;
}
public void setPriceCode(int arg)
{
_priceCode = arg;
}
public string getTitle()
{
return _title;
}
}
class Rental
{
private Movie _movie;
private int _daysRented;
public Rental(Movie movie, int daysRented)
{
_movie = movie;
_daysRented = daysRented;
}
public int getDaysRented()
{
return _daysRented;
}
public Movie getMovie()
{
return _movie;
}
}
class Customer
{
private string _name;
private ArrayList _rentals = new ArrayList();
public Customer(string name)
{
_name = name;
}
public void addRental(Rental arg)
{
_rentals.Add(arg);
}
public string getName()
{
return _name;
}
public string statement()
{
double totalAmount = 0;
int frequentRenterPoints = 0;
string result = "Rental record for " + getName() + "/n";
for (int index = 0; index < _rentals.Count; index++)
{
double thisAmount = 0;
Rental each = (Rental)_rentals[index];
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;
}
frequentRenterPoints++;
if (each.getMovie().getPriceCode() == Movie.NEW_RELEASE &&
each.getDaysRented() > 1)
frequentRenterPoints++;
result += "/t" + each.getMovie().getTitle() + "/t" + thisAmount.ToString() + "/n";
totalAmount += thisAmount;
}
result += "Amount owed is " + totalAmount.ToString() + "/n";
result += "You earned " + frequentRenterPoints.ToString() + "frequent renter points";
return result;
}
如果发现自己需要为程序添加一个特性，而代码结构是你无法很方便地那么做，那么就先重构那个程序，使特性的添加比较容易进行，然后再添加特性。
二：分解并重组statement（）
代码块愈小，代码的功能就愈容易管理，代码的处理和搬移也都愈轻松。
重构步骤的本质：每次修改的幅度都很小，所以任何错误都很容易发现，这样就不必耗费大把时间调试。
1：分解SWITCH
找出函数内部的局部变量和参数，thisAmount被修改，each未被修改。
任何不会被修改的变量都可以被我们当成参数传入新的函数，至于会被修改的变量就绪格外小心。
如果只有一个变量会被修改，我们可以把它当做返回值。
public string statement()
{
double totalAmount = 0;
int frequentRenterPoints = 0;
string result = "Rental record for " + getName() + "/n";
for (int index = 0; index < _rentals.Count; index++)
{
double thisAmount = 0;
Rental each = (Rental)_rentals[index];
thisAmount = amountFor(each);
frequentRenterPoints++;
if (each.getMovie().getPriceCode() == Movie.NEW_RELEASE &&
each.getDaysRented() > 1)
frequentRenterPoints++;
result += "/t" + each.getMovie().getTitle() +
"/t" + thisAmount.ToString() + "/n";
totalAmount += thisAmount;
}
result += "Amount owed is " + totalAmount.ToString() + "/n";
result += "You earned " + frequentRenterPoints.ToString() + "frequent renter points";
return result;
}
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;
}
2：搬移“金额计算”代码
函数应该放在它所使用的数据所属OBJECT或CLASS内，所以amountFor应该放入到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;
}
public string statement()
{
double totalAmount = 0;
int frequentRenterPoints = 0;
string result = "Rental record for " + getName() + "/n";
for (int index = 0; index < _rentals.Count; index++)
{
double thisAmount = 0;
Rental each = (Rental)_rentals[index];
thisAmount = each.getCharge();
frequentRenterPoints++;
if (each.getMovie().getPriceCode() == Movie.NEW_RELEASE &&
each.getDaysRented() > 1)
frequentRenterPoints++;
result += "/t" + each.getMovie().getTitle() + "/t" + thisAmount.ToString() + "/n";
totalAmount += thisAmount;
}
result += "Amount owed is " + totalAmount.ToString() + "/n";
result += "You earned " + frequentRenterPoints.ToString() + "frequent renter points";
return result;
}
使用查询代替临时变量thisAmount
临时变量会导致大量参数被传来传去，而其实完全没有这种必要。很容易失去它们的踪迹，特别是在常常的函数之中更是如此。
public string statement()
{
double totalAmount = 0;
int frequentRenterPoints = 0;
string result = "Rental record for " + getName() + "/n";
for (int index = 0; index < _rentals.Count; index++)
{
Rental each = (Rental)_rentals[index];
frequentRenterPoints++;
if (each.getMovie().getPriceCode() == Movie.NEW_RELEASE &&
each.getDaysRented() > 1)
frequentRenterPoints++;
result += "/t" + each.getMovie().getTitle() + "/t" + each.getCharge().ToString() + "/n";
totalAmount += each.getCharge();
}
result += "Amount owed is " + totalAmount.ToString() + "/n";
result += "You earned " + frequentRenterPoints.ToString() + "frequent renter points";
return result;
}
3：提炼“常客积点计算”代码
寻找临时变量each和frequentRenterPoints，
public string statement()
{
double totalAmount = 0;
int frequentRenterPoints = 0;
string result = "Rental record for " + getName() + "/n";
for (int index = 0; index < _rentals.Count; index++)
{
Rental each = (Rental)_rentals[index];
frequentRenterPoints += each.getFrequentRenterPoints();
result += "/t" + each.getMovie().getTitle() +
"/t" + each.getCharge().ToString() + "/n";
totalAmount += each.getCharge();
}
result += "Amount owed is " + totalAmount.ToString() + "/n";
result += "You earned " + frequentRenterPoints.ToString() + "frequent renter points";
return result;
}
public int getFrequentRenterPoints()
{
if (getMovie().getPriceCode() == Movie.NEW_RELEASE &&
getDaysRented() > 1)
return 2;
else
return 1;
}
4：去除临时变量totalAmount，由于totalAmount在内部循环内用到了，所以需要专门一出来循环计算
类似处理frequentRenterPoints
目前重构不仅增加了代码量，还增加了循环次数，这些对性能的影响可以放到后续的优化过程中考虑。
public string statement()
{
string result = "Rental record for " + getName() + "/n";
for (int index = 0; index < _rentals.Count; index++)
{
Rental each = (Rental)_rentals[index];
result += "/t" + each.getMovie().getTitle() + "/t" + each.getCharge().ToString() + "/n";
}
result += "Amount owed is " + getTotalCharge().ToString() + "/n";
result += "You earned " + getTotalFrequentRenterPoints().ToString() + "frequent renter points";
return result;
}
private double getTotalCharge()
{
double result = 0;
for (int index = 0; index < _rentals.Count; index++)
{
Rental each = (Rental)_rentals[index];
result += each.getCharge();
}
return result;
}
private int getTotalFrequentRenterPoints()
{
int result = 0;
for (int index = 0; index < _rentals.Count; index++)
{
Rental each = (Rental)_rentals[index];
result += each.getFrequentRenterPoints();
}
return result;
}
三：利用多态取代与价格相关的条件（SWITCH）逻辑，
1： 即使用的话，SWTICH数据也应该在对象自己的数据上使用，而非在别人的数据上使用。
public double getCharge(int daysRented)
{
double result = 0;
switch (getPriceCode())
{
case Movie.REGULAR:
result += 2;
if (daysRented > 2)
result += (daysRented - 2) * 1.5;
break;
case Movie.NEW_RELEASE:
result += daysRented * 3;
break;
case Movie.CHILDRENS:
result += 1.5;
if (daysRented > 3)
result += (daysRented - 3) * 1.5;
break;
}
return result;
}
public double getCharge()
{
return getMovie().getCharge(getDaysRented());
}
在重构一下
public double getCharge()
{
return _movie.getCharge(_daysRented);
}
public int getFrequentRenterPoints()
{
if (_movie.getPriceCode() == Movie.NEW_RELEASE && _daysRented > 1)
return 2;
else
return 1;
}
2：使用继承
abstract class Price
{
public abstract int getPriceCode();
public abstract double getCharge(int daysRented);
}
class ChildrenPrice : Price
{
public override int getPriceCode()
{
return Movie.CHILDRENS;
}
public override double getCharge(int daysRented)
{
double result = 1.5;
if (daysRented > 3)
result += (daysRented - 3) * 1.5;
return result;
}
}
class NewReleasePrice : Price
{
public override int getPriceCode()
{
return Movie.NEW_RELEASE;
}
public override double getCharge(int daysRented)
{
return daysRented * 3;
}
}
class RegularPrice : Price
{
public override int getPriceCode()
{
return Movie.REGULAR;
}
public override double getCharge(int daysRented)
{
double result = 2;
if (daysRented > 2)
result += (daysRented - 2) * 1.5;
return result;
}
}
class Movie
{
public const int CHILDRENS = 2;
public const int NEW_RELEASE = 1;
public const int REGULAR = 0;
private string _title;
private int _priceCode;
private Price _price;
public Movie(string title, int priceCode)
{
_title = title;
setPriceCode(priceCode);
}
public string getTitle()
{
return _title;
}
public int getPriceCode()
{
return _price.getPriceCode();
}
public void setPriceCode(int arg)
{
switch(arg)
{
case REGULAR:
_price = new RegularPrice();
break;
case CHILDRENS:
_price = new ChildrenPrice();
break;
case NEW_RELEASE:
_price = new NewReleasePrice();
break;
default:
throw new Exception();
}
}
public double getCharge(int daysRented)
{
return _price.getCharge(daysRented);
}
}
使用同样方法处理积点，继承只需要修改一个地方即可，其余不变动
public int getFrequentRenterPoints()
{
return _movie.getFrequentRenterPoints(_daysRented);
}
public int getFrequentRenterPoints(int daysRented)
{
return _price.getFrequentRenterPoints(daysRented);
}
abstract class Price
{
public virtual int getFrequentRenterPoints(int daysRented)
{
return 1;
}
}
class NewReleasePrice : Price
{
public override int getFrequentRenterPoints(int daysRented)
{
return (daysRented > 1) ? 2 : 1;
}
}
1179
被折叠的 条评论
为什么被折叠?
到【灌水乐园】发言
