重构——一个小例子
菜鸟区域,老鸟绕路!
原代码,这是一个可以借阅影片的小程序,你可以想象成某个大型系统,我想代码应该都能很容易看懂:
using System; using System.Collections.Generic; using System.Linq; using System.Web; namespace Reconsitution { public class Movie { /// <summary> /// 常规的 /// </summary> public const int Regular = 0; /// <summary> /// 新发布 /// </summary> public const int NewRelease = 1; /// <summary> /// 儿童版 /// </summary> public const int Childrens = 2; public string Title { get; private set; } public int PriceCode { get; set; } public Movie(string title, int priceCode) { Title = title; PriceCode = priceCode; } } public class Rental { public Movie @Movie { get; private set; } public int DaysRented { get; private set; } public Rental(Movie movie, int daysRented) { @Movie = movie; DaysRented = daysRented; } } public class Customer { public string Name { get; private set; } public List<Rental> Rentals { get { return _rentals; } } private readonly List<Rental> _rentals = new List<Rental>(); public Customer(string name) { Name = name; } public void AddRental(Rental rental) { _rentals.Add(rental); } public string Statement() { double totalAmount = 0; int frequentRenterPoints = 0; string result = "Rental Record for " + Name + "\n"; foreach (Rental rental in _rentals) { 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.NewRelease: thisAmount += rental.DaysRented*3; break; case Movie.Childrens: thisAmount += 1.5; if (rental.DaysRented > 3) thisAmount += (rental.DaysRented - 3) * 1.5; break; } frequentRenterPoints ++; if ((rental.Movie.PriceCode == Movie.NewRelease) && rental.DaysRented > 1) frequentRenterPoints ++; result += "\t" + rental.Movie.Title + "\t" + thisAmount + "\n"; totalAmount += thisAmount; } result += "Amount owed is " + totalAmount + "\n"; result += "You earned " + frequentRenterPoints + " frequent renter points"; return result; } } }
Customer customer = new Customer("Dennis Ding"); customer.AddRental(new Rental(new Movie("Water World", Movie.Childrens), 5)); customer.AddRental(new Rental(new Movie("Roman Holiday", Movie.Regular), 3)); customer.AddRental(new Rental(new Movie("The First Blood", Movie.NewRelease), 6)); TextBox1.Text = customer.Statement();
一、拆解主体方法
简要预览代码,我们很直观的发现 Customer 中的方法 Statement 过长,不便于阅读,于是我们首先便考虑解体方法 Statement,我们可以这样考虑,为什么要重构,重构再于便于后期维护和功能拓展,对于Statement方法,生成一个文字版的影片借阅结果单,如果我们版本更新,要生成一个Html的结果单,或者直接返回数据Json,形成接口供第三方调用呢,我们不得不重新写一个叫HtmlStatement 或者 JsonStatement的方法,因为我们把细节到计算的整个生成逻辑多放在了这一个方法中了!这是一种很不合理的设计!
public string Statement() { double totalAmount = 0; int frequentRenterPoints = 0; string result = "Rental Record for " + Name + "\n"; foreach (Rental rental in _rentals) { var thisAmount = AmountFor(rental); frequentRenterPoints ++; if ((rental.Movie.PriceCode == Movie.NewRelease) && rental.DaysRented > 1) frequentRenterPoints ++; result += "\t" + rental.Movie.Title + "\t" + thisAmount + "\n"; totalAmount += thisAmount; } result += "Amount owed is " + totalAmount + "\n"; result += "You earned " + frequentRenterPoints + " frequent renter points"; return result; } private static 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.NewRelease: thisAmount += rental.DaysRented*3; break; case Movie.Childrens: thisAmount += 1.5; if (rental.DaysRented > 3) thisAmount += (rental.DaysRented - 3)*1.5; break; } return thisAmount; }
这时候,我们发现,方法 AmountFor 并没有使用到和 Customer 相关的数据,但却放在 Customer 中,于是我们先考虑吧方法移到 Rental 中,
public class Rental { public Movie Movie { get; private set; } public int DaysRented { get; private set; } public Rental(Movie movie, int daysRented) { Movie = movie; DaysRented = daysRented; } public double AmountFor() { double thisAmount = 0; switch (this.Movie.PriceCode) { case Movie.Regular: thisAmount += 2; if (DaysRented > 2) thisAmount += (DaysRented - 2) * 1.5; break; case Movie.NewRelease: thisAmount += DaysRented * 3; break; case Movie.Childrens: thisAmount += 1.5; if (DaysRented > 3) thisAmount += (DaysRented - 3) * 1.5; break; } return thisAmount; } }
原 Statement 方法变成 :
public string Statement() { double totalAmount = 0; int frequentRenterPoints = 0; string result = "Rental Record for " + Name + "\n"; foreach (Rental rental in _rentals) { var thisAmount = rental.AmountFor(); frequentRenterPoints ++; if ((rental.Movie.PriceCode == Movie.NewRelease) && rental.DaysRented > 1) frequentRenterPoints ++; result += "\t" + rental.Movie.Title + "\t" + thisAmount + "\n"; totalAmount += thisAmount; } result += "Amount owed is " + totalAmount + "\n"; result += "You earned " + frequentRenterPoints + " frequent renter points"; return result; }
(为了让方法符合实际功能,我们可以把方法 AmountFor 命名为 GetCharge)
在方法Statement中,此时我们看到这样的代码
var thisAmount = rental.GetCharge(); //……被省略的代码 result += "\t" + rental.Movie.Title + "\t" + thisAmount + "\n"; totalAmount += thisAmount;
由于 rental.GetCharge() 被使用两次,为了避免二次计算,代码中使用了一个临时变量 thisAmount,于是乎就产生了一个《重构与性能》的话题,这是一个很大的话题,所以我暂时也没深入学习,在重构中,我们可以去掉这样的临时变量(一两个不可怕,但是在一个项目中过度的临时变量使得代码无比混乱),实际上我们去掉这样的临时变量,可以尝试从 Rental 类中寻求优化方法。至于《重构与性能》后面介绍!
代码如下:
//……被省略的代码
result += "\t" + rental.Movie.Title + "\t" + rental.GetCharge() + "\n";
totalAmount += rental.GetCharge();
再往后,Statement方法中,有这样的代码:
frequentRenterPoints ++; if ((rental.Movie.PriceCode == Movie.NewRelease) && rental.DaysRented > 1) frequentRenterPoints ++;
frequentRenterPoints 是一个在方法中定义的临时变量(阿飘),这小部分代码的作用也不过是计算积分(现在的各种商店都有积分),这两句代码只是积分的计算方式,和实际生成逻辑没有关联,于是我们考虑把它拆出来。
public string Statement() { double totalAmount = 0; int frequentRenterPoints = 0; string result = "Rental Record for " + Name + "\n"; foreach (Rental rental in _rentals) { frequentRenterPoints += GetFrequentRenterPoints(rental); result += "\t" + rental.Movie.Title + "\t" + rental.GetCharge() + "\n"; totalAmount += rental.GetCharge(); } result += "Amount owed is " + totalAmount + "\n"; result += "You earned " + frequentRenterPoints + " frequent renter points"; return result; } private int GetFrequentRenterPoints(Rental rental) { if ((rental.Movie.PriceCode == Movie.NewRelease) && rental.DaysRented > 1) return 2; return 1; }
接下来,我们接着处理“阿飘”的问题,对于临时变量,能少用则少用,这是某某牛人给出的重构名言一则。
代码中 totalAmount 的计算可以单独封装为:
private double GetTotalAmount() { return _rentals.Sum(rental => rental.GetCharge()); }
到此时,Statement方法已经比较简洁了,我们Copy再稍加修改就可以提供HtmlStatement和JsonStatement了,因为方法里面的计算逻辑已经被我们提取出来!
二、拆分后的方法调整
下一步,对于刚刚拆解的方法GetCharge,主体为一个switch,而switch却依赖对象 Movie的一个成员PriceCode,既然这样,为什么不把计算方式迁移到Movie类呢:
public double GetCharge() { double thisAmount = 0; switch (this.Movie.PriceCode) { case Movie.Regular: thisAmount += 2; if (DaysRented > 2) thisAmount += (DaysRented - 2) * 1.5; break; case Movie.NewRelease: thisAmount += DaysRented * 3; break; case Movie.Childrens: thisAmount += 1.5; if (DaysRented > 3) thisAmount += (DaysRented - 3) * 1.5; break; } return thisAmount; }
于是我们尝试在Movie中新建:
public double GetCharge(int daysRented) { double thisAmount = 0; switch (PriceCode) { case Movie.Regular: thisAmount += 2; if (daysRented > 2) thisAmount += (daysRented - 2) * 1.5; break; case Movie.NewRelease: thisAmount += daysRented * 3; break; case Movie.Childrens: thisAmount += 1.5; if (daysRented > 3) thisAmount += (daysRented - 3) * 1.5; break; } return thisAmount; }
原方法则可用:
public double GetCharge(int daysRented) { return Movie.GetCharge(DaysRented); }
然后看Customer中方法:
private int GetFrequentRenterPoints(Rental rental) { if ((rental.Movie.PriceCode == Movie.NewRelease) && rental.DaysRented > 1) return 2; return 1; }
public int GetFrequentRenterPoints() { if ((Movie.PriceCode == Movie.NewRelease) && DaysRented > 1) return 2; return 1; }
接着发现和上面同样使用了Movie的成员,尝试移动到Movie中:
public int GetFrequentRenterPoints(int daysRented) { if ((PriceCode == Movie.NewRelease) && daysRented > 1) return 2; return 1; }
Rental中可以这样:
public int GetFrequentRenterPoints() { return Movie.GetFrequentRenterPoints(DaysRented); }
和之前的GetTotalAmount方法一样,我们也可以有个GetTotalFrequentRenterPoints:
private double GetTotalFrequentRenterPoints() { return _rentals.Sum(rental => rental.GetFrequentRenterPoints()); }
于是Statement就表示为:
public string Statement() { string result = "Rental Record for " + Name + "\n"; foreach (Rental rental in _rentals) { result += "\t" + rental.Movie.Title + "\t" + rental.GetCharge() + "\n"; } result += "Amount owed is " + GetTotalAmount() + "\n"; result += "You earned " + GetTotalFrequentRenterPoints() + " frequent renter points"; return result; }
三、大的变化——继承
例子中,我们的Movie类分成三个类型
/// <summary> /// 常规的 /// </summary> public const int Regular = 0; /// <summary> /// 新发布 /// </summary> public const int NewRelease = 1; /// <summary> /// 儿童版 /// </summary> public const int Childrens = 2;
从而出现了方法 GetCharge 中计算的条件判断,当我们做同一件事使用不同的方式时候,我们想到了继承!对不同的电影类型,有不同的价格,我们认为价格是一个基类,每种电影的价格为子类,最终代码:
using System; using System.Collections.Generic; using System.Linq; using System.Web; namespace Reconsitution { public class Movie { #region 常量电影分类 /// <summary> /// 常规的 /// </summary> public const int Regular = 0; /// <summary> /// 新发布 /// </summary> public const int NewRelease = 1; /// <summary> /// 儿童版 /// </summary> public const int Childrens = 2; #endregion public string Title { get; private set; } public int PriceCode { get { return _price.GetPriceCode(); } private set { switch (value) { case Regular: _price = new RegularPrice(); break; case Childrens: _price = new ChidrensPrice(); break; case NewRelease: _price = new NewReleasePrice(); break; default: throw new Exception("Incorrect Price Code!"); } } } private Price _price ; public Movie(string title, int priceCode) { Title = title; PriceCode = priceCode; } public double GetCharge(int daysRented) { return _price.GetCharge(daysRented); } public int GetFrequentRenterPoints(int daysRented) { return _price.GetFrequentRenterPoints(daysRented); } } #region 价格类Price public abstract class Price { public abstract int GetPriceCode(); public abstract double GetCharge(int daysRented); public abstract int GetFrequentRenterPoints(int daysRented); } public class ChidrensPrice : Price { public override int GetPriceCode() { return Movie.Childrens; } public override double GetCharge(int daysRented) { double thisAmount = 1.5; if (daysRented > 3) thisAmount += (daysRented - 3) * 1.5; return thisAmount; } public override int GetFrequentRenterPoints(int daysRented) { return 1; } } public class NewReleasePrice : Price { public override int GetPriceCode() { return Movie.NewRelease; } public override double GetCharge(int daysRented) { double thisAmount = daysRented * 3; return thisAmount; } public override int GetFrequentRenterPoints(int daysRented) { return (daysRented > 1) ? 2 : 1; } } public class RegularPrice : Price { public override int GetPriceCode() { return Movie.Regular; } public override double GetCharge(int daysRented) { double thisAmount = 2; if (daysRented > 2) thisAmount += (daysRented - 2) * 1.5; return thisAmount; } public override int GetFrequentRenterPoints(int daysRented) { return 1; } } #endregion public class Rental { public Movie Movie { get; private set; } public int DaysRented { get; private set; } public Rental(Movie movie, int daysRented) { Movie = movie; DaysRented = daysRented; } public double GetCharge() { return Movie.GetCharge(DaysRented); } public int GetFrequentRenterPoints() { return Movie.GetFrequentRenterPoints(DaysRented); } } public class Customer { public string Name { get; private set; } public List<Rental> Rentals { get { return _rentals; } } private readonly List<Rental> _rentals = new List<Rental>(); public Customer(string name) { Name = name; } public void AddRental(Rental rental) { _rentals.Add(rental); } private double GetTotalAmount() { return _rentals.Sum(rental => rental.GetCharge()); } private double GetTotalFrequentRenterPoints() { return _rentals.Sum(rental => rental.GetFrequentRenterPoints()); } public string Statement() { string result = "Rental Record for " + Name + "\n"; foreach (Rental rental in _rentals) { result += "\t" + rental.Movie.Title + "\t" + rental.GetCharge() + "\n"; } result += "Amount owed is " + GetTotalAmount() + "\n"; result += "You earned " + GetTotalFrequentRenterPoints() + " frequent renter points"; return result; } } }
可能有人会问了, 为什么要这样做,是不是没有必要,我想你可以这样设想:正如之前提及的,对于同一一件事,如果使用的不同的方式去解决,就应该尝试通过继承分离出来,好处在于,如果我有新的电影分类出现,将只需要再继承一个Movie产生新的子类,就能解决问题,或者某类型电影价格、积分计算方式变动,我们也只用更改相应子类的计算方式,不影响整体方式!