[buaa-SE-2017]个人作业-Week2
个人作业-Week2
一、代码复审Checklist
1.概要部分
1.1 代码能符合需求和规格说明么？
本次作业的需求可以分成基本的功能实现和大规模数据下程序的健壮性，以及少量的异常处理能力，也就是说我们可以通过一般功能测试和压力测试和少量的鲁棒性测试来进行测试。
程序的测试结果首先参考了2017BUAA软工助教 个人项目测试结果
测试-c的结果如下图：
| NumberID | -c 1 | -c 5 | -c 100 | -c 500 | -c 1000 | -c 50000 | -c 1000000 |
|---|---|---|---|---|---|---|---|
| 15061104 | 0.9 | 0.117 | 0.218 | 0.76 | 1.326 | 60.102 | -8 |
测试-s的结果如下图：
| NumberID | -s 1.txt | -s 5.txt | -s 100.txt | -s 500.txt | -s 1000.txt | -s 50000.txt | -s 1000000.txt |
|---|---|---|---|---|---|---|---|
| 15061104 | 0.148 | 0.2 | 2.637 | 12.7 | 25.165 | -2 | -4 |
接下来分别说明这些测试的结果。
- 一般功能测试
测试结果显示，程序在可以完成少量数据规模下解决问题的需求，这里不再赘述。 - 压力测试
测试结果显示，程序在生成1million个数独的时候无法生成足够的数独。我在自己的PC上进行了同样的测试，即输入：
suduku.exe -c 1000000
等待10分钟无果。
- 异常数据
我设计了若干测试样例，它们的说明和测试结果见下表：
| 样例 | 语句 | 说明 | 预期结果 | 实际结果 |
|---|---|---|---|---|
| case1 | sudoku.exe -c asd | 参数非法 | 退出并报错 | 报错并退出 |
| case2 | sudoku.exe -c 0 | 边界值 | 退出并报错，或者sudoku.txt为空 | 报错并退出 |
| case3 | sudoku.exe -c -1 | 参数越界 | 退出并报错 | 报错并退出 |
| case4 | sudoku.exe -c 10000000 | 参数越界 | 退出并报错 | 报错并退出 |
| case5 | sudoku.exe -s nonexist.txt | 文件不存在 | 退出并报错 | 退出 |
| case6 | sudoku.exe -s nosolution.txt | 文件中数独不可解 | 退出并报错 | sudoku.txt中生成全0数独 |
| case7 | sudoku.exe -s | 参数不够 | 退出并报错 | 报错并退出 |
- 总结
总的来说，程序能够完成基本的需求，但是程序不能在大规模数据下保持健壮性，同时对于少数异常不能很好的处理。
1.2 代码设计是否有周全的考虑？
根据1.1中的测试结果可知，程序在考虑设计上有些欠妥当。
1.3 代码可读性如何？
从代码的：层次结构，注释的详尽程度，注释的易读程度，变量的命名清晰度来分析。
-
层次结构
优点：作者使用的是DLX算法，代码大部分采用了OO的风格，代码中大致包括DLXNode,DLXGenerator,DLXSolver,SudokuLoader,SudokuSolver，这些类的划分比较清晰易懂，从main函数开始的层次结构也比较清楚。
缺点：Issue1（1）main.cpp中的几个函数solvePuzzle、createSudoku等很有面向过程的风格，很让人困惑，也许更好的方法是把这些函数封装到相关类的方法中。（2）程序的输出由类SudokuLoader来实现，这个功能明显和这个类的命名有冲突，也许可以新建一个SudokuOutput类来处理。 -
注释的详尽程度
优点：程序对大部分方法都有简略的说明，在一些具体的算法步骤上也有说明，有一定的可读性。
缺点：程序对类的说明较少，也许可以对每个类加一个简要的注释来说明其职责、可变性等。 -
注释的易读程度
较好，没有生僻词。虽然方法只有说明没有具体的例子，但因为本次作业比较简单，所以不需要也可以读懂。 -
变量的命名清晰度
比较清晰，基本能从名字中了解到变量的功能。 -
总结
总体来说，代码可读性较高，但在结构设计和类的功能划分上仍有部分细节有些混乱。
1.4 代码容易维护么？
可维护性主要是指代码是否能适应各种各样的变化，这次作业结构比较简单，所以总体来说问题不大。
不过测试代码中用到了itoa函数，这个函数在linux中不可用，会导致程序的可维护性降低。见Issue5
1.5 代码的每一行都执行并检查过了吗？
除了一些冗余的代码，基本可以全部覆盖，冗余代码的说明在2.5节。
2.设计规范部分
2.1 设计是否遵从已知的设计模式或项目中常用的模式？
没有特定的设计模式。
2.2 有没有硬编码或字符串/数字等存在？
存在。Issue3
在SudokuLoader::writeToFile函数中使用了硬编码:
char content[19 * 9 + 2];
2.3 代码有没有依赖于某一平台，是否会影响将来的移植（如Win32到Win64）？
代码本身没有影响，但是测试代码中含有itoa会影响移植到linux之后的测试。
2.4 开发者新写的代码能否用已有的Library/SDK/Framework中的功能实现？在本项目中是否存在类似的功能可以调用而不用全部重新实现？
基本没有，由于使用了DLX，其中的一些基本操作和数据结构需要重新定义。
2.5 有没有无用的代码可以清除？（很多人想保留尽可能多的代码，因为以后可能会用上，这样导致程序文件中有很多注释掉的代码，这些代码都可以删除，因为源代码控制已经保存了原来的老代码。）
有：
void solveWithAllAnswers(DLXNode *listHead, vector<int>& tempSolution, vector<vector<int>>& lastSolution, int depth);
这个方法的定义和代码都可以删除。Issue4
3.代码规范部分
3.1 修改的部分符合代码标准和风格么（详细条文略）?
代码基本有统一的风格，问题已经在之前阐述过。
4.具体代码部分
4.1有没有对错误进行处理？对于调用的外部函数，是否检查了返回值或处理了异常？
内部的异常处理在1.1中阐述过。
对于外部操作，在用fstream来操作文件的时候，有如下代码：
if (strcmp(argv[1], "-s") == 0) { //Solve puzzle from file
fstream puzzleFile;
puzzleFile.open(argv[2], ios::in);
solvePuzzle(puzzleFile);
puzzleFile.close();
} else if (strcmp(argv[1], "-c") == 0 && atoi(argv[2]) > 0 && atoi(argv[2]) <= sudokuMaximum) { //Create puzzle file
fstream sudokuFile;
sudokuFile.open("sudoku.txt", ios::out);
createSudoku(sudokuFile, atoi(argv[2]));
sudokuFile.close();
} else {
reportError();
}
这里在打开文件之后没有检查文件是否打开,建议使用is_open()方法来检查是否打开。
4.2参数传递有无错误，字符串的长度是字节的长度还是字符（可能是单/双字节）的长度，是以0开始计数还是以1开始计数？
- 无错误。
- 字节的长度，其中只涉及到ascii码，不涉及双字节字符。
- 从0开始计数。
4.3 边界条件是如何处理的？Switch语句的Default是如何处理的？循环有没有可能出现死循环？
- 边界条件1.1中测试过，基本没有问题。
- 没有Switch语句
- 除了一个while循环，其他函数都是for循环的形式，while循环的判断式中的变量在循环体内自增，所以没有死循环，for循环的限定条件和变量的增减都有明确的定义，所以没有可能出现死循环。
4.4 有没有使用断言（Assert）来保证我们认为不变的条件真的满足？
没有
4.5 对资源的利用，是在哪里申请，在哪里释放的？有没有可能导致资源泄露（内存、文件、各种GUI资源、数据库访问的连接，等等）？有没有可能优化？
代码中有3次用new关键字进行的资源的申请，下面是其中一处：
SudokuSolver solver = SudokuSolver();
DLXNode* listHead = new DLXNode();
vector<int> answer;
这三处申请都没有释放。
考虑到申请的不是数组，并且是有限次申请，所以导致资源泄露的可能性比较小。
但还是建议应该在程序结束之后释放相应的资源。
由于是有限的new，所以优化的空间比较小。
4.6数据结构中是否有无用的元素？
没有，DLXNode中定义的成员都有明确的使用目的。
5.效能
5.1 代码的效能（Performance）如何？最坏的情况是怎样的？
在1.1中我们看到，代码的效能比较不尽人意，生成50000个数独就要60秒，而解50000个数独的时候没有生成相应的文件。
5.2 代码中，特别是循环中是否有明显可优化的部分（C++中反复创建类，C#中string的操作是否能用StringBuilder 来优化）？
没有，程序的热路径都在DLX的递归调用上。
5.3 对于系统和网络调用是否会超时？如何处理?
会超时，判断应该是算法递归层数加深变得无法处理，应该进一步优化算法。
6.可读性
1.3中已经有过阐述，总体来说比较易读，推荐对每个类进行简单的功能和性质说明。
7.可测试性
作者基于简单的数据规模测试了3个核心的功能函数，即和生成和求解数独有关的3个函数。
但这样测试的粒度过大，会导致发生错误的时候定位难，同时测试中也没有包括一些异常检测和压力测试。
建议添加相应的测试，并将核心方法分解成更细粒度的方法进行测试。
二、设计一个代码规范
1.cpplint提供的代码规范和自己代码规范的不同
（1）是否添加copyright
（2）在include .h头文件的时候应该声明相应的路径
（3）建议使用using-declarations，举例来说就是，把cout替换成std::cout。
（4）用int64而不是long,long是c中的类型
（5）//和注释之间应该有一个空格
（6）建议采用K&R风格的大括号，我的代码中大部分使用这个风格，只有定义函数的时候会把包围函数的左大括号换行，cpplint中也只是使用了'almost'这个词来限定这条规则
（7）用空格来代替tab
（8）’，‘之后应该空格
（9）行结束之前有多余的空格
（10）else放在if的有大括号之后
（11）代码块结束之后有冗余的空行
2.之前没想到的规范以及规范意义的思考
（1）添加copyright
加作者还是比较有必要的，毕竟是自己的劳动成果。
（2）声明.h文件的时候应该声明相应的路径
这条没想到过，不过思考了一下确实有一定的必要性，会避免一些二义性问题。
（3）using-declarations
这个规则在有多个命名空间的时候很重要。
（4）用int64而不是long
C++下应该统一使用C++的类型
（5）//和注释之间应该有一个空格
这个应该影响不大
（6）空格代替tab
这个问题在构建之法中也提到过，会影响程序的可读性，之前没有注意，之后会改进。
（7）’，‘之后应该空格
之前没有想到，但似乎不影响可读性就可以。
（8）行结束之前有多余的空格
这个问题没有想到，但是其中的意义有什么意义呢？
（9）代码块结束之后有冗余的空行
同（8），这个规则是为了让代码块更紧凑吗？
3.自己设计的代码规则
我和partner商量之后定下了下面的规则：
风格规范：
缩进格式：4个空格
行宽：80个字符
左括号位置：K&R风格
命名风格：小驼峰式命名
长语句的换行：不影响可读性即可
指针声明格式：星号紧挨类型
public / protected / private 顺序
return之后的表达式是否加圆括号：必要时才加括号
命名空间内代码是否缩进：不缩进
水平留白：运算符前后留白，函数的参数之间不留白
垂直留白：函数之间留白不超过一行，函数体内留白处加注释
注释：每个函数加注释
设计规范：
是否支持goto语句：不支持
是否支持struct：不支持
是否支持运算符重载：不支持
变量定义与初始化是否同时完成：是
