〇、comment链接
https://github.com/hanayashiki/Sudoku/issues/1
一、代码复审
1.概要部分
(1)代码能符合需求和规格说明么?
经测试,对于合法输入,代码能以要求的格式给出正确的结果。
(2)代码设计是否有周全的考虑?
对于错误输入考虑不太周全。-c中在数字中插入字母(如“2a2”)也可以识别,但个人感觉认定为非法输入比较好。如果只输入-c或-s,不输入第三个参数,程序会crash;如果输入-s,但第三个参数是非法路径,也会crash。对于其它类型的非法输入有判断。
(3)代码可读性如何?
可读性较好,在下面可读性部分详细说。
(4)代码容易维护么?
容易维护。首先代码分为了若干的.cpp文件,比起把所有类和函数放在一个文件中,这种方式更便于查找需要的类和函数,清晰了不少。而且,代码的封装性很好,函数功能很明确,改动代码的代价很小。
(5)代码的每一行都执行并检查过了吗?
检查过了。
2.设计规范部分
(1)设计是否遵从已知的设计模式或项目中常用的模式?
没有使用设计模式。
(2)有没有硬编码或字符串/数字等存在?
有,代码中有很多3和⑨。Matrix类中也有如下代码:
1 const int MASK1 = 0x55555555;
2 const int MASK2 = 0x33333333;
3 const int MASK4 = 0x0f0f0f0f;
4 const int MASK8 = 0x00ff00ff;
5 const int MASK16 = 0x0000ffff;
还有关于学号的部分……
1 matrix.fill_in_figure(1, 1, (7 + 7) % 9 + 1);
或许这些数字以宏定义的形式注明会更好。
(3)代码有没有依赖于某一平台,是否会影响将来的移植(如Win32到Win64)?
依赖于Windows,但不影响移植。
(4)开发者新写的代码能否用已有的Library/SDK/Framework中的功能实现?在本项目中是否存在类似的功能可以调用而不用全部重新实现?
不存在的。
(5)有没有无用的代码可以清除?
有很多被注释掉的代码,有一部分是cout,用于测试时输出,建议通过调试的手段替代频繁的cout。
3.代码规范部分
(1)修改的部分符合代码标准和风格么(详细条文略)?
两个代码风格完全不一致的函数……
1 bool read_file(FILE* f, int sudoku[9][9])
2 {
3 int readin = 0;
4 for (int i = 0; i < 9; i++)
5 {
6 for (int j = 0; j < 9; j++)
7 {
8 int d;
9 readin += fscanf_s(f, "%d", &d);
10 sudoku[i][j] = d;
11 }
12 }
13 //matrix.display();
14 //cout << "readin = " << readin << endl;
15 return readin == 81;
16 }
1 void shuffle(char* str) {
2 int l = strlen(str);
3 for (int i = 0; i < l; i++) {
4 int r = random(l);
5 char randc = str[r];
6 char tmp = str[i];
7 str[i] = randc;
8 str[r] = tmp;
9 }
10 //cout << str << endl;
11 }
4.具体代码部分
(1)有没有对错误进行处理?对于调用的外部函数,是否检查了返回值或处理了异常?
调用fopen_s函数后没有对文件指针进行NULL判断。
(2)参数传递有无错误,字符串的长度是字节的长度还是字符(可能是单/双字节)的长度,是以0开始计数还是以1开始计数?
无错误,字节长度,以0和1开始倒着计数的都有。
大部分都是像shuffle函数这样以0开始计数:
1 void shuffle(char* str) {
2 int l = strlen(str);
3 for (int i = 0; i < l; i++) {
4 int r = random(l);
5 char randc = str[r];
6 char tmp = str[i];
7 str[i] = randc;
8 str[r] = tmp;
9 }
10 //cout << str << endl;
11 }
这是find_lock中的部分代码,以1开始计数:
1 for (int i = 1; i <= 9; i++) {
2 and_result_rc = and_result_rc >> 1;
3 if ((and_result_rc & 0x1) == 0x1) {
4 //std::cout << "cut 1" << std::endl;
5 return i;
6 }
7 }
(3)边界条件是如何处理的?Switch语句的Default是如何处理的?循环有没有可能出现死循环?
大部分采用i<n的形式判断,少数采用i<=n的形式判断。没有switch语句,但有类似的if-else语句,如main函数中就用else对未知的任务种类进行报错。不会出现死循环。
(4)有没有使用断言(Assert)来保证我们认为不变的条件真的满足?
没有。
(5)对资源的利用,是在哪里申请,在哪里释放的?有没有可能导致资源泄露(内存、文件、各种GUI资源、数据库访问的连接,等等)?有没有可能优化?
Change是通过clean函数释放资源,Node是在DLX的析构函数中被释放
(6)数据结构中是否有无用的元素?
不存在的。
5.效能
(1)代码的效能(Performance)如何?最坏的情况是怎样的?
生成1000000个数独需要54.18s,解决50000个数独需要49.189s,不是很理想。
(2)代码中,特别是循环中是否有明显可优化的部分(C++中反复创建类,C#中string的操作是否能用StringBuilder 来优化)?
在create任务中会创建许多Change储存数独以进行回溯,这部分可以优化为在一个Change上通过填数和撤销填数进行变更。
(3)对于系统和网络调用是否会超时?如何处理?
没有用到系统和网络调用。
6.可读性
(1)代码可读性如何?有没有足够的注释?
可读性较好,代码重要的部分有一些注释,而且绝大部分函数名、变量名都很得当,这是我要学习的地方。
main的read_argv取名不太恰当,由于argv共有三个参数,而这里只用到了argv[1],所以取argv这个名字明显不太合适。这个函数的目的是判断任务类型,个人认为不如将函数名设置成read_mission更为得当。
有些比较精巧的行列计算可以define一个预定义函数,如:
1 new_row[81 + 81 + 81 + (3 * (i / 3) + j / 3) * 9 + fill - 1] = 1;
这一行代码在sudoku_m.cpp的sudoku2matrix函数中,其中(3 * (i / 3) + j / 3)的目的是计算计算宫的编号,如果不加注释理解起来比较困难,这种问题在我的代码中也有出现。
二维链表那一部分理解起来感觉有些痛苦,建议多加点注释……
create函数开头用注释简要介绍了一下思路,好评,十分便于理解。
另外,综合了我之前写的代码,个人认为在以下部分增添注释可大大增加代码可读性:
①类的Overview;
②类成员的意义;
③函数参数及返回值的意义;
④for/while循环的意义,一次循环究竟代表着什么?
以上部分是我在阅读代码时最容易犯晕的地方,就连自己写代码有时也会犯糊涂,应当加以重视。
7.可测试性
(1)代码是否需要更新或创建新的单元测试?
最好添加一个验证数独重复性的功能。
(2)还可以有针对特定领域开发(如数据库、网页、多线程等)的核查表。
本次作业不存在的。
二、设计一个代码规范
1.工具提供的代码规范和你个人的代码风格有什么不同?
和Cpplint提供的代码规范相比,我的代码有以下不同:
·没有copyright
·有长度大于80个字符的行
·使用Tab进行缩进
·逗号后面要空格,大括号前面要空格,大于小于号周围要空格(我空了啊……为什么报错?)
·分号后面要空格(为什么?)
2.工具提供的代码规范里有哪些部分是你之前没有想到的?
只有copyright没有想到。
3.为什么要这样规范?这样的规范有意义吗?
意义肯定是有的,我觉得规范的意义一是在于增加代码可读性,二是在团队项目中保持代码风格的一致性。
其实我认为不管采用哪种规范,满足保持一致性、便于阅读这两点就好,其余的可以随机应变,不应该硬性强求。很早以前就在网络上看到过有关Tab/Space、大括号换不换行之类的问题,其实无论哪种选择都各有利弊,和个人习惯有很大关系。比如大括号换行的问题,换行的好处是层次更清晰,不换行的好处是代码更紧凑。我之前的编程习惯是不换行,很幸运,这一点和cpplint的标准一样,但之前也看到过很多老师偏爱大括号换行,感觉问题也不大,但让我这样写我就会很不适应。类似的还有goto语句,看了其它同学的博客,cpplint禁止使用goto语句,这一点和《构建之法》上的说法不符。我承认goto是有很大的弊端,但是只要保证不改变代码的顺序性,用于跳出多层循环是一个非常好的选择。因此我认为,代码规范并不绝对,应该随机应变,编程者应当参考但不应当受此禁锢。
但不管怎样,在同一个项目中应保持代码风格的一致性,不能“Space和Tab共存,骆驼命名与下划线命名齐飞”,不然会带来很糟糕的代码阅读体验……
4.总结的代码规范
·每行长度小于等于80字符
·大括号不换行
·使用Tab,但设置Tab自动替换为四个Space
·所有if、for或while等涉及大括号的语句,不要省略大括号,也不要在同一行写两条语句,以便随时增减代码
·下划线命名
·类名、结构体名首字母大写
·所有声明放在.h文件中,所有定义放在.cpp文件中