[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. 變量定義與初始化是否同時完成：是