[BUAA_SE_2017]代码复审-Week2

代码复审

CheckList

1.概要部分	代码能符合需求和规格说明么？	符合，经过-c及-s合法参数测试，程序均能生成、求解相应数独。
	代码设计是否有周全的考虑？	对于非法输入，程序处理不够周全。 -c 100000000 电脑死机无法动弹 -c -1 依旧会生成一个数独至Sudoku.txt中 -s 非法内容文件 程序错误退出
	代码可读性如何？	通过清晰的变量名称以及明确分工的函数，代码可读性非常高。
	代码容易维护吗？	代码容易维护，函数分工相当明确、逻辑清晰，因此函数之间耦合度不高，维护难度也不高。
	代码的每一行都执行并检查过了吗？	程序中几乎没有冗余的代码，在输入合法以及一些不合法的情况下，代码每一行基本都执行到。
2.设计规范部分	设计是否遵从已知的设计模式或项目中常用的模式？	没有用到设计模式，运用了面向对象的思想（虽然就一个类），充分实现了功能。
	有没有硬编码或字符串/数字等存在？	存在，设置为局部常量，用以判断某格数字合法性。 const int complete = 0x3fe;
	代码有没有依赖于某一平台，是否会影响将来的移植（如Win32到Win64）？	个人感觉并不影响：程序中include的都是C++ 标准库头文件，对移植无影响。 #include "SudokuSolver.h" #include <cstdio> #include <utility> #include <cassert> #include <ctime> #include <iostream> #include <cstdlib>
	开发者新写的代码能否用已有的Library/SDK/Framework中的功能实现？在本项目中是否存在类似的功能可以调用而不用全部重新实现？	不能，程序中的函数分工已经非常明确，并且各处已最大化利用已有函数，因此代码冗余少。
	有没有无用的代码可以清除？（很多人想保留尽可能多的代码，因为以后可能会用上，这样导致程序文件中有很多注释掉的代码，这些代码都可以删除，因为源代码控制已经保存了原来的老代码。）	程序设计者再最后一次commit前自我进行了一次冗余代码清楚的工作，因此程序中不存在大段注释掉的代码，但有两行被优化被注释了的代码存在： std::string SudokuSolver::generateN(int n, SudokuBoard &board) {     std::string r;     //for (int i = 0; i < n; i++)         //r += generate(board).toString() + ' ';     _solveCount = 0;     _solveLimit = n;     solutions = new std::vector>();     generate(board);     for (auto& s : *solutions)         r += *s + ' ';     delete solutions;     return r; }
3.代码规范部分	修改的部分符合代码标准和风格么（详细条文略）?	代码风格整体一致，一目了然，较为清晰。但是for循环或if语句之后存在不加花括号的情况，与其它多行for或if有一些矛盾，有待改进。
4.具体代码部分	有没有对错误进行处理？对于调用的外部函数，是否检查了返回值或处理了异常？	进行了错误处理： try {     n = std::stoi(arg);     printf("%d ", n); } catch (...) {     printf("invalid argument %s", arg);     return 0; } 对外部调用函数，也检查了返回值：如strcmp并判断返回值与0的大小关系、fopen并判断返回值是否为NULL等。
	参数传递有无错误，字符串的长度是字节的长度还是字符（可能是单/双字节）的长度，是以0开始计数还是以1开始计数？	字符串均是char*来实现，无误。
	边界条件是如何处理的？Switch语句的Default是如何处理的？循环有没有可能出现死循环？	除了遍历数独边界条件设置9*9之外，其它遍历方式均采用c++的for(:)遍历模式，且循环体中不会更改循环条件，因此不会产生死循环。
	有没有使用断言（Assert）来保证我们认为不变的条件真的满足？	仅在单元测试中使用了assert来进行判断。
	对资源的利用，是在哪里申请，在哪里释放的？有没有可能导致资源泄露（内存、文件、各种GUI资源、数据库访问的连接，等等）？有没有可能优化？	一个较典型的例子： for (auto p : puzzles) {     SudokuBoard *s = solver.solve(*p);     fprintf(output, "%s ", (*(s->toString())).c_str());     delete s; } 资源需要时申请，无需时释放做的较好；但是由于在-c情况下对第二个参数的判断不足，导致_solveLimit变量可以非常大，驻留在内存中的char过多，导致了占用完所有内存（电脑就这么挂了）。可以通过设置阈值来阶段性输出，释放内存空间。
	数据结构中是否有无用的元素？	没有，整体思路采用二进制来实现，没有冗余元素。
5.效能	代码的效能（Performance）如何？最坏的情况是怎样的？	因为该程序生成与求解采用同一套算法，经过优化剪枝后，该回溯算法效能表现较好，但是求解的时间还是较长，原因在于每一次生成只需要回溯即可产生不同解，而每一次求解需要重新遍历才能求出答案，当数独仅存在唯一解时（最坏情况），耗时最长。
	代码中，特别是循环中是否有明显可优化的部分（C++中反复创建类，C#中string的操作是否能用StringBuilder 来优化）？	该程序由c++实现，存在反复创建类的情况，但是程序运行分析中该创建时间耗时并不显著，无须优化。（在此提出一点，优化策略是每次寻找可行最小的那个格子进行填充，但每次寻找需要遍历整个数独，或许在这上面耗时过多？）
	对于系统和网络调用是否会超时？如何处理?	不会超时，程序仅对文件进行读写，并判断了文件结束符，因此不会；不存在网络调用。
6.可读性	代码可读性如何？有没有足够的注释？	代码可读性强：变量、函数命名准确；代码风格总体一致，逻辑清晰有序；按嵌套使用缩进，容易阅读、修改；关键地方有注释，言简意赅。
7.可测试性	代码是否需要更新或创建新的单元测试？	需要进行更新单元测试，用以检验命令行参数的合法性，避免无效输入或无效文件格式。
	还可以有针对特定领域开发（如数据库、网页、多线程等）的核查表。	无。

设计代码规范

个人项目使用C#语言开发，因此选用StyleCop来进行检查。

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

  • 所有方法名称需以大写开头；
  • 判断语句各部分需加上括号；
  • 注释语句需加在每行一个空格后，而我认为在哪里都可以，只要便于提示；

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

  • private成员变量要全部定义在public成员变量之后；
  • 所有public成员变量必须定义为private；
  • The variable name 'sHeads' begins with a prefix that looks like Hungarian notation；
  • if、for、while与左括号之间需加一个space；
  • Statements or elements wrapped in curly brackets must be followed by a blank line；

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

  • 通过StyleCop进行了检查，每一个.cs文件几乎都能发现80+处不符合代码规范的地方，大体分为这么几类：
    • Spacing：运算符、关键字前后一空格，借此可以将其完全区分出来，提高可读性；
    • Layout：花括号换行问题，尚不清楚为何右花括号需要换行后多空一行；
    • Naming：不使用匈牙利命名法；
    • Readability：成员变量前加上this;
    • Maintainability：将类成员变量定义为private;使用小括号清晰界定运算符优先级。便于维护。
    • Ordering & Documentation：涉及到调用外部库的using顺序；需添加文档。
  • 这些规范是有意义的。从程序可读性角度来说，关键字、运算符以及花括号的使用规范，很大程度上决定了代码的格式，这些要求是必须遵守的；其次，从程序可维护的角度来看，代码的书写要尽可能提升逻辑表达的清晰度，即一目了然，无须再根据代码进行人为的第二次理解或计算（容易造成错误，使得debug时又衍生出更多bug）；最后，从文档角度来说，代码规范不仅体现在代码中，对代码功能的说明更加重要，因此文档也是代码规范一个不可或缺的部分。

• 代码规范：

  • Spacing, Layout, Readability, Maintainability 以及 Ordering & Documentation是必须严格遵守的，但是对于Naming而言，究竟使用匈牙利命名法还是其它命名法，有待深究。对于C#来说，每一个变量类型都是需要定义的，使用匈牙利命名法或许显得多余；但对于其它语言来说，可能变量类型不需要定义…… Naming应该根据项目的不同来进行规定。