代码细节重构:请对个人代码指手划脚(四)

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

目标代码

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

结语

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

以前我发表过不少文章,都是从人文角度来引导新手的,今天这篇文章,也正好说明了人文因素对程序员素质的影响!优化

要作事,请首先作好人,对本身的代码负责就是对本身负责,对本身负责就是对团队负责,对团队负责就是对企业利益负责,由于咱们是同一个团队,拥有一样的利益!编码

相关文章
相关标签/搜索