重构——一个小例子

菜鸟区域,老鸟绕路!

原代码,这是一个可以借阅影片的小程序,你可以想象成某个大型系统,我想代码应该都能很容易看懂:

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;
}

我们可以迁移到Rental中:

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产生新的子类,就能解决问题,或者某类型电影价格、积分计算方式变动,我们也只用更改相应子类的计算方式,不影响整体方式!

 

posted @ 2016-07-21 10:10  程序员丁  阅读(1223)  评论(0编辑  收藏  举报