【进来讨论一下】代码审查,应该做的和不该做的几件事儿
§代码审查,应该做的几件事儿1)正确性
是否满足客户需求
是否和需求分析以及设计资料的内容保持一致
一致的意思:不仅要求功能正确,而且要求完备
极值(最大值、最小值、特殊值(比如0、负数、null、NaN)、异常值(字母、回车/换行等控制字符))等等特殊情况是否都已经被考虑到了,这些情况下程序的动作是否满足需求
还有哪些情况没有被考虑?没有被实现?
2)结构性
现在的功能加入到系统后,
对整个系统有什么好的影响,
以及不好的影响(程序员一般都过于乐观,这点可能需要第三者帮着冷静的分析一下才行)
以后再追加新的功能时,是否可以不大改结构就能够实现,千万别“将就”
3)易读性
换一个完全不懂这个系统的人,是否能通过代码立即理解原作者的意图(需要代码审查者把自己假想为一个完全的新人才行)
推敲一下,是否有过多的不必要的注释:
经年累月后,人们逐渐发现,只有代码本身才可以真正被信赖
最小限度的注释,以及自说明的代码才是最好的方式
1次书写N次阅读,多站在今后的阅读者的立场上考虑问题
关心一下系统日志(log):
当出现Bug时,可以很容易的分析出是哪里出了什么问题吗?
如果写代码的人自己都不能分分秒知道问题出在哪里,还能指望谁?
例:
看系统日志知道是系统运行时发生异常造成系统崩溃了
log应该分分秒告诉你:是谁的问题?什么异常?什么时候发生的?哪里出的?为什么会出?怎么解决?(5W1H)
4)性能
有没有更好的,更合理的实现方法
性能等是否有问题、有没有内存泄露、资源泄露等用测试的手法也不易发现的问题
§代码审查,不应该做的几件事儿
1)通过工具能够自动检查的问题
比如项目组的编码规范不允许使用TAB,而需要统一使用空格来进行缩进
这样的检查如果人工来做的话,会花费大量不必要的时间
还好有Eclipse Checkstyle这样的自动工具
代码审查时仅需要确认一下Checkstyle的检查是否实施
所有的检查结果是否都是OK的
如果有NG的地方,是否是被允许的
是否需要更新Checkstyle的规则
2)通过简单测试就能够发现的问题
比如系统是否能启动起来,大体上的功能是否已经实现
如果这些最基本的问题还没有解决的话,直接开代码审查就是在浪费时间
3)过去经常发生,或者已经经常指出的问题
网上有很多成型的东东,可以参考
这些问题可以通过Check List的形式,让开发人员做自检
并把检查结果报告给代码审查的责任人
可以每次考察Check List中不同的一两项,来确认开发人员已经充分理解自检的目的
比如:参数是否为null的判定,除数是否为零的判定,异常处理等最基本的需要注意的问题
如果之后发现了Bug,一定要考虑是否需要更新Check List以杜绝发生相似的问题
另外,如果Check List已经不能发现任何问题的话,
可以考虑分类(新加入项目组的开发人员使用的,以及成手使用的),以节约成手的时间
总之,这个东东最好小而强大,没用的东西要定期删除
原创,转发请注明出处