0x01 :代碼規劃的要求ios
Q:這些規範都是官僚制度下產生的浪費你們的編程時間、影響人們開發效率, 浪費時間的東西。(反駁)程序員
首先,咱們須要明確編碼規範的定義,編碼規範同時包括了編碼風格和其它規範(代碼設計上的規範,如設計模式、程序設計、模塊之間的邏輯關聯等)。正則表達式
編碼風格,牽扯到「縮進、空格使用、註釋、命名習慣」等多方面的因素,是依致特定編程語言制定的軟件工程開發的「約定」,而相同的編碼風格,可使得軟件開發過程當中輕鬆瀏覽任意一段代碼,充分保證不一樣的開發人員可以依據統一的編碼格式輕鬆理解代碼的邏輯層次,內在含義(好比匈牙利命名法)等,所以,簡而言之,代碼風格的規範會帶來三個較爲明顯的好處:算法
代碼更易維護:軟件工程中程序的開發、維護和測試必然會由不一樣的開發人員處理,所以依據統一的代碼風格,能夠快速推斷出代碼的做用,變量的實際含義,甚至能準肯定位到某一引入的包,有效提升代碼的可維護性。編程
保證代碼集體全部制,避免交流壁壘:代碼集體全部制,可以有效增大巴士因子(一個項目能承受多少個程序員被車撞了而不影響項目的正常進行),即不會由於少數深刻理解代碼、掌握大量信息的人暫時沒法工做而使得項目停滯。設計模式
消除長久的格式紛爭:使得開發人員沒必要由於編碼風格的不一樣而爭吵,保證軟件工程的開發效率。數組
代碼設計上的規範,包含函數的設計規範,goto語句的跳轉,錯誤處理等方面的問題,而缺少必定的代碼規範也將急劇增長代碼可維護性。好比goto語句的大量濫用,「上帝」函數或「重複需求」函數的編寫,也將致使軟件工程開發效率的急劇下降,究其根源,軟件開發並非我的興趣軟件的開發,須要保證開發人員之間沒有溝通壁壘,而統一的編碼規範將規避這一問題。數據結構
Q:我是個藝術家,手藝人,我有本身的規範和原則。(反駁)app
藝術家不等於滑稽家,就像在多數狀況下有人說「畫筆嚴重拘束了咱們創造力,我要用手指塗鴉,這是我技能和創造力的體現」,可能常人也會詫異通常。對於開發人員,「本身的規範和原則」是編碼過程當中長期養成的習慣,所以懷有自負的心理是機器正常的。但在軟件工程的團隊協做中,編碼風格並不影響自己思想的展現,思想、創造力、算法並不會由於編碼風格的不一樣而受到影響;但編碼風格的「特立獨行「將致使其餘開發人員沒法理解你的代碼,下降了總體的可維護性,就像藝術家由於他人沒法理解做品的編碼形式而受到影響。框架
Q:規範不能強求一概,應該容許不少例外。(反駁)
咱們必須認可,對於一家公司的不一樣項目,擁有不一樣的編碼規範是無可厚非的(好比對於C#這種強類型的語言,匈牙利命名法可能並不是必需);可是編碼規範對於某一項目而言不是最理想的,但對於大型公司而言,統一的編碼風格能使得公司自己更容易對這一項目進行維護。所以,例外是能夠存在的,好比,在某一項目中,咱們須要遵照統一的編碼規範,但特定的項目,能夠擴展特定的項目規範和結構。
Q:我擅長制定編碼規範,大家聽個人就行了。(反駁/支持)
這種狀況的須要分狀況討論:
咱們必須認可,在大多數狀況,統一制定的編碼規範是較爲理想的或是合理的,而這種抱怨的直接緣由,可能就是「我認爲這種編碼規範近似與小學生制定的,嚴重下降了個人代碼質量「這種傲慢自大的心理,由於代碼風格雖然說可能會在必定程度上下降你的積極性,但總體而言,程序較強的可讀性會大大彌補這一損失。
在少部分狀況,咱們必須認可,荒謬的編碼規範會組織一個優秀的程序員寫出優秀的代碼,這是,就須要你我的的努力去說法這一愚蠢的想法,但如果從新制定編碼規範,對歷史代碼而言恐怕將會產生巨大的工做量。
0x02 :同伴代碼複審狀況
0x0204:std::getline()從VS2012到VS2013
while (getline(ex, formula) == NULL
&& getline(an, answer) == NULL) {
}
在Visual Studio 2012中編譯經過而在2013版本中編譯器提供了C1903的錯誤提示:"fatal error C1903: unable to recover from previous error(s); stopping compilation",而將「== NULL」刪去後從新編譯,程序編譯經過且功能與此前VS2012編譯的exe文件功能相同,這裏咱們翻閱MSDN提供的C++ Standard Library來查詢std::getline()的實現過程。
template<class _E, class _TYPE, class _A> inline
basic_istream<_E, _TYPE>& getline(
basic_istream<_E, _TYPE>& Istream,
basic_string<_E, _TYPE, _A>& Xstring,
const _E _D=_TYPE::newline( )
);
以上代碼段摘自https://msdn.microsoft.com/en-us/library/3ah895zy(v=vs.110).aspx,咱們能夠看到VS2012中平臺工具集仍使用V110,而具體的範例合併於string:getline()中,簡述言之即爲由input stream中返回一段字符串;
l At end-of-file, in which case the internal state flag of is is set to ios_base::eofbit.
l After the function extracts an element that compares equal to delim, in which case the element is neither put back nor appended to the controlled sequence.
l After the function extracts str.max_size elements, in which case the internal state flag of is is set to ios_base::failbit.
l Some other error other than those previously listed, in which case the internal state flag of is is set to ios_base::badbit
以上信息段摘自https://msdn.microsoft.com/en-us/library/2whx1zkx(v=vs.120).aspx,咱們能夠看到VS2013平臺工具集使用V120,而在實現過程當中調用ios_base::iostate的有效位實現更廣範圍的檢查,而關於返回值的問題,「The istream returned by getline() is having its operator void*() method implicitly called, which returns whether the stream has run into an error. As such it's making more checks than a call to eof().「,stackoverflow上存在一段很是精煉的回答,即在發生NULL的相似錯誤時經過錯誤信息的置位來實現,與2012相比作出了更多的檢查,而MSDN文檔給出的範例也首次使用以下代碼段來解釋說明這一新特性:
while (getline(cin, str)) {
v1.push_back(str);
}
這裏特別鳴謝濤神,http://home.cnblogs.com/u/-OwO-/,能幫忙調試C1903的error提示,並最終在本身「手殘重載工程「的時候想到平臺工具集的變化,並最終發現VS工具集API的優化措施(Version2012-2013的變化)
0x0208:總體函數代碼初步審查簡述
函數定義 |
功能理解 |
初步審查 |
int cal(int fr1, int nm1, int fr2, int nm2, char op) |
(fr1/nm1) {op} (fr2/nm2) = (lastfr/lastnm) |
|
int cal(int fr1, int nm1, int fr2, int nm2, char op) |
首先調用int_or_div函數將獲取的answer字符串存儲爲(int_a + fr_a/nm_a)的形式(測試3),隨後經過逐字符求解逆波蘭表達式計算formula的值(測試4),並與此前的結果進行比較 |
|
void initial() |
初始化全局變量 |
|
int int_or_div (string element) |
解析數字字符串,並識別整數、真分數、帶真分數分別返回1,2,3的值,而在識別失敗時返回0(測試5) |
建議經過正則表達式解析,而沒必要逐字符串識別,致使if-else的高測試成本 |
int isnumber(string str) |
將數字字符串轉換爲數字(測試6) |
徹底能夠調用atoi()函數,沒必要手動實現 |
vector <string> mid_2_post (string formula) |
將中綴表達式轉換爲後綴表達式(測試7:優先級設置測試),同時添加"#"符號做爲表達式的結尾,方便後續計算的終止(測試8) |
|
int newdiv(int range) |
生成非零除數(測試9) |
|
char newop() |
生成隨機運算符 |
switch-case結構無實現必要,可直接初始化final類型的數組,而後直接經過下標訪問 |
int priority (string op1, string op2) |
運算符優先級比較 |
switch-case結構無實現必要,直接構建二維數組,且數組值正負表示比較結果 |
void reduc(int fra, int nmt, int exe_or_ans) |
完善(lastfr/lastnm)的取值(設置必定參數方便後續修改),如分母爲0(測試10),分母爲負值(測試11),對非最簡分數的化簡(測試12);同時經過exe_or_ans的參數實現取值的更改(測試13) |
switch-case結構略有些濫用,致使代碼可讀性略差,且case內部分代碼重複冗雜; |
void newformula(int num, int range) |
建立表達式,形如{Number}{Op}{Number}{Op}…,並在除數爲0時生成新除數,因爲同時的括號生成保證了運算符一定從左到右運算,所以沒必要考慮A/(B-B)的生成(測試14) |
查重的檢查並未出如今程序中,所以,這裏程序的正確性存在必定的疑問 |
vector <string> split(string str, string spl) |
以spl爲「分割符」將目標字符串str分割,存儲於vertor<string>中返回 |
徹底能夠調用strtok()函數實現所需功能 |
int main() |
對讀取輸入並經過if-else結構解析,同時調用此前定義的函數實現表達式生成和判斷功能 |
存在部分筆誤,具體狀況將進一步詳細說明;功能略有欠缺 |
*關於具體的測試和代碼複審將在0x020c進行進一步的詳解
0x020c:General層次代碼複審
[N] Does the code work? Does it perform its intended function, the logic is correct etc
首先,從功能實現的角度,存在五方面的問題
ü 「使用 -r 參數控制題目中數值(天然數、真分數和真分數分母)的範圍,該參數能夠設置爲1或其餘天然數。該參數必須給定,不然程序報錯並給出幫助信息「,首先,程序在功能上容許參數殘缺,即MyApp.exe –r 10或MyApp.exe –n 10均能正確生成,與需求不符,但這裏極可能是我的對需求有着不一樣層次的理解,並且更改很是簡單,只需刪除」case 3「的代碼塊,所以,代碼工做能夠斷定爲」正常「,若修改,則只需對main()函數的中的switch-case結構進行刪減便可
ü 「程序一次運行生成的題目不能重複「,功能上未進行查重的檢測工做,致使會生成重複表達式,與功能需求相違背,這裏補充說明一處筆誤:
if (args[1] == "-n") {
} else if (args[1] == "r") { /*should be -r*/
}
圖 1 :輸入」Arithmetic1.exe –n 10 –r 2」時重複表達式的輸出
ü 「參數輸入時能夠交換-r和-n的輸入順序,而程序能保持正常運行「,而程序在交換-r和-n順序後沒法正常運行
圖 2 :輸入順序爲"-R -N"時斷定爲無效輸入
圖 3 :輸入順序爲"-N -R"時斷定爲正常輸入
ü 「生成的題目中計算過程不能產生負數「,而顯然在以下樣例中」Arithmetic1.exe –n 10 –r 10「中,出現了負數,但在最終運算上結果正確,所以表達式支持負數運算是功能擴展,但生成題目與需求不符
圖 4 :輸入」Arithmetic1.exe –n 10 –r 10」時負數的出現
ü 在main()函數中能夠發現,程序將number限定在[1,9999]區間,將range限定在[1,100]區間,所以對於range參數和number參數在極大和極小狀況的輸入下程序會打印錯誤信息:「Invalid exercise number「或」Invalid range number「等提示信息,而對於」2.3「」$TEAMORL^」等錯誤輸入也能準確斷定,所以,程序的魯棒性符合要求
ü 程序的運算符與需求不符,應輸入輸出Unicode編碼爲’\u00D7’和’\u00F7’的字符,而將其替換爲’*’’/’運算符,所以,在執行」Arithmetic1.exe –e exercises.txt –a answer.txt」會返回錯誤信息(exercises.txt文件中符號爲’\u00D7’時)
[N+] Is all the code easily understood?
雖然程序自己嵌套了大量switch-case結構和if-else結構致使代碼自己冗雜,但因爲各部分函數的命名比較清晰,因此能較快分辨各函數功能,並最終推測和驗證出各函數的功能,但這裏摘取一部分代碼片斷:
case '+':
case '-':
switch (c2) {
case '+': return 2;//highter
case '-':return 2;
case '*': return 0;//lower
case '/': return 0;
case '(': return 0;
case ')': return 2;
case '#': return 2;
default: return -1;//cannot be compared
}
這裏徹底能夠構建一個二維數組來實現以下圖的比較功能,但函數名int priority (string op1, string op2)能夠較爲清晰理解出這是運算符的優先級的比較函數,所以,若是刪除相似的switch結構,重構相似上圖的冗長代碼,封裝可能的功能模塊,會使得程序的可讀性進一步加強
[Y] Does it conform to your agreed coding conventions? These will usually cover location of braces, variable and function names, line length, indentations, formatting, and comments
程序自己的代碼風格相對較好,總體使用TAB縮進,函數名命名和程序內變量命名符合必定規律,但仍有必定的「二義性「,建議使用匈牙利命名法方便閱讀,同時全局變量前加入」global_「標識符做爲提示可能會使得程序的代碼風格有着進一步的提高
[Y] Is there any redundant or duplicate code?
程序自己存在冗雜和重複的代碼片斷,
int number = isnumber(args[2]);
if (number <= 0 || number > 9999) {
cout << "Invalid exercise number." << endl;
}
int range = isnumber(args[4]);
if (range <= 0 || range > 100) {
cout << "Invalid range number." << endl;
}
在輸入數據的特判中,此代碼段重複出現至少4次,強烈建議將封裝爲void InputJudgement(string range, string number)函數統一進行,同時,程序主體的代碼段一樣出現相似問你,也可封裝爲void ProgramRun()函數使得main函數內不至於存在大量大量的if-else結構快,且長達120行
[Y] Is the code as modular as possible?
除去剛纔說起的「redundant or duplicate code「外,程序自己的模塊化程序較好,首先聲明,這裏所說的」模塊化「是指程序各部分的功能劃分清晰而不多存在交叉重複的功能;但程序自己經過全局變量的反覆初始化實現答案的計算和存儲,而缺乏肯定的數據結構來存儲分數,使得程序自己的擴展性大大下降,而單文件的方式仍是有欠考慮,須要重構,能夠參考以下的數據結構
typedef struct _factor{
long denominator;
long numerator;
}factor;
[N] Is there any commented out code?
不存在與調試相關的代碼,但我的傾向於保留調試相關代碼並作出以下結構的定義,這在後期的輸出檢查中將極其方便地觀察到各部分的運行情況
#define DEBUG 0
if (DEBUG) {
}
[Y] Do loops have a set length and correct termination conditions?
循環設置的長度和條件都可以吻合,且符合程序自己的邏輯
[Y] Can any of the code be replaced with library functions?
ü int int_or_div (string element),函數解析數字字符串,並識別整數、真分數、帶真分數分別返回1,2,3的值,而在識別失敗時返回0,但因爲函數自己實現時是逐字符串讀入,嵌套了大量的if-else結構致使測試成本較高,且自己效率並不高,建議調用#include<boost/regex.hpp> ,即便用C++的Boost庫,從而精簡代碼,清晰功能
ü vector <string> split(string str, string spl),函數以spl爲「分割符」將目標字符串str分割,存儲於vertor<string>中返回,徹底能夠調用strtok()函數實現所需功能{char *strtok(char s[], const char *delim)}
ü int isnumber(string str),將數字字符串轉換爲數字,徹底能夠調用atoi()函數,沒必要手動實現
ü void reduc(int fra, int nmt, int exe_or_ans)中存在分子分母的化簡功能,採用了「for (int i = 2; fra >= i; ++i) 「的暴力求解方法,這裏建議修改爲標準的展轉相除法這種O(log)級的函數,模板以下圖:
int gcd (int a, int b)
{
return a <= b ? gcd(b,a)
:
a % b == 0 ? b
: gcd(b, a%b);
}
[Y] Can any logging or debugging code be removed?
能夠
0x0210:Security\Documentation\Test層次代碼複審
[Y] Are all data inputs checked (for the correct type, length, format, and range) and encoded?
這次全部的輸入檢查都進行了很好的檢查,由於全部的限制條件都經過逐字符的方式進行檢查,所以輸入的判斷上處理得很是全面,具體的特判說明詳見0x0208~0x020c的說明
[N] Where third-party utilities are used, are returning errors being caught?
未使用第三方庫,而程序中並未主動經過異常機制處理,而是經過參數的錯誤直接返回並終止程序,所以,檢查相對較爲完善,魯棒性較強
[Y] Are invalid parameter values handled?
[Y] Do comments exist and describe the intent of the code?
提供了精煉的README,說明在輸入上通過了慎重的考慮,通過驗證與文檔吻合,但部分功能仍偏離需求
[N] Are all functions commented?
並無有啓發性的註釋,總註釋篇幅不超過10行,所以程序的理解上相對較差,但因爲函數命名清晰所以致使程序可讀性不差
[N] Is any unusual behavior or edge-case handling described?
僅在代碼中體現,註釋中並無進一步描述
[N] Are data structures and units of measurement explained?
無,但各函數的功能劃分清晰
[N] Is there any incomplete code? If so, should it be removed or flagged with a suitable marker like ‘TODO’?
無,但部分能夠用庫函數處理的地方須要重構完善
[Y] Is the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
函數功能自己測試較爲容易,具體的功能測試也陳列在0x0208的表格中,總體思路較爲清晰
[Y-] Do tests exist and are they comprehensive? i.e. has at least your agreed on code coverage.
從README的文檔中能夠看出,進行了總體輸入輸出的測試,但具體的單元測試並無發現
[N] Do unit tests actually test that the code is performing the intended functionality?
[N] Are arrays checked for ‘out-of-bound’ errors?
在數組使用前,必定獲取其自己的長度來進行限制,所以,能夠在必定程度上避免OutOfBound的越界錯誤
0x03 :結對編程代碼選取
考慮到編程語言的熟練程度,和模塊化的程序,所以,最終編程決定放棄C++語言編寫的此程序,而將它其中的一些想法提煉出來用以重構,而選取面向對象的C#編程語言實現結對編程項目;今後次同伴的代碼複審結果分析,編程夥伴的異常情況的討論很是全面,在構造輸入時幾乎特判了所有異常狀況,在測試方面很是適合,所以最終在討論總體的框架後,採起開發—測試的最基礎的結對編程方式,由我完成基本模塊的編寫後,並完成必定的單元測試後,採用黑盒、白盒測試疊加的方式進行測試,同時,在後期XML文件的讀入和參數設定上由他設計,預留一部分接口來保證程序正確性。