实验四 代码评审

一、实验目的

1） 了解代码审查的含义；

2） 掌握相关编程规范检查工具的安装与使用；

二、实验内容及要求

Code Review中文应该译作“代码审查”或是“代码评审”或“代码复查”，是指通过阅读代码来检查源代码与编码标准的符合性以及代码质量的活动。Code Review主要用来在软件工程过程中改进代码质量，通过代码评审可以达到如下目的：

●在项目早期就能够发现代码中的BUG

●帮助初级开发人员学习高级开发人员的经验，达到知识共享

●避免开发人员犯一些很常见，很普通的错误

●保证项目组人员的良好沟通

●项目或产品的代码更容易维护

     代码评审主要内容是编程规范，重构方法，架构设计，性能安全，日志，可读性，扩展性等问题。通过代码评审可查找和修复引入到开发阶段的应用程序的错误，提高软件的整体素质和开发者的技能。代码评审的作用和意义已在很多技术团队内达成共识，可是很多时候并未被有效执行，甚至被认为是一项费时费力的工作。借助一些工具可以更容易，更有效率地来进行Code Review。

1、以小组形式，针对前面“实验一”中所完成的代码，进行代码评审(走查)，重点检查以下情况。你也可有查询相关材料，建立更细化的检查清单（check list）

- 程序是否能正常工作，代码是否实现预期的功能，逻辑是否正确。

- 代码是否遵循的编程规范

- 代码是否尽可能的模块化

- 所有的数据输入是否都进行了检查

- 是否有注释，并且描述了代码的意图

- 代码的可理解性和可测试性

2、按“实验二”的分组方式，两人一组，随机分配另一组的代码作为本组评审和分析的对象

一些编码规范的检查工具如下，也可自行查找工具使用。

java语言      

- 采用使用eclipse Checkstyle插件 

- CheckStyle是SourceForge下的一个项目，提供了一个帮助JAVA开发人员遵守某些编码规范的工具。它能够自动化代码规范检查过程，从而使得开发人员从这项重要但枯燥的任务中解脱出来。它可以根据设置好的编码规则来检查代码。比如符合规范的变量命名，方法体的最大行数，重复代码检查等等。

C++语言

- 可使用Google代码规范工具Cpplint。

- Cpplint是一个python脚本，Google使用它作为自己的C++代码规范检查工具，VSCcode可配置Cpplint对C++代码进行规范检查。

python

- 可采用pylint

- Pylint 是一个 Python 代码分析工具，它分析 Python 代码中的错误，查找不符合代码风格标准（Pylint 默认使用的代码风格是 PEP 8，具体信息，请参阅参考资料）和有潜在问题的代码。目前在 eclipse 的 pydev 插件中也集成了 Pylint，VSCcode可安装pylint插件

    当发现了项目存在的问题后，可通过Bug跟踪系统向项目维护者反馈问题（issue）,管理Issue的系统称为BTS（Bug Tracking System，Bug跟踪系统）。当今具有代表性的BTS有Redmine、Trac、BugZilla等。GitHub自身也加入了BTS的功能。在GitHub上，可以将它作为软件开发者之间的交流工具。通过github的issues功能开发者可以便捷的发现软件的BUG并报告，想向项目所有人询问或用来追踪各种想法探讨准备实施的任务。

三、实验过程

（1）配置代码审查工具。要求采用屏幕截图的方式配置的过程；

待审查项目组的源代码语言：C语言

采用的审查工具： PC-Lint

简单介绍 PC-Lint：PC-Lint 是GIMPEL SOFTWARE公司开发的C/C++软件代码静态分析工具，它的全称是PC-Lint/FlexeLint for C/C++，PC-Lint 能够在Windows、MS-DOS和OS/2平台上使用，以二进制可执行文件的形式发布，而FlexeLint 运行于其它平台，以源代码的形式发布。PC-lint 在全球拥有广泛的客户群，许多大型的软件开发组织都把PC-Lint 检查作为代码走查的第一道工序。PC-Lint不仅能够对程序进行全局分析，识别没有被适当检验的数组下标，报告未被初始化的变量，警告使用空指针以及冗余的代码，还能够有效地帮你提出许多程序在空间利用、运行效率上的改进点。

配置PC-Lint：

1.

a.首先, 当然要下载软件，从http://www.61ic.com/down/othe/pclint.rar处可以下载到一个8.0版本的pclint.

b.将pclint.rar解压, 并将如图D:/pclint/lnt 下的3个文件lib-w32.lnt，env-vc6.lnt，co-msc60.lnt拷贝至D:/pclint根目录下

c.再在安装目录下创建std.lnt和options.lnt两个文件，其中std.lnt的内容如下,其中-i后面的路径名为VC的安装路径和VC Include 文件路径，根据自己的修改便可。options.lnt 内容可为空，为定制内容,以后需要时再添加

d.打开VC6，tools--->customize-->tools 新建一个名为pclint的项，在下面填入

command:  C:/pclint/lint-nt.exe
arguments:  -u c:/pclint/std.lnt  c:/pclint/env-vc6.lnt  "$(FilePath)"

Use Output Window  打上勾
close 完成。 这时VC窗口tools菜单下应该多了一个pclint选项，可以用它来运行lint程序，对c/c++代码进行静态检查了。
 

 2.如果一个VC项目中包含多个C或C++文件，有时需要同时对这一系列的文件进行lint检查，我们可以通过配置一个pclint_project来达到目的。

a.先从http://www.weihenstephan.de/~syring/win32/UnxUtils.zip下载UnxUtils.zip.

b.解压UnxUtils.zip到指定路径,打开VC6，tools--->customize-->tools 新建一个名为pclint_project的项，只不过下面的commands和arguments内容不同。

commands: D:/unix/usr/local/wbin/find.exe
arguments: $(FileDir) -name *.c  -o -name *.cpp | D:/unix/usr/local/wbin/xargs lint-nt -i"D:/unix/usr/local"  -u D:/pclint/std.lnt  c:/pclint/env-vc6.lnt

Use Output Window打上勾，close退出。这时VC tools菜单下又多了一个pclint_project项，可以对一个VC项目运行lint检查程序.

（2）使用工具对原始代码进行评审和分析，记录结果，期间不要有任何修改；

问题记录如下：

（3）对工具执行结果进行人工分析，结合检查清单和人工走查的出代码修改建议；

问题1修改意见：因为time返回的是有符号的，而srand接受的是无符号的，可进行强制类型转换；

问题2修改意见：C语言——switch case语句 (switch，case和default是关键字)，虽然case已经包含了所有情况，default仍然不可省；

问题3修改意见：程序中变量d的直接使用虽然对整个程序并无影响。，但是规范编写时，仍需要初始化一个值；

问题4：前三个解决过后自动消除。

解决过后如下所示：

（4）通过github issues向项目维护者提交问题(issue),注意一个issue 只报告一个问题，多个问题需放在多个issue中，以便跟踪。

（5）记录总结实验过程中遇到的问题和解决过程

实验过程中最大的困难莫过于找到这个审查C语言的工具了，审查工具千千万，C语言的却极少┗( ▔, ▔ )┛，查阅很久资料才最终决定用PC-Lint，然后配置过程也有点曲折，对于这个的配置，资料比较少，很多博客内容都是很多年前的了，好多资源都失效了，然后照着配了好几次都失败了用不了，其中我将2点容易出错的地方总结如下：

1:将pclint放在任何目录都是可以的，但是，如果是放在例如: Program Files这样的目录中，由于该目录中间有空格，所以，解析时常常会出错，需要在绝对路径外面加上引号，例如:"D:\Program Files\pclint\lint-nt.exe"

2:运行时，会提示PC-lint for C/C++ (NT) Ver. 8.00e, Copyright Gimpel Software 1985-2001d:\pclint\co-msc60.Int(214) : Error 307: Can't open indirect file 'lib-ole.Int'Tool returned code:2这个错误，打开co-msc60.Int, 我们可以看到该文件最后一行对lib-ole.Int' 的调用，简单的处理直接注释掉就行了，如需用到OLE,请设置lib-ole.Int的绝对路径，并请参考下pclint的相关文档

经过此次试验初次体验到什么是代码审查，自己本身的代码也是C语言编写的，审查过后发现错误还挺多的，远不止这组的4个了，所以今后写代码也要更注意于这方面才行。

Tips:后续我又用了cpplint工具进行审查，结果就是完全不一样了。一开始是124个错误，过滤掉空格问题，剩余12个，全部都是rand（…）的线程安全性问题，个人觉得这个似乎更合理一些，上面审查出来的问题有点不太好理解。