BUAA软工个人作业Week2-代码复审

Posted 2020-10-09 InuyashaSAMA

一. 代码复审Check List

---

1.概要部分

---

代码能符合需求和规格说明么？

对-c的测试：

可以看到程序不支持1000000的数独终局输出，读源码发现常量MaxCounts定义为了100000，导致无法计算十万以上的数独终局。

另外，实际命令行运行发现程序无法自行停止，查源码发现最后的getchar()导致这一问题。

对-s的测试：发现输出的文件为空，不符合要求，交流后了解到是由于审题有错，导致读取文件时按用空格表示数独的空（而不是0）的格式读取，于是程序无法处理数字间有空格的正确格式文件。

相关issue：https://github.com/FelixCaae/SudokuGen/issues/1

代码设计是否有周全的考虑？

见上一条，由于粗心的缘故导致出现了一些小问题，对于一些输入条件也有一些BUG，例如输入-c abc将会正常执行，不输出错误，当然这些问题后来都解决了。

代码可读性如何？

部分类及函数没有注释，读起来十分费劲，也有的函数添加了详尽的注释，能够让人容易读懂。整体采用面向对象的设计思路，函数功能简洁，故总体而言，代码可读性较好，但不算很出色，建议多加注释。

代码容易维护么？

代码充分体现了面向对象的思想，函数的长度都较短，换句话说修改代码时不需要做太多重构，应该较为容易维护。

代码的每一行都执行并检查过了吗？

存在clock计时和getchar这些冗余代码，Table.h中有Set函数的一个重载void Set(int num[][9]);没有定义和使用，其余代码均是必要的，且经执行都是正确的。

2.设计规范部分

---

设计是否遵从已知的设计模式或项目中常用的模式？

没有设计模式，总体来说遵循面向对象的思路。

有没有硬编码或字符串/数字等存在？

存储数独的二维数组使用int[9][9]，其他没有。

代码有没有依赖于某一平台，是否会影响将来的移植（如Win32到Win64）？

依赖于windows，但是并不会影响win32到win64的移植。

开发者新写的代码能否用已有的Library/SDK/Framework中的功能实现？在本项目中是否存在类似的功能可以调用而不用全部重新实现？

没有，全部函数都无法用已知的调用功能实现。

有没有无用的代码可以清除？（很多人想保留尽可能多的代码，因为以后可能会用上，这样导致程序文件中有很多注释掉的代码，这些代码都可以删除，因为源代码控制已经保存了原来的老代码。）

没有注释掉的代码，至于冗余代码，见概要的最后一条。

3.代码规范部分

---

修改的部分符合代码标准和风格么（详细条文略）?

部分if后没有加大括号和换行，头文件没有添加头文件保护符，例如：

4.具体代码部分

---

有没有对错误进行处理？对于调用的外部函数，是否检查了返回值或处理了异常？

对于文件的IO测试时，成功对不存在的文本文件101.txt进行了错误处理：

但是对于非法的生成数独规模参数却没有进行判断，例如下面这种输入依然生成了100个数独终局。

在对这个参数进行判断的时候进行了如下操作：

但并没有对sscanf_s的返回值进行处理判断写入成功与否，而是在外面包裹了try块，这样是不行的，因为sscanf不会主动抛出异常，实际上在输入abc时这个函数返回0，这是可以通过返回值判断的（当然输入100a的话仍然会写入100，需另行处理）。

参数传递有无错误，字符串的长度是字节的长度还是字符（可能是单/双字节）的长度，是以0开始计数还是以1开始计数？

经过检查，参数传递无错误，字符串的长度是字节的长度，从0开始计数。

边界条件是如何处理的？Switch语句的Default是如何处理的？循环有没有可能出现死循环？

文件的IO通过fgetc和feof的返回值进行判断，没有使用Switch语句，经过检查For循环的循环条件都是有限的，While循环都通过文件操作函数的返回值进行判断终止操作了，所以不会出现死循环。

有没有使用断言（Assert）来保证我们认为不变的条件真的满足？

没有使用。

对资源的利用，是在哪里申请，在哪里释放的？有没有可能导致资源泄露（内存、文件、各种GUI资源、数据库访问的连接，等等）？有没有可能优化？

在构造函数中申请的内存经检查均在析构函数中进行了释放，故不会出现内存泄漏，优化暂时来说不需要。

数据结构中是否有无用的元素？

没有。

5.效能

---

代码的效能（Performance）如何？最坏的情况是怎样的？

在上面提到的BUG修复后，通过对其提交的Release版本的程序进行测试，测试量级为1000000，运行时间为8秒，由于他生成和求解数独所用的算法都是回溯递归，所以速度不快，但是这样的速度可以接受了。解数独较坏的情况是需要很多次的重复回溯，但是这样的话其实是难以避免的，因为很难判断从哪开始填能减少回溯次数。

代码中，特别是循环中是否有明显可优化的部分（C++中反复创建类，C#中string的操作是否能用StringBuilder 来优化）？

没有，循环体都很精简，没有重复创建类之类的操作。

对于系统和网络调用是否会超时？如何处理?

没有系统和网络调用。

6.可读性

---

代码可读性如何？有没有足够的注释？

整体来说代码可读性还OK，但是SdkBuffer和FileHandler的类函数建议加注释，因为初次阅读会很困惑，Table.h的函数虽然很乱（有4个函数重载），但加了注释勉强提高了理解速度。

7.可测试性

---

代码是否需要更新或创建新的单元测试？

不需要，多数类都进行过单元测试。

还可以有针对特定领域开发（如数据库、网页、多线程等）的核查表。

不需要。

二、设计一个代码规范

---

工具提供的代码规范和你个人的代码风格有什么不同？

下载了cpplint对我的main函数所在cpp文件进行了测试，结果如下：

总结出来如下问题：

1. 检测到tab，这是因为当初建项目时没有设置tab为4个空格，在写到一半的时候才设置，导致有一部分代码是没有转换过的。
2. 数组长度为变量长度，这不是个错误，我的数组长度是定值，只不过其中使用了const常量，我想这是这个工具无法察觉的。
3. 前半个大括号应该在前一句的结尾，这个我觉得都OK，换行可以更加清晰。
4. 一行代码的长度应该小于等于80个字符。看了一下这是因为我那一行加了注释……
5. //和注释之间应该加一个空格。好吧我接受，毕竟好看一点。
6. else应该就写在}后面，这我不同意，看着乱，还是换行好。
7. 加copyright，可以。
8. 不要使用namespace。

工具提供的代码规范里有哪些部分是你之前没有想到的？

我觉得copyright是一定没想到，但是它要求if和else一定写成：

if{

    do sth;

}else{

    do sth;

}

我没想到这一点，我觉得全部换行更好看也更加便于阅读，换句话说我觉得这样写是不好的。

为什么要这样规范？这样有意义吗？

这样的规范例如tab换成4个空格是方便在其他地方编辑查看，每个人都是用同样的代码规范可以便于协作和阅读，同时一些要求可以使得代码更易读。

根据构建之法书上编码规范里提到的那些要点整理一份在结对编程时使用的代码规范：

1. 4个空格缩进；
2. 每行字符长度不超过80；
3. if语句写大括号，且所有大括号独占一行；
4. 多条语句一定分行；
5. 类型/类/函数名用Pascal形式，变量名用lowerCamel形式；
6. 每个函数头部写注释表明注意事项和功能等；
7. 类成员的声明先public后private，成员变量写最前面；