此文为译文，原文地址请点击。
本文通过重构一个垃圾代码，阐述了如何写出优秀的代码。开发人员及代码审核人员需按照此规范开发和审核代码。此规范以C#为例，JAVA的童鞋一并参考，C++的童鞋自行脑补吧。
简介
这篇文章的目的是展示如何将一段垃圾代码重构成一个干净的、可扩展性和可维护的代码。我将解释如何通过最佳实践和更好的设计模式来改写它。
阅读本文你需要有以下基础：
- c# 基础
- 依赖注入，工厂模式，策略模式
此文中的例子源于实际项目，这里不会有什么使用装饰模式构建的披萨，也不会使用策略模式的计算器，这些例子是非常好的说明，但是它们很难被在实际项目中使用。
一段垃圾代码
在我们真实的产品中有这么一个类：
public class Class1
{
public decimal Calculate(decimal amount, int type, int years)
{
decimal result = 0;
decimal disc = (years > 5) ? (decimal)5/100 : (decimal)years/100;
if (type == 1)
{
result = amount;
}
else if (type == 2)
{
result = (amount - (0.1m * amount)) - disc * (amount - (0.1m * amount));
}
else if (type == 3)
{
result = (0.7m * amount) - disc * (0.7m * amount);
}
else if (type == 4)
{
result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount));
}
return result;
}
}
这是一段非常糟糕的代码（二胡：如果你没觉得这段代码很糟糕，那你目前状态可能很糟糕了），我们不太清楚这个类的目的是什么。它可能是一个网上商城的折扣管理类，负责为客户计算折扣。
这个类完全具备了不可读、不可维护、不可扩展的特点，它使用了很多坏的实践和反模式的设计。
下面我们逐步分析这里到底有多少问题？
命名问题 - 我们只能通过猜测这个类到底是为了计算什么。这实在是在浪费时间。
如果我们没有相关文档或者重构这段代码，那我们下一次恐怕需要花大量的时间才能知道这段代码的具体含义。魔数 - 在这个例子中，你能猜到变量type是指客户账户的状态吗。通过if-else来选择计算优惠后的产品价格。
现在，我们压根不知道账户状态1，2，3，4分别是什么意思。
此外，你知道0.1，0.7，0.5都是什么意思吗？
让我们想象一下，如果你要修改下面这行代码：result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount));隐藏的BUG - 因为代码非常糟糕，我们可能会错过非常重要的事情。试想一下，如果我们的系统中新增了一类账户状态，而新的账户等级不满足任何一个if-else条件。这时，返回值会固定为0。
不可读 - 我们不得不承认，这是一段不可读的代码。不可读=更多的理解时间+增加产生错误的风险
DRY – 不要产生重复的代码
我们可能不能一眼就找出它们，但它们确实存在。
例如：disc *(amount - (0.1m * amount));
同样的逻辑：disc *(amount - (0.5m * amount));
这里只有一个数值不一样。如果我们无法摆脱重复的代码，我们会遇到很多问题。比如某段代码在五个地方有重复，当我们需要修改这部分逻辑时，你很可能只修改到了2至3处。单一职责原则
这个类至少具有三个职责：
1 选择计算算法
2 根据账户状态计算折扣
3 根据账户网龄计算折扣
它违反了单一职责原则。这会带来什么风险?如果我们将要修改第三个功能的话，会影响到另外第二个功能。这就意味着，我们每次修改都会改动我们本不想修改的代码。因此，我们不得不对整个类进行测试，这实在很浪费时间。
重构
通过以下9布，我会告诉你们如何避免上述风险并实现一个干净的、可维护的、可测试的代码。
-
命名，命名，命名
这是良好代码的最重要方面之一。我们只需要改变方法，参数和变量的命名。现在，我们可以确切的知道下面的类是负责什么的了。public decimal ApplyDiscount(decimal price, int accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100;
if (accountStatus == 1)
{
priceAfterDiscount = price;
}
else if (accountStatus == 2)
{
priceAfterDiscount = (price - (0.1m * price)) - (discountForLoyaltyInPercentage * (price - (0.1m * price)));
}
else if (accountStatus == 3)
{
priceAfterDiscount = (0.7m * price) - (discountForLoyaltyInPercentage * (0.7m * price));
}
else if (accountStatus == 4)
{
priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price)));
} return priceAfterDiscount;
}
但是我们任然不知道账户状态1，2，3到底是什么意思。
-
魔数
在C#中避免魔数我们一般采用枚举来替换它们。这里使用AccountStatus 枚举来替换if-else中的魔数。public enum AccountStatus { NotRegistered = 1, SimpleCustomer = 2, ValuableCustomer = 3, MostValuableCustomer = 4 }
现在我们来看看重构后的类，我们可以很容易的说出哪一个账户状态应该用什么算法来计算折扣。混合账户状态的风险迅速的下降了。public class DiscountManager
{
public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100; if (accountStatus == AccountStatus.NotRegistered)
{
priceAfterDiscount = price;
}
else if (accountStatus == AccountStatus.SimpleCustomer)
{
priceAfterDiscount = (price - (0.1m * price)) - (discountForLoyaltyInPercentage * (price - (0.1m * price)));
}
else if (accountStatus == AccountStatus.ValuableCustomer)
{
priceAfterDiscount = (0.7m * price) - (discountForLoyaltyInPercentage * (0.7m * price));
}
else if (accountStatus == AccountStatus.MostValuableCustomer)
{
priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price)));
}
return priceAfterDiscount;
}
} -
更多的代码可读性
在这一步中，我们使用switch-case 来替换 if-else if来增强代码可读性。
同时，我还将某些长度很长的语句才分成两行。比如：**priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price)));**
被修改为：**priceAfterDiscount = (price - (0.5m * price));priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);**
以下是完整的修改：public class DiscountManager
{
public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100;
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price;
break;
case AccountStatus.SimpleCustomer:
priceAfterDiscount = (price - (0.1m * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
case AccountStatus.ValuableCustomer:
priceAfterDiscount = (0.7m * price);
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = (price - (0.5m * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
}
return priceAfterDiscount;
}
} -
消除隐藏的BUG
正如我们之前提到的，我们的ApplyDiscount方法可能将为新的客户状态返回0。
我们怎样才能解决这个问题？答案就是抛出NotImplementedException。
当我们的方法获取账户状态作为输入参数，但是参数值可能包含我们未设计到的未知情况。这时，我们不能什么也不做，抛出异常是这时候最好的做法。public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100;
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price;
break;
case AccountStatus.SimpleCustomer:
priceAfterDiscount = (price - (0.1m * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
case AccountStatus.ValuableCustomer:
priceAfterDiscount = (0.7m * price);
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = (price - (0.5m * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
default:
throw new NotImplementedException();
}
return priceAfterDiscount;
} 分析算法
在这个例子中，我们通过两个标准来计算客户折扣：
账户状态
-
账户网龄
通过网龄计算的算法都类似这样：(discountForLoyaltyInPercentage * priceAfterDiscount)
但是对于账户状态为ValuableCustomer的算法却是：0.7m * price
我们把它修改成和其他账户状态一样的算法：price - (0.3m * price)public class DiscountManager
{
public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5 / 100 : (decimal)timeOfHavingAccountInYears / 100;
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price;
break;
case AccountStatus.SimpleCustomer:
priceAfterDiscount = (price - (0.1m * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
case AccountStatus.ValuableCustomer:
priceAfterDiscount = (price - (0.3m * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = (price - (0.5m * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
default:
throw new NotImplementedException();
}
return priceAfterDiscount;
}
}
-
消除魔数的另一种方法
使用静态常量来替换魔数。0.1m,0.2m,0.3m,我m，我们并不知道它们是什么意思。
此外decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5/100 : (decimal)timeOfHavingAccountInYears/100;中，数字5也非常神秘。
我们必须让它们更具有描述性，这时使用常量会是一个比较好的办法。
我们来定义一个静态类：public static class Constants
{
public const int MAXIMUM_DISCOUNT_FOR_LOYALTY = 5;
public const decimal DISCOUNT_FOR_SIMPLE_CUSTOMERS = 0.1m;
public const decimal DISCOUNT_FOR_VALUABLE_CUSTOMERS = 0.3m;
public const decimal DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS = 0.5m;
}
接着修改DiscountManager类：
public class DiscountManager
{
public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY / 100 : (decimal)timeOfHavingAccountInYears / 100;
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price;
break;
case AccountStatus.SimpleCustomer:
priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
case AccountStatus.ValuableCustomer:
priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
break;
default:
throw new NotImplementedException();
}
return priceAfterDiscount;
}
}
-
消除重复的代码
为了消除重复的代码，这里将部分算法提取出来。首先，我们建立两个扩展方法：public static class PriceExtensions
{
public static decimal ApplyDiscountForAccountStatus(this decimal price, decimal discountSize)
{
return price - (discountSize * price);
} public static decimal ApplyDiscountForTimeOfHavingAccount(this decimal price, int timeOfHavingAccountInYears)
{
decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY / 100 : (decimal)timeOfHavingAccountInYears / 100;
return price - (discountForLoyaltyInPercentage * price);
}
}
通过方法名称，我们就可以知道它的职责是什么，现在修改我们的例子：
public class DiscountManager
{
public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price;
break;
case AccountStatus.SimpleCustomer:
priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS)
.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
break;
case AccountStatus.ValuableCustomer:
priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS)
.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
break;
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS)
.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
break;
default:
throw new NotImplementedException();
}
return priceAfterDiscount;
}
}
-
删除没用的代码
我们应该写出简短的代码，因为简短的代码=减少BUG发生的机率，并且也让我们缩短理解业务逻辑的时间。
我们发现，这里三种状态的客户调用了相同的方法：
.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
这里可以合并代码：public class DiscountManager
{
public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price;
break;
case AccountStatus.SimpleCustomer:
priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS);
break;
case AccountStatus.ValuableCustomer:
priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS);
break;
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS);
break;
default:
throw new NotImplementedException();
}
priceAfterDiscount = priceAfterDiscount.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
return priceAfterDiscount;
}
}
9.最后，得到干净的代码
最后，让我们通过引入依赖注入和工厂方法模式来得到最终的版本吧。
先来看卡最终结果：
public class DiscountManager
{
private readonly IAccountDiscountCalculatorFactory _factory;
private readonly ILoyaltyDiscountCalculator _loyaltyDiscountCalculator;
public DiscountManager(IAccountDiscountCalculatorFactory factory, ILoyaltyDiscountCalculator loyaltyDiscountCalculator)
{
_factory = factory;
_loyaltyDiscountCalculator = loyaltyDiscountCalculator;
}
public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price);
priceAfterDiscount = _loyaltyDiscountCalculator.ApplyDiscount(priceAfterDiscount, timeOfHavingAccountInYears);
return priceAfterDiscount;
}
}
public interface ILoyaltyDiscountCalculator
{
decimal ApplyDiscount(decimal price, int timeOfHavingAccountInYears);
}
public class DefaultLoyaltyDiscountCalculator : ILoyaltyDiscountCalculator
{
public decimal ApplyDiscount(decimal price, int timeOfHavingAccountInYears)
{
decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY / 100 : (decimal)timeOfHavingAccountInYears / 100;
return price - (discountForLoyaltyInPercentage * price);
}
}
public interface IAccountDiscountCalculatorFactory
{
IAccountDiscountCalculator GetAccountDiscountCalculator(AccountStatus accountStatus);
}
public class DefaultAccountDiscountCalculatorFactory : IAccountDiscountCalculatorFactory
{
public IAccountDiscountCalculator GetAccountDiscountCalculator(AccountStatus accountStatus)
{
IAccountDiscountCalculator calculator;
switch (accountStatus)
{
case AccountStatus.NotRegistered:
calculator = new NotRegisteredDiscountCalculator();
break;
case AccountStatus.SimpleCustomer:
calculator = new SimpleCustomerDiscountCalculator();
break;
case AccountStatus.ValuableCustomer:
calculator = new ValuableCustomerDiscountCalculator();
break;
case AccountStatus.MostValuableCustomer:
calculator = new MostValuableCustomerDiscountCalculator();
break;
default:
throw new NotImplementedException();
}
return calculator;
}
}
public interface IAccountDiscountCalculator
{
decimal ApplyDiscount(decimal price);
}
public class NotRegisteredDiscountCalculator : IAccountDiscountCalculator
{
public decimal ApplyDiscount(decimal price)
{
return price;
}
}
public class SimpleCustomerDiscountCalculator : IAccountDiscountCalculator
{
public decimal ApplyDiscount(decimal price)
{
return price - (Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS * price);
}
}
public class ValuableCustomerDiscountCalculator : IAccountDiscountCalculator
{
public decimal ApplyDiscount(decimal price)
{
return price - (Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS * price);
}
}
public class MostValuableCustomerDiscountCalculator : IAccountDiscountCalculator
{
public decimal ApplyDiscount(decimal price)
{
return price - (Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS * price);
}
}
首先，我们摆脱了扩展方法（静态类），如果我们想对ApplyDiscount方法进行单元测试是比较困难的，废除我们对PriceExtensions扩展类也进行测试。
为了避免这个问题，我们创建了DefaultLoyaltyDiscountCalculator 类来替换ApplyDiscountForTimeOfHavingAccount这个扩展方法，此类还实现了ILoyaltyDiscountCalculator接口。现在，当我们要测试DiscountManager类时，我们只构造函数注入ILoyaltyDiscountCalculator接口的实现即可。这里使用了依赖注入。
通过这样做，我们将网龄折扣的算法迁移到类似DefaultLoyaltyDiscountCalculator 的不同类中，这样当我们修改某一个算法不会覆盖到其他业务。
对于根据账户状态来计算折扣的业务，我们需要在DiscountManager中删除两个职责：
根据账户状态选择计算的算法
实现计算算法
这里我们通过DefaultAccountDiscountCalculatorFactory工厂类来解决这个问题，DefaultAccountDiscountCalculatorFactory工厂类实现了IAccountDiscountCalculatorFactory接口。
我们的工厂将决定选择哪一个折扣算法。接着，工厂类被通过构造函数注入到DiscountManager类中。
下面我只需要在DiscountManager 中使用工厂：priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price);
以上，我们解决了第一个问题，下面我们需要实现计算算法。根据账户状态，提供不同的算法，这正好符合策略模式。我们需要构建三个策略：NotRegisteredDiscountCalculator，SimpleCustomerDiscountCalculator，MostValuableCustomerDiscountCalculator已经抽象出来的接口IAccountDiscountCalculator。
好了，现在我们有可一段干净可读的代码了，这段代码中所有的类都只有一个职责：DiscountManager - 管理
DefaultLoyaltyDiscountCalculator - 网龄计算折扣
DefaultAccountDiscountCalculatorFactory - 根据账户状态选择折扣策略
-
NotRegisteredDiscountCalculator,SimpleCustomerDiscountCalculator,MostValuableCustomerDiscountCalculator-计算折扣算法
我们来比较一下修改前后的代码：public class Class1
{
public decimal Calculate(decimal amount, int type, int years)
{
decimal result = 0;
decimal disc = (years > 5) ? (decimal)5 / 100 : (decimal)years / 100;
if (type == 1)
{
result = amount;
}
else if (type == 2)
{
result = (amount - (0.1m * amount)) - disc * (amount - (0.1m * amount));
}
else if (type == 3)
{
result = (0.7m * amount) - disc * (0.7m * amount);
}
else if (type == 4)
{
result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount));
}
return result;
}
}
修改后：
public class DiscountManager
{
private readonly IAccountDiscountCalculatorFactory _factory;
private readonly ILoyaltyDiscountCalculator _loyaltyDiscountCalculator;
public DiscountManager(IAccountDiscountCalculatorFactory factory, ILoyaltyDiscountCalculator loyaltyDiscountCalculator)
{
_factory = factory;
_loyaltyDiscountCalculator = loyaltyDiscountCalculator;
}
public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
{
decimal priceAfterDiscount = 0;
priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price);
priceAfterDiscount = _loyaltyDiscountCalculator.ApplyDiscount(priceAfterDiscount, timeOfHavingAccountInYears);
return priceAfterDiscount;
}
}
总结
本文通过简单易懂的方法重构了一段问题代码，它显示了如何在实际情况中使用最佳实践和设计模式来帮助我们写出干净的代码。
就我的工作经验来说，本文中出现的不良做法是经常发生的。编写这种代码的人总是认为他们能够保持这种规则，但不幸的是系统和业务往往都会越来越复杂，每次修改这类代码时都会带来巨大的风险。