一个C++程序重构的例子——糟糕的代码

由于工作中常用c++的原因，在看《Refactoring — Improving the Design of Exsiting Code》这本书时，将java的例子写成了c++程序，略做总结，以深理解。

重构的习惯与思想，跟语言无关。但c++是一个难掌控的语言，记下这些优化改进，希望上进的c++ programmer们，能实现日积跬步，终至千里的梦想。

一、待优化的程序

程序描述：

（1）我们将优化的是一个影片出租用的小程序，该程序会记录每个顾客的消费金额并打印出来。

（2）程序输入为：顾客租的影片及对应的租期；程序的处理为：根据顾客租用影片时间及影片类型，计算费用；输出：打印消费单。

（3）影片有三种类型：普通影片、儿童影片及新上映影片。

（4）另外，模仿时下潮流，程序还提供了积分制度，为常客计算积分，积分会根据影片是否为新上映影片而不同。

程序代码如下：

// 影片类
class Movie
{
public:
enum MovieType {
REGULAR = 0, CHILDREN, NEW_REALESE
};

Movie(const string &title, int type)
{
m_title = title;
m_type = type;
}

int GetType()
{
return m_type;
}

int SetType(int type)
{
m_type = type;
}

string GetTitle()
{
return m_title;
}

private:
// 影片名字
string m_title;

// 影片类型
int m_type;
};

 影片类是个纯数据类。 

// 租赁类
class Rental
{
public:
Rental(Movie *movie, int days_rented)
{
m_movie = movie;
m_days_rented = days_rented;
}

int GetDaysRented() const
{
return m_days_rented;
}

Movie * GetMovie() const
{
return m_movie;
}

private:
// 租赁的影片对象
Movie *m_movie;

// 租赁的天数
int m_days_rented;
};

Rental类表示某个顾客租了一部影片。

// 顾客类
class Customer
{
public:
Customer(const string &name)
{
m_name = name;
}

~Customer()
{
CleanUp();
}

void AddRental(Rental *rental)
{
m_rentals.push_back(rental);
}

string GetName() const
{
return m_name;
}

string GenerateReports();

private:
void CleanUp()
{
vector<Rental *>::iterator it = m_rentals.begin();
for (; it != m_rentals.end(); ++it)
{
delete (*it);
}
m_rentals.clear();
}

template <typename T>
string NumToString(T num)
{
stringstream s_stream;
s_stream << num;
return s_stream.str();
}

private:
// 顾客的名字
string m_name;

// 顾客租赁的所有影片
vector<Rental *> m_rentals;
};

Customer类用来表示一个顾客。GenerateReports（）函数用来生成某个顾客租赁影片的详细报表。

这三个类的UML类图描述如下：



 GenerateReports（）函数的完整代码如下：

string Customer::GenerateReports()
{
// 租赁消费总额
double total_amount = 0;
// 积分
int renter_points = 0;

// 返回的字符串形式的报表
string result = "Retal Record for " + GetName() + "\n";
vector<Rental *>::const_iterator it = m_rentals.begin();
for (; it != m_rentals.end(); ++it)
{
Rental *a_rental = *it;
double amount = 0;

// 计算每个租赁的消费额
switch (a_rental->GetMovie()->GetType())
{
case Movie::REGULAR:
amount += 2;
if (a_rental->GetDaysRented() > 2)
{
amount += (a_rental->GetDaysRented() - 2) * 1.5;
}
break;
case Movie::NEW_REALESE:
amount += a_rental->GetDaysRented() * 3;
break;
case Movie::CHILDREN:
amount += 1.5;
if (a_rental->GetDaysRented() > 3)
{
amount += (a_rental->GetDaysRented() - 3) * 1.5;
}
break;
default:
break;
}

// 添加积分
++renter_points;

// 对两天以上的新片给以奖励积分
if (a_rental->GetMovie()->GetType() == Movie::NEW_REALESE
&& a_rental->GetDaysRented() > 1)
{
++renter_points;
}

// 将本次租赁信息添加到results中
result += "\t" + a_rental->GetMovie()->GetTitle() + "\t"
+ NumToString(amount) + "\n";
total_amount += amount;
}

// 添加总消费额及积分
result += "Amount is " + NumToString(total_amount) + "\n";
result += "You earned " + NumToString(renter_points) + " frequent renter points.";

return result;
}

GenerateReports函数的交互过程如下：



二、代码的评价

Martin Fowler 对上述代码做了如下评价：

（1）上述代码不符合面向对象精神。

（2）对于一个独立的小程序，快速而简单的设计一个程序并没有错；但如果上述代码是复杂系统中的一小部分，这段代码令人堪忧。

（3）Customer类里那个长长的GenerateReports函数承担了太多职责，做了很多本该由其他类完成的工作。

（4）编译器不会在乎上述的丑陋代码，它能正常完成工作，但是，当我们打算修改系统的时候，就涉及到了人，人会在乎上述代码的丑陋，因为人会夹杂美学上的判断。

（5）差劲的系统是很难修改的，因为很难找到修改点，如果很难找到修改点，程序员就很可能犯错，引入bug。 

另外，如果需求变化了，上述代码会怎样？

（1）在web时代，如果用户希望输出HTML报表呢？

    你会发现，在打印HTML报表的函数中，你根本无法重用GenerateReports函数的任何行为。

    你唯一可做的是，编写一个全新的GenerateHtmlReports函数，重复GenerateReports的行为，当然，这种改变好像也不怎么费力，也许你很快就完成了！

（2）如果计费标准发生了变化，譬如租用新影片的计费发生变化，该怎么办呢？

    这个时候，你必须同时修改GenerateHtmlReports（）和GenerateReports（），而且要保证对两个函数修改的一致性。

    当后续需求变化还需要修改时，ctrl+c和ctrl+v的带来的问题就来了，假若你编写的是一个永不修改程序也罢，但如果程序的生命周期很长，就可能会维护，会面临修改，那么复制和粘贴就会造成潜在的威胁。

（3）很快，需求变更又来了，用户决定改变影片的分类规则，但是还没有决定如何修改。

   不管分类规则如何变化，总会影响计费规则和积分规则，就必须对GenerateReports的函数做出修改。

   随着规则越来越复杂，两个函数的一致性越来越难保证，不犯错的机会将会越来越少。

三、重构

也许，你倾向于尽量少的修改程序，因为这段代码还可以工作，它并没有挂掉，但是，虽然它没有挂掉，它伤了程序员的心。它使程序员的生活变得难过，因为你发现，当你需要满足客户的修改需求时，你越来越难完成客户的需求......

相信很多程序员都有这样的经历，这个时候，就是需要重构技术登场了！