代码细节重构:请对我的代码指手划脚(四)
这是上周在代码审阅会议上讨论到的一段代码,这段代码的作用是根据指定记录数量和页面大小来计算最大分页数量的。
目标代码
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”这个方法被多次重新定义,且功能完全一样。应该是开发人员复制粘贴过来的,原本也是想针对复制粘贴来讲讲这段代码的,但后来发现了更多的问题:
- 代码重用问题。通过在整个解决方案搜索,发现这个方法被原封不动的复制粘贴了36次,总计在37个类当中出现了37次,而且这些类全部都是静态类。这里的问题就是对于重复出现的代码逻辑,而且是业务逻辑简单且又完全一样的代码,没能实现代码重用,这会给后续的代码维护带来诸多不便;
- 类型安全问题。这段代码的操作对象都是数字,数字属于值类型,类型安全问题不会显得特别突出。但数字本身却会有一些其他的隐含问题,比如数据类型转换、分母不能为零等;
- 分母不能为零。很显然,这段代码里面包含了除法,而对于除法运算没有做分母不能为零的逻辑处理。对于新手程序员来说这是很容易被忽略的问题,但对于有一定经验的研发人员来讲,这一点势必已经刻骨铭心了;
- 代码性能问题。我们知道在托管代码中进行各种类型转换都会带来不小的性能损失,而如上代码便存在着一些可优化的部分;
- 业务逻辑问题。认真观察和分析如上代码,你会发现该方法的输入参数在特例条件下会输出不符合实际期望的结果。比如输出结果可能是小于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 }
结语
俗话说细节决定成败。作为程序员,从代码的细节我们就能看得出一个人编程水平的高低和开发经验的多寡,这也从一定意义上决定了你的成与败!
之前我发表过很多文章,都是从人文角度来引导新手的,今天这篇文章,也正好说明了人文因素对程序员素质的影响!
要做事,请首先做好人,对自己的代码负责就是对自己负责,对自己负责就是对团队负责,对团队负责就是对企业利益负责,因为我们是同一个团队,拥有同样的利益!
使用流程引擎整体解决方案
前面对流程引擎个方面已经有了比较详细的介绍,本篇将最后介绍流程调用的整体解决方案。
在具体项目调用流程引擎之前,我们需要做的是先把流程引擎以及可视化的流程设计器嵌入到自己的项目中,具体的嵌入方式请参考:流程引擎及流程设计器的嵌入方式。
欢迎加入流程交QQ流群:251834323。
在进行流程调用之前,首先需要根据具体的项目扩展流程引擎公开的相关接口(IPersonList)。此接口的作用为:把具体项目相关的人员信息、部门信息、角色信息等提供给流程引擎调用。
IPersonList接口包含两个方法。GetPersonList及GetGlobalKeyValue,前者扩展的目的为:让流程引擎可以通过调用此方法,获得人员类型ID对应的所有的人员列表。后者扩展的目的为:使具体的人员类型对流程引擎透明化(角色,具体有那些角色、部门,具体有哪些部门等),以便可视化流程设计中对相关人员类型处理。

实现相关接口后即可根据具体的业务逻辑利用可视化流程设计器进行流程的设计。
由于此流程引擎并未嵌入表单设计相关事项(后期可能加入表单设计引擎),所以如果想使用此流程引擎,还需要自行设计相关流转表单。当设计好表单后,可调用流程引擎相关方法达到公文流转。
以下提到的方法具体说明请参考:通用流程相关方法说明及调用事例。
1.调用WorkflowOperation中的方法IsExistTask或GetCurrentStep判断表单针对某流程的任务是否已经发送。
2.如果任务未发送,调用WorkflowOperation中的SendWorkflow方法,发起公文的流转。
3.如果任务已发送,调用TaskListOperation中的GetPersonTask或GetByFormTaskList方法获取待处理的任务列表。
4.调用WorkflowOperation中的IsHereafter方法,判断待处理任务的节点步骤的处理人类型是否为将来指定,如果为将来指定,需弹出人员选择框选择具体的处理人,然后进入步骤5。
5.调用TaskListOperation中的TaskListUpdate方法,对待处理任务进行处理。
6.再次调用WorkflowOperation中的SendWorkflow方法,推动公文的流转。
7.判断6中的方法的返回值,确定推动状态,如果未到达结束节点步骤,重复步骤3-6,否则对表单做后续的相关处理,如改变表单状态等。
在公文的正常流转中,难免会出现许许多多的特殊情况,例如,审批人的更换、添加、移除等。在此流程引擎中,也有相关的解决方案。即,通过如下方法对处理人进行增删。
TaskListOperation类中的AddHandlePerson方法对处理人的增加。
TaskListOperation类中的RemoveHandlePerson方法对处理人的移除。
以上既是,包含流转发起、流转推动、任务列表获取等在内的基本流程应用。
注:流程引擎正在不断的努力完善中,同时希望有相同爱好的人能加入QQ群251834323,共同为开源软件做一份微薄之力。
相关文章连接:
通用流程设计:http://www.cnblogs.com/qidq/p/workflow.html
可视化流程设计——流程设计器演示(基于Silverlight):http://www.cnblogs.com/qidq/p/Workflow_Silverlight.html