代码细节重构：请对我的代码指手划脚（四）

这是上周在代码审阅会议上讨论到的一段代码，这段代码的作用是根据指定记录数量和页面大小来计算最大分页数量的。

目标代码

 1 /// <summary>
 2 /// 根据指定记录数量和页面大小返回分页页数。
 3 /// </summary>
 4 /// <param name="totalRecords">记录总数。</param>
 5 /// <param name="pageSize">页面大小。</param>
 6 /// <returns>返回分页页数。</returns>
 7 public static long ComputePages(int totalRecords, int pageSize)
 8 {
 9     var total = Convert.ToDouble(totalRecords);
10     var size = Convert.ToDouble(pageSize);
11 
12     var temp = total / size;
13 
14     return (long)Math.Ceiling(temp);
15 }

代码问题

在代码审阅会议上起初并没有留意到这段代码，但随着翻阅了多个代码页之后便发现“ComputePages”这个方法被多次重新定义，且功能完全一样。应该是开发人员复制粘贴过来的，原本也是想针对复制粘贴来讲讲这段代码的，但后来发现了更多的问题：

  1. 代码重用问题。通过在整个解决方案搜索，发现这个方法被原封不动的复制粘贴了36次，总计在37个类当中出现了37次，而且这些类全部都是静态类。这里的问题就是对于重复出现的代码逻辑，而且是业务逻辑简单且又完全一样的代码，没能实现代码重用，这会给后续的代码维护带来诸多不便；
  2. 类型安全问题。这段代码的操作对象都是数字，数字属于值类型，类型安全问题不会显得特别突出。但数字本身却会有一些其他的隐含问题，比如数据类型转换、分母不能为零等；
  3. 分母不能为零。很显然，这段代码里面包含了除法，而对于除法运算没有做分母不能为零的逻辑处理。对于新手程序员来说这是很容易被忽略的问题，但对于有一定经验的研发人员来讲，这一点势必已经刻骨铭心了；
  4. 代码性能问题。我们知道在托管代码中进行各种类型转换都会带来不小的性能损失，而如上代码便存在着一些可优化的部分；
  5. 业务逻辑问题。认真观察和分析如上代码，你会发现该方法的输入参数在特例条件下会输出不符合实际期望的结果。比如输出结果可能是小于0的负数；

重构建议

找到了问题所在，那么就会有一定的解决方案：

  • 对于代码重用，可以在某个业务逻辑相关的类中集中定义这个方法，而不是分散到不同的类中。不过，本文不会具体讨论这个问题；
  • 对于类型安全、类型转换，具体到数字的时候，我们应该适当的考虑一下通过数学的方法来解决这些问题，而不是死板的套取程序语言规范；
  • 对于分母不能为零这一类定理性的问题，我们在编码过程中一定要小心在意！千万不能抱着侥幸心理；
  • 对于业务逻辑上的问题，我们在编码之前一定要认真思考、琢磨，弄清楚要解决的问题及可能产生的后果；

重构结果

为了使得大家更容易理解，我将代码重构的一些具体思路和想法以备注的方式贴在代码里面了，大家认真观察，有问题可以提出。

 1 /// <summary>
 2 /// 根据指定记录数量和页面大小返回分页页数。
 3 /// </summary>
 4 /// <param name="totalRecords">记录总数。</param>
 5 /// <param name="pageSize">页面大小。</param>
 6 /// <returns>返回分页页数。</returns>
 7 /* 
 8     * 1、方法返回值从long修改为int。原因如下：
 9     *    ·输入参数皆为int，且运算关系为除法，所以输出结果必定在int范围之内。不使用unit是因为那会给整个解决方案的其他部分代码带来很多的编码痛苦；
10     *    ·小小的内存性能改进；
11     */
12 public static int ComputePages(int totalRecords, int pageSize)
13 {
14     /*
15         * 2、对记录总数的类型安全判断和业务逻辑处理：
16         *    ·记录总数肯定是大于等于零的数；
17         *    ·记录总数为零的时候，页数肯定也是零，此时后续的代码逻辑没有意义，可以直接返回0；
18         */
19     if (totalRecords < 1) return 0;
20     /*
21         * 3、对页面大小的类型安全判断和业务逻辑处理：
22         *    ·页面大小不能为0，因为分母不能为0；
23         *    ·页面大小不能为负数，因为这与事实不符；
24         *    ·当页面大小为1的时候，页数其实就是记录总数，直接返回totalRecords，这同时也避免了后续的代码无意义的执行，减少了性能损耗；
25         */
26     if (pageSize < 1) return totalRecords;
27 
28     /*
29         * 4、对原方法业务逻辑的重构、类型转换方式的优化及性能优化：
30         *    ·通过任何数字乘以（或除以）浮点数其结果仍然为浮点数的方式完成自动的、隐式的类型转换，这不但使得代码更加简洁，且优化了性能；
31         *    ·代码简单、可读性强，更加容易理解业务逻辑；
32         */
33     return (int)Math.Ceiling(totalRecords*1.0/pageSize);
34 }

结语

俗话说细节决定成败。作为程序员，从代码的细节我们就能看得出一个人编程水平的高低和开发经验的多寡，这也从一定意义上决定了你的成与败！

之前我发表过很多文章，都是从人文角度来引导新手的，今天这篇文章，也正好说明了人文因素对程序员素质的影响！

要做事，请首先做好人，对自己的代码负责就是对自己负责，对自己负责就是对团队负责，对团队负责就是对企业利益负责，因为我们是同一个团队，拥有同样的利益！

 

posted @ 2014-01-06 13:55  O.C  阅读(2159)  评论(8)  编辑  收藏  举报