[翻译]C# BAD PRACTICES: Learn how to make a good code by bad example---C#:如何将坏的代码重新编译为好的代码

自己的前言说明:

 本文原作者:Radoslaw Sadowski,原文链接为:C# BAD PRACTICES: Learn how to make a good code by bad example

本系列还有其他文章,后续将慢慢翻译。

 

引言:

我的名字叫Radoslaw Sadowski,我现在是一个微软技术开发人员。我从开始工作时就一直接触的微软技术.

在工作一年后,我看到的质量很差的代码的数量基本上都可以写成一整本书了。

这些经历让我变成了一个想要清洁代码的强迫症患者。

写这篇文章的目的是为了通过展示质量很差的类的例子来说明如何书写出干净的、可延伸的和可维护的代码。我会通过好的书写方式和设计模式来解释坏的代码带来的问题,以及替换他的好的解决方法。

第一部分是针对那些拥有C#基础知识的开发人员——我会展示一些常见的错误,然后再展示一些让代码变得可读性的方法与技巧。高级部分主要针对那些至少拥有设计模式概念的开发人员——我将会展示完全干净的、单元可测试的代码。

为了能够理解这篇文章你需要至少掌握以下两个部分的基本知识:

  • C#语言
  • 依赖注入、工厂设计模式、策略设计模式

本文中所涉及的例子都将会是现实中实实在在的具体的特性,而不是用装饰模式来做披萨或者用策略模式来做计算器这样的示例。

(ps解释:看过设计模式相关的书籍的人应该会知道很多这方面的书籍都是用这种例子,只是为了帮助读者理解设计模式)

                                        

因为我发现这种类型的产品不好用来解释,相反这些理论性的例子却是非常适合用来在本文中做解释的。

我们常常会听到说不要用这个,要用那个,但是却不知道这种替换的理由。今天我将会努力解释和证明那些好的书写习惯以及设计模式是真的是在拯救我们的开发生活!

 提示:

  •  在本文中我不会花时间来讲解C#的特性和涉及模式之类(我也解释不完),网上有很多关于这方面的好的理论的例子。我将集中讲述如何在我们日常工作中使用这些东西。
  • 例子是一种比较容易的突出我们要说明的问题的方法,但是仅限于描述的问题——因为我发现当我在学习哪些包含着主要代码的例子时,我发现在理解文章的总体思想方面会有困难。
  •  我不是说我文中说的方法是惟一的解决方式,我只是能保证这些方法将会是让你的代码变得更高质量的途径。
  • 我并不关心下面这些代码的什么错误处理,日志记录等等。我要表述的只是用来解决日常编码一些问题的方法。

那就开始吧….

那些糟糕透了的类...

下面的例子是我们现实中的类:

 1 public class Class1
 2 {
 3   public decimal Calculate(decimal amount, int type, int years)
 4   {
 5     decimal result = 0;
 6     decimal disc = (years > 5) ? (decimal)5/100 : (decimal)years/100; 
 7     if (type == 1)
 8     {
 9       result = amount;
10     }
11     else if (type == 2)
12     {
13       result = (amount - (0.1m * amount)) - disc * (amount - (0.1m * amount));
14     }
15     else if (type == 3)
16     {
17       result = (0.7m * amount) - disc * (0.7m * amount);
18     }
19     else if (type == 4)
20     {
21       result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount));
22     }
23     return result;
24   }
25 }

上面这个例子真的是一种非常差的书写方式。你能知道这个类是用来干嘛的么?这个东西是用来做一些奇怪的运算的么?我们文章就从他开始入手来讲解吧

现在我来告诉你,刚刚那个类是用来当顾客在网上买东西的时候为他们计算对应折扣的折扣计算和管理的类。

-难以置信吧!

-可是这是真的!

这种写法真的是将难以阅读、难以维护和难以扩展这三种集合在一起了,而且拥有着太差的书写习惯和错误的模式。

除此之外还有其他什么问题么?

1.命名方式-从源代码中我们可以连蒙带猜估计出来这个计算方法和输出结果是什么。而且我们想要从这个类中提取计算算法将会是一件非常困难的事情。

这样带来的危害是:

最严重的问题是:浪费时间,

 

如果我们需要满足客户的商业咨询,要像他们展示算法细节,或者我们需要修改这段代码,这将花费我们很长的时间去理解我们的计算方法的逻辑。即使我们不记录他或重构代码,下次我们/其他开发人员再看这段代码的时候,还是需要花费同等的时间来研究这些代码是干嘛的。而且在修改的同时还容易出错,导致原来的计算全部出错。

 2.魔法数字

 

在这个例子中type是变量,你能猜到它代表着客户账户的等级么?If-else if语句是用来实现如何选择计算出产品价格折扣的方法。

现在我们不知道什么样的账户是1,2,3或4。现在想象一下,当你不得不为了那些有价值的VIP客户改变他们的折扣计算方式的时候,你试着从那些代码中找出修改的方法---这个过程可能会花费你很长的时间不说,还很有可能犯错以至于修改那些基础的一般的客户的账户,毕竟像2或者3这些词语毫无描述性的。但是在我们犯错以后,那些一般的客户却很高兴,因为他们得到了VIP客户的折扣。:)

3.没有明显的bug

因为我们的代码质量很差,而且可读性非常差,所以我们可能轻易就忽略掉很多非常重要的事情。想象一下,现在突然在系统中增加一种新的客户类型-金卡用户,而在我们的系统中任何一种新的账户类型最后获得的价格将是0元。为什么呢?因为在我们的if-else if语句中没有任何状态是满足新的状态的,所以只要是未处理过的账户类型,最后返回值都将变成0。一旦我们的老板发现这件事,他将会大发雷霆-毕竟他已经免费卖给这样用户很多很多东西了!

4.没有可读性

我们必须承认上面这段代码的可读性是真的糟糕。

她让我们花费了太多的时间去理解这段代码,同时代码隐藏错误的几率太大了,而这就是没有可读性的最重要的定义。

 5.魔法数字(再次)

你从代码中能知道类似0.1,0.7,0.5这些数字的意思么?好的,我承认我不知道。只有我们自己编写这些代码我们才知道这是什么意思,别人是无法理解的。

你试试想想如果让你修改下面这句代码,你会怎么样:

result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount));

因为这个方法完全不可读,所以你修改的过程中只能尝试着把第一个0.5改成0.4而保持第二个0.5不懂。这可能会是一个bug,但是却是最好的最合适的修改方式。因为这个0.5什么都没有告诉我们。

同样的事也存在将years变量转换到disc变量的转换过程中

decimal disc = (years > 5) ? (decimal)5/100 : (decimal)years/100;

这是用来计算折扣率的,会通过账户在我们系统的时间的百分比来获取。好的,那么现在问题来了,如果时间刚刚好就是5呢?

6.简洁-不要反复做无用功

虽然第一眼看的时候不容易看出来,但是仔细研究一下就会发现:我们的代码里有很多重复的地方。例如:disc * (amount - (0.1m * amount));

而与之有同样效果的还有(只是变了一个参数而已):disc * (amount - (0.5m * amount))

在这两个算术中,唯一的区别就只是一个静态参数,而我们完全可以用一个可变的参数来替代。

如果我们不试着在写代码的时候从一直ctri+c,ctrl+v中摆脱出来,那我们将遇到的问题就是我们只能修改代码中的部分功能,因为我们不知道有多少地方需要修改。上面的逻辑是计算出在我们系统中每个客户对应年限获得的折扣,所以如果我们只是贸然修改两到三处,很容易造成其他地方的前后不一致。

7.每个类有着太多的复杂的责任区域

我们写的类至少背负了三个责任:

  1. 选择计算的运算法则
  2. 为每个不同状态的账户计算折扣率
  3. 根据每个客人的年限计算出对应的折扣率

这个违背了单一责任原则。那么这会带来什么危害呢?如果我们想要改变上诉3个特性中的两个,那就意味着可能会碰触到一些其他的我们并不想修改的特性。所以在修改的时候我们不得不重新测试所有的类,那么这就造成了很重的时间的浪费。

那就开始重构吧…

在接下来的9个步骤中我将向你展示我们如何避免上诉问题来构建一个干净的易维护,同时又方便单元测试的看起来一目了然的代码。

 

I:命名,命名,命名

恕我直言,这是代码中最重要的一步。我们只是修改方法/参数/变量这些的名字,而现在我们可以直观的了解到下面这个类代表什么意思。

 1 public class DiscountManager
 2 {
 3   public decimal ApplyDiscount(decimal price, int accountStatus, int timeOfHavingAccountInYears)
 4   {
 5     decimal priceAfterDiscount = 0;
 6     decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5/100 : (decimal)timeOfHavingAccountInYears/100; 
 7     if (accountStatus == 1)
 8     {
 9       priceAfterDiscount = price;
10     }
11     else if (accountStatus == 2)
12     {
13       priceAfterDiscount = (price - (0.1m * price)) - (discountForLoyaltyInPercentage * (price - (0.1m * price)));
14     }
15     else if (accountStatus == 3)
16     {
17       priceAfterDiscount = (0.7m * price) - (discountForLoyaltyInPercentage * (0.7m * price));
18     }
19     else if (accountStatus == 4)
20     {
21       priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price)));
22     }
23  
24     return priceAfterDiscount;
25   }
26 }

虽然如此,我们还是不理解1,2,3,4代表着什么,那就继续往下吧!

II:魔法数

C#中避免出现不理解的魔法数的方法是通过枚举来替代。我通过枚举方法来替代在if-else if 语句中出现的代替账户状态的魔法数。

1 public enum AccountStatus
2 {
3   NotRegistered = 1,
4   SimpleCustomer = 2,
5   ValuableCustomer = 3,
6   MostValuableCustomer = 4
7 }

现在在看我们重构了的类,我们可以很容易的说出那个计算法则是用来根据不用状态来计算折扣率的。将账户状态弄混的几率就大幅度减少了。

 1 public class DiscountManager
 2 {
 3   public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
 4   {
 5     decimal priceAfterDiscount = 0;
 6     decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5/100 : (decimal)timeOfHavingAccountInYears/100;
 7  
 8     if (accountStatus == AccountStatus.NotRegistered)
 9     {
10       priceAfterDiscount = price;
11     }
12     else if (accountStatus == AccountStatus.SimpleCustomer)
13     {
14       priceAfterDiscount = (price - (0.1m * price)) - (discountForLoyaltyInPercentage * (price - (0.1m * price)));
15     }
16     else if (accountStatus == AccountStatus.ValuableCustomer)
17     {
18       priceAfterDiscount = (0.7m * price) - (discountForLoyaltyInPercentage * (0.7m * price));
19     }
20     else if (accountStatus == AccountStatus.MostValuableCustomer)
21     {
22       priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price)));
23     }
24     return priceAfterDiscount;
25   }
26 }

III:更多的可读性

在这一步中我们将通过将if-else if 语句改为switch-case 语句,来增加文章的可读性。

同时,我也将一个很长的计算方法拆分为两句话来写。现在我们将“ 通过账户状态来计算折扣率”与“通过账户年限来计算折扣率”这两者分开来计算。

例如:priceAfterDiscount = (price - (0.5m * price)) - (discountForLoyaltyInPercentage * (price - (0.5m * price)));

我们将它重构为:priceAfterDiscount = (price - (0.5m * price));
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);

这就是修改后的代码:

 1 public class DiscountManager
 2 {
 3   public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
 4   {
 5     decimal priceAfterDiscount = 0;
 6     decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5/100 : (decimal)timeOfHavingAccountInYears/100;
 7     switch (accountStatus)
 8     {
 9       case AccountStatus.NotRegistered:
10         priceAfterDiscount = price;
11         break;
12       case AccountStatus.SimpleCustomer:
13         priceAfterDiscount = (price - (0.1m * price));
14         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
15         break;
16       case AccountStatus.ValuableCustomer:
17         priceAfterDiscount = (0.7m * price);
18         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
19         break;
20       case AccountStatus.MostValuableCustomer:
21         priceAfterDiscount = (price - (0.5m * price));
22         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
23         break;
24     }
25     return priceAfterDiscount;
26   }
27 }

IV:没有明显的bug

我们终于找到我们隐藏的bug了!

因为我刚刚提到的我们的方法中对于不适合的账户状态会在造成对于所有商品最后都返回0。虽然很不幸,但却是真的。

那我们该如何修复这个问题呢?那就只有通过没有错误提示了。

你是不是会想,这个会不会是开发的例外,应该不会被提交到错误提示中去?不,他会的!

当我们的方法通过获取账户状态作为参数的时候,我们并不想程序让我们不可预知的方向发展,造成不可预计的失误。

 这种情况是绝对不允许出现的,所以我们必须通过抛出异常来防止这种情况。

下面的代码就是通过抛出异常后修改的以防止出现不满足条件的情况-修改方式是将抛出异常防止 switch-case语句中的default 句中。

 1 public class DiscountManager
 2 {
 3   public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
 4   {
 5     decimal priceAfterDiscount = 0;
 6     decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5/100 : (decimal)timeOfHavingAccountInYears/100;
 7     switch (accountStatus)
 8     {
 9       case AccountStatus.NotRegistered:
10         priceAfterDiscount = price;
11         break;
12       case AccountStatus.SimpleCustomer:
13         priceAfterDiscount = (price - (0.1m * price));
14         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
15         break;
16       case AccountStatus.ValuableCustomer:
17         priceAfterDiscount = (0.7m * price);
18         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
19         break;
20       case AccountStatus.MostValuableCustomer:
21         priceAfterDiscount = (price - (0.5m * price));
22         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
23         break;
24       default:
25         throw new NotImplementedException();
26     }
27     return priceAfterDiscount;
28   }
29 }

V:分析计算方法

在我们的例子中我们有两个定义给客户的折扣率的标准:

  1. 账户状态;
  2. 账户在我们系统中存在的年限

对于年限的计算折扣率的方法,所有的计算方法都有点类似:

(discountForLoyaltyInPercentage * priceAfterDiscount)

当然,也还是存在例外的:0.7m * price

所以我们把这个改成这样:price - (0.3m * price)

 1 public class DiscountManager
 2 {
 3   public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
 4   {
 5     decimal priceAfterDiscount = 0;
 6     decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5/100 : (decimal)timeOfHavingAccountInYears/100;
 7     switch (accountStatus)
 8     {
 9       case AccountStatus.NotRegistered:
10         priceAfterDiscount = price;
11         break;
12       case AccountStatus.SimpleCustomer:
13         priceAfterDiscount = (price - (0.1m * price));
14         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
15         break;
16       case AccountStatus.ValuableCustomer:
17         priceAfterDiscount = (price - (0.3m * price));
18         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
19         break;
20       case AccountStatus.MostValuableCustomer:
21         priceAfterDiscount = (price - (0.5m * price));
22         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
23         break;
24       default:
25         throw new NotImplementedException();
26     }
27     return priceAfterDiscount;
28   }
29 }

现在我们将整理所有通过账户状态的计算方法改为同一种格式:price - ((static_discount_in_percentages/100) * price)

VI:通过其他方式再摆脱魔法数

接下来让我们的目光放在通过账户状态计算折扣率的计算方法中的静态变量:(static_discount_in_percentages/100)

然后带入下面数字距离试试:0.1m,0.3m,0.5m

这些数字其实也是一种类型的魔法数-他们也没有直接告诉我们他们代表着什么。

我们也有同样的情况,比如将“有账户的时间”折价为“忠诚折扣”。

decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? (decimal)5/100 : (decimal)timeOfHavingAccountInYears/100;

数字5让我们的代码变得神秘了起来。

我们必须做些什么让这个变得更具表现性。

我会用另外一种方法来避免魔法数的表述的出现-也就是C#中的常量(关键词是const),我强烈建议在我们的应用程序中专门定义一个静态类来存储这些常量。

在我们的例子中,我是创建了下面的类:

1 public static class Constants
2 {
3   public const int MAXIMUM_DISCOUNT_FOR_LOYALTY = 5;
4   public const decimal DISCOUNT_FOR_SIMPLE_CUSTOMERS = 0.1m;
5   public const decimal DISCOUNT_FOR_VALUABLE_CUSTOMERS = 0.3m;
6   public const decimal DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS = 0.5m;
7 }

经过一定的修改,我们的DiscountManager类就变成了这样了:

 1 public class DiscountManager
 2 {
 3   public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
 4   {
 5     decimal priceAfterDiscount = 0;
 6     decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY/100 : (decimal)timeOfHavingAccountInYears/100;
 7     switch (accountStatus)
 8     {
 9       case AccountStatus.NotRegistered:
10         priceAfterDiscount = price;
11         break;
12       case AccountStatus.SimpleCustomer:
13         priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS * price));
14         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
15         break;
16       case AccountStatus.ValuableCustomer:
17         priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS * price));
18         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
19         break;
20       case AccountStatus.MostValuableCustomer:
21         priceAfterDiscount = (price - (Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS * price));
22         priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyInPercentage * priceAfterDiscount);
23         break;
24       default:
25         throw new NotImplementedException();
26     }
27     return priceAfterDiscount;
28   }
29 }

我希望你也认同我这个方法会更加使代码自身变得更具有说明性:)

VII:不要再重复啦!

 

我们可以通过分拆算法的方式来移动我们的计算方法,而不是仅仅简单的复制代码。

我们会通过扩展方法。

首先我们会创建两个扩展方法。

 1 public static class PriceExtensions
 2 {
 3   public static decimal ApplyDiscountForAccountStatus(this decimal price, decimal discountSize)
 4   {
 5     return price - (discountSize * price);
 6   }
 7  
 8   public static decimal ApplyDiscountForTimeOfHavingAccount(this decimal price, int timeOfHavingAccountInYears)
 9   {
10      decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY/100 : (decimal)timeOfHavingAccountInYears/100;
11     return price - (discountForLoyaltyInPercentage * price);
12   }
13 }

正如方法的名字一般,我不再需要单独解释一次他们的功能是什么。现在就开始在我们的例子中使用这些代码吧:

 

 1 public class DiscountManager
 2 {
 3   public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
 4   {
 5     decimal priceAfterDiscount = 0;
 6     switch (accountStatus)
 7     {
 8       case AccountStatus.NotRegistered:
 9         priceAfterDiscount = price;
10         break;
11       case AccountStatus.SimpleCustomer:
12         priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS)
13           .ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
14         break;
15       case AccountStatus.ValuableCustomer:
16         priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS)
17           .ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
18         break;
19       case AccountStatus.MostValuableCustomer:
20         priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS)
21           .ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
22         break;
23       default:
24         throw new NotImplementedException();
25     }
26     return priceAfterDiscount;
27   }
28 }

扩展方法让代码看起来更加友善了,但是这个代码还是静态的类,所以会让你单元测试的时候遇到困难,甚至不可能。那么出于摆脱这个问题的打算我们在最后一步来解决这个问题。我将展示这些是如何简化我们的工作生活的。但是对于我个人而言,我喜欢,但是并不算是热衷粉。

不管怎样,你现在同意我们的代码看起来友善多了这一点么?

那我们就继续下去吧!

VIII:移除那些多余的代码

在写代码的时候原则上是我们的代码越是精简越好。精简的代码的意味着,越少的错误的可能性,在阅读理解代码逻辑的时候花费的时间越少。

所以现在开始精简我们的代码吧。

我们可以轻易发现我们三种客户账户下有着相同的方法:

.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);

我们可不可以只写一次呢?我们之前将未注册的用户放在了抛出异常中,因为我们的折扣率只会计算注册用户的年限,并没有给未注册用户留有功能设定。所以,我们应该给未注册用户设定的时间为多少呢? -0年

那么对应的折扣率也将变成0了,这样我们就可以安全的将折扣率交付给未注册用户使用了,那就开始吧!

 1 public class DiscountManager
 2 {
 3   public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
 4   {
 5     decimal priceAfterDiscount = 0;
 6     switch (accountStatus)
 7     {
 8       case AccountStatus.NotRegistered:
 9         priceAfterDiscount = price;
10         break;
11       case AccountStatus.SimpleCustomer:
12         priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS);
13         break;
14       case AccountStatus.ValuableCustomer:
15         priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS);
16         break;
17       case AccountStatus.MostValuableCustomer:
18         priceAfterDiscount = price.ApplyDiscountForAccountStatus(Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS);
19         break;
20       default:
21         throw new NotImplementedException();
22     }
23     priceAfterDiscount = priceAfterDiscount.ApplyDiscountForTimeOfHavingAccount(timeOfHavingAccountInYears);
24     return priceAfterDiscount;
25   }
26 }

我们还可以将这一行移除到switch-case语句外面。好处就是:更少的代码量!

IX:提高-最后的得到干净整洁的代码

好了,现在我们可以像阅读一本书一样方便来审视我们的代码了,但是这就够了么?我们可以将代码变得超级精简的!

好的,那就开始做一些改变来实现这个目标吧。我们可以使用依赖注入和使用策略模式这两种方式。

这就是我们今天最后整理出来的代码了:

 1 public class DiscountManager
 2 {
 3   private readonly IAccountDiscountCalculatorFactory _factory;
 4   private readonly ILoyaltyDiscountCalculator _loyaltyDiscountCalculator;
 5  
 6   public DiscountManager(IAccountDiscountCalculatorFactory factory, ILoyaltyDiscountCalculator loyaltyDiscountCalculator)
 7   {
 8     _factory = factory;
 9     _loyaltyDiscountCalculator = loyaltyDiscountCalculator;
10   }
11  
12   public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
13   {
14     decimal priceAfterDiscount = 0;
15     priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price);
16     priceAfterDiscount = _loyaltyDiscountCalculator.ApplyDiscount(priceAfterDiscount, timeOfHavingAccountInYears);
17     return priceAfterDiscount;
18   }
19 }
 1 public interface ILoyaltyDiscountCalculator
 2 {
 3   decimal ApplyDiscount(decimal price, int timeOfHavingAccountInYears);
 4 }
 5  
 6 public class DefaultLoyaltyDiscountCalculator : ILoyaltyDiscountCalculator
 7 {
 8   public decimal ApplyDiscount(decimal price, int timeOfHavingAccountInYears)
 9   {
10     decimal discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY) ? (decimal)Constants.MAXIMUM_DISCOUNT_FOR_LOYALTY/100 : (decimal)timeOfHavingAccountInYears/100;
11     return price - (discountForLoyaltyInPercentage * price);
12   }
13 }
 1 public interface IAccountDiscountCalculatorFactory
 2 {
 3   IAccountDiscountCalculator GetAccountDiscountCalculator(AccountStatus accountStatus);
 4 }
 5  
 6 public class DefaultAccountDiscountCalculatorFactory : IAccountDiscountCalculatorFactory
 7 {
 8   public IAccountDiscountCalculator GetAccountDiscountCalculator(AccountStatus accountStatus)
 9   {
10     IAccountDiscountCalculator calculator;
11     switch (accountStatus)
12     {
13       case AccountStatus.NotRegistered:
14         calculator = new NotRegisteredDiscountCalculator();
15         break;
16       case AccountStatus.SimpleCustomer:
17         calculator = new SimpleCustomerDiscountCalculator();
18         break;
19       case AccountStatus.ValuableCustomer:
20         calculator = new ValuableCustomerDiscountCalculator();
21         break;
22       case AccountStatus.MostValuableCustomer:
23         calculator = new MostValuableCustomerDiscountCalculator();
24         break;
25       default:
26         throw new NotImplementedException();
27     }
28  
29     return calculator;
30   }
31 }
 1 public interface IAccountDiscountCalculator
 2 {
 3   decimal ApplyDiscount(decimal price);
 4 }
 5  
 6 public class NotRegisteredDiscountCalculator : IAccountDiscountCalculator
 7 {
 8   public decimal ApplyDiscount(decimal price)
 9   {
10     return price;
11   }
12 }
13  
14 public class SimpleCustomerDiscountCalculator : IAccountDiscountCalculator
15 {
16   public decimal ApplyDiscount(decimal price)
17   {
18     return price - (Constants.DISCOUNT_FOR_SIMPLE_CUSTOMERS * price);
19   }
20 }
21  
22 public class ValuableCustomerDiscountCalculator : IAccountDiscountCalculator
23 {
24   public decimal ApplyDiscount(decimal price)
25   {
26     return price - (Constants.DISCOUNT_FOR_VALUABLE_CUSTOMERS * price);
27   }
28 }
29  
30 public class MostValuableCustomerDiscountCalculator : IAccountDiscountCalculator
31 {
32   public decimal ApplyDiscount(decimal price)
33   {
34     return price - (Constants.DISCOUNT_FOR_MOST_VALUABLE_CUSTOMERS * price);
35   }
36 }

首先我们摆脱了扩展方法(也就是静态类),之所以要摆脱这种是因为扩展方法与折扣计算方法之间存在了紧耦合的关系。如果我们想要单元测试我们的方法ApplyDiscount的时候将变得不太容易,因为我们必须统一测试与之紧密关联的类PriceExtensions

为了避免这个,我创建了DefaultLoyaltyDiscountCalculator 类,这里面包含了ApplyDiscountForTimeOfHavingAccount扩展方法,同事我通过抽象接口ILoyaltyDiscountCalculator隐藏了她的具体实现。现在,当我想测试我们的类DiscountManager的时候,我就可以通过 ILoyaltyDiscountCalculator模拟注入虚构对象到DiscountManager类中通过构造函数显示测试功能。这里我们运用的就叫依赖注入模式。

在做这个的同时,我们也将计算折扣率这个功能安全的移交到另一个不同的类中,如果我们想要修改这一段的逻辑,那我们就只需要修改DefaultLoyaltyDiscountCalculator 类就好了,而不需要改动其他的地方,这样减少了在改动他的时候产生破坏其他地方的风险,同时也不需要再增加单独测试的时间了。

下面是我们在DiscountManager类中使用分开的逻辑类:

priceAfterDiscount = _loyaltyDiscountCalculator.ApplyDiscount(priceAfterDiscount, timeOfHavingAccountInYears);

为了针对账户状态的逻辑来计算折扣率,我创建了一些比较复杂的东西。我们在DiscountManager类中有两个责任需要分解出去。

  1. 根据账户状态如何选择对应的计算方法。
  2. 特殊计算方法的细节

为了将第一个责任移交出去,我创建了工厂类(DefaultAccountDiscountCalculatorFactory),为了实现工厂模式,然后再把这个隐藏到抽象IAccountDiscountCalculatorFactory里面去。

我们的工厂会决定选择哪种计算方法。最后我们通过依赖注册模式构造函数将工厂模式注射到DiscountManager类中

下面就是运用了工厂的DiscountManager类:

priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price);

 以上会针对不同的账户状态返回何时的策略,然后调用ApplyDiscount 方法。

第一个责任已经被交接出去了,接下来就是第二个了。

 接下来我们就开始讨论策略了…..

因为不同的账户状态会有不用的折扣计算方法,所以我们需要不同的实现策略。座椅非常适用于策略模式。

在我们的例子中,我们有三种策略:

NotRegisteredDiscountCalculator
SimpleCustomerDiscountCalculator
MostValuableCustomerDiscountCalculator

他们包含了具体的折扣计算方法的实现并被藏在了抽象IAccountDiscountCalculator里。

这就允许我们的类DiscountManager使用合适的策略,而不需要知道具体的实现。我们的类只需要知道与ApplyDiscount方法相关的IAccountDiscountCalculator 接口返回的对象的类型。

NotRegisteredDiscountCalculator, SimpleCustomerDiscountCalculator, MostValuableCustomerDiscountCalculator这些类包含了具体的通过账户状态选择适合计算的计算方法的实现。因为我们的这三个策略看起来相似,我们唯一能做的基本上就只有针对这三种计算策略创建一个方法然后每个策略类通过一个不用的参数来调用她。因为这会让我们的代码变得越来越多,所以我现在决定不这么做了。

好了,到目前为止我们的代码变得可读了,而且每个类都只有一个责任了-这样修改他的时候会单独一一对应了:

  1. DiscountManager-管理代码流
  2. DefaultLoyaltyDiscountCalculator-可靠的计算折扣率的方法
  3. DefaultAccountDiscountCalculatorFactory-决定根据账户状态选择哪个策略来计算
  4. NotRegisteredDiscountCalculator, SimpleCustomerDiscountCalculatorMostValuableCustomerDiscountCalculator – 根据账户状态计算折扣率

现在开始比较现在与之前的方法:

 1 public class Class1
 2 {
 3     public decimal Calculate(decimal amount, int type, int years)
 4     {
 5         decimal result = 0;
 6         decimal disc = (years > 5) ? (decimal)5 / 100 : (decimal)years / 100;
 7         if (type == 1)
 8         {
 9             result = amount;
10         }
11         else if (type == 2)
12         {
13             result = (amount - (0.1m * amount)) - disc * (amount - (0.1m * amount));
14         }
15         else if (type == 3)
16         {
17             result = (0.7m * amount) - disc * (0.7m * amount);
18         }
19         else if (type == 4)
20         {
21             result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount));
22         }
23         return result;
24     }
25 }

这是我们的新的重构的代码:

1 public decimal ApplyDiscount(decimal price, AccountStatus accountStatus, int timeOfHavingAccountInYears)
2 {
3   decimal priceAfterDiscount = 0;
4   priceAfterDiscount = _factory.GetAccountDiscountCalculator(accountStatus).ApplyDiscount(price);
5   priceAfterDiscount = _loyaltyDiscountCalculator.ApplyDiscount(priceAfterDiscount, timeOfHavingAccountInYears);
6   return priceAfterDiscount;
7 }

总结

在本文中,代码被极其简化了,使得所有的技术和模式的解释更容易了。它展示了如何解决常见的编程问题,以及使用良好的实践和设计模式以适当、干净的方式解决这些问题的好处。

在我的工作经历中,我多次在这篇文章中强调了不良的做法。它们显然存在于许多应用场合,而不是在一个类中,如在我的例子中那样,这使得发现它们更加困难,因为它们隐藏在适当的代码之间。写这种代码的人总是争辩说,他们遵循的是简单愚蠢的规则。不幸的是,几乎所有的系统都在成长,变得非常复杂。然后,这个简单的、不可扩展的代码中的每一个修改都是非常重要的,并且带来了巨大的风险。

请记住,您的代码将长期存在于生产环境中,并将在每个业务需求更改上进行修改。因此编写过于简单、不可扩展的代码很快就会产生严重的后果。最后一点是对开发人员有利,尤其是那些在你自己之后维护你的代码。

如果你有一些问题根据文章不要犹豫联系我!

PS:

根据文章敲的源代码:

链接:https://pan.baidu.com/s/1i7BH6Krl_vPnwtBCJjpQ8w 密码:9spz

posted @ 2018-07-13 10:23  多多陪着小五  阅读(989)  评论(2编辑  收藏  举报