[buaa-SE-2017]我的做业-Week2

我的做业-Week2

---

1、代码复审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个函数。
但这样测试的粒度过大，会致使发生错误的时候定位难，同时测试中也没有包括一些异常检测和压力测试。
建议添加相应的测试，并将核心方法分解成更细粒度的方法进行测试。

---

2、设计一个代码规范

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商量以后定下了下面的规则：

风格规范：

1. 缩进格式：4个空格

2. 行宽：80个字符

3. 左括号位置：K&R风格

4. 命名风格：小驼峰式命名

5. 长语句的换行：不影响可读性便可

6. 指针声明格式：星号紧挨类型

7. public / protected / private 顺序

8. return以后的表达式是否加圆括号：必要时才加括号

9. 命名空间内代码是否缩进：不缩进

10. 水平留白：运算符先后留白，函数的参数之间不留白

1. 垂直留白：函数之间留白不超过一行，函数体内留白处加注释

2. 注释：每一个函数加注释

设计规范：

1. 是否支持goto语句：不支持

2. 是否支持struct：不支持

3. 是否支持运算符重载：不支持

4. 变量定义与初始化是否同时完成：是