結對編程初涉獵——結對夥伴的代碼復審
至此為止,個人作業階段就結束了,從此便進入團隊合作階段。這次是先從結對編程開始,雖然還沒有進入正式的開發狀態,但也是結對編程的小開端,同時也是一個復習代碼復審這部分內容的過程。
閱讀目錄
1.題目要求
2.結對體驗
3.代碼審查表
4.隊友代碼及優缺點評價
5.個人感想
題目要求
要求:
(1). 首先在同學中找一個同伴,範圍不限,可以在1~5班中隨意組合,建議盡量不要找同組的成員,女同學盡量找男同學結對,但是不做強制要求;
(2). 從以往個人完成的項目中選擇一個作品,例如:以往的數據結構課程設計或者其它具有比較完整功能的小系統,代碼至少要大於100行;
(3). 將代碼上傳至個人GitHub或Coding.net系統中
(4). 對同伴的作品進行代碼復審,並參照C/C++代碼審查表和Java代碼審查表 這兩篇博文的內容自行設計代碼審查表並填寫內容;
(5). 將對夥伴審查的結果以表格的形式寫到自己的博客作業裏,博客中應該附有夥伴作業的GitHub或Coding.net的代碼地址;
(6). 對同伴的代碼寫一篇500字以上的評論,介紹同伴的優缺點。
結對體驗
剛開始聽說有結對編程項目的時候,我其實內心還挺想和班級裏的男生一起組隊的,因為很多男生的編程動手能力確實比大部分女生的動手能力要強(當然這裏排除那些確實很厲害的女生),我想通過這個結對項目向男生吸取一些經驗,多多學習一些他們的優勢。
但是下課後,我的室友,也就是我這次的結對編程小夥伴雷鎵——一個性格豪放樂觀又開朗的女生,向我拋出了橄欖枝,想讓我作為我倆的領航者。其實在這裏我也有考慮多方面因素,包括我們倆人的編程能力(確實還有點擔心如果題目過於難,我們的實力到時候能否撐起這個編程項目,我能否把我們倆人的任務安排好等),但也有一些好處,我們倆是室友,編程時間可以隨時規劃,交流起來毫不費力等等......
所以,最後還是爽快地答應了,或許,兩個女程序員能擦出更好的火花呢?
代碼審查表
雷鎵同學的博客地址
雷鎵同學的代碼地址
代碼審查表如下:
功能模塊名稱 | 最短路徑求解 | ||
審查人 | 司佳宇 | 審查日期 | 2018.4.6 |
代碼名稱 | 最短路徑求解 | 代碼作者 | 雷鎵 |
文件結構 | |||
重要性 | 審查項 | 結論 | |
頭文件和定義文件的名稱是否合理? | 是 | ||
頭文件和定義文件的目錄結構是否合理? | 是 | ||
版權和版本聲明是否完整? | 否 | ||
重要 | 頭文件是否使用了 ifndef/define/endif 預處理塊? | 是 | |
頭文件中是否只存放“聲明”而不存放“定義” | 是 | ||
程序的版式 | |||
重要性 | 審查項 | 結論 | |
空行是否得體? | 是 | ||
代碼行內的空格是否得體? | 否 | ||
長行拆分是否得體? | 是 | ||
“{” 和 “}” 是否各占一行並且對齊於同一列? | 否 | ||
重要 | 一行代碼是否只做一件事?如只定義一個變量,只寫一條語句。 | 否 | |
重要 | If、for、while、do等語句自占一行,不論執行語句多少都要加 “{}”。 | 否 | |
重要 | 在定義變量(或參數)時,是否將修飾符 * 和 & 緊靠變量名?註釋是否清晰並且必要? | 無 | |
重要 | 註釋是否有錯誤或者可能導致誤解? | 無 | |
重要 | 類結構的public, protected, private順序是否在所有的程序中保持一致? | 無 | |
命名規則 | |||
重要性 | 審查項 | 結論 | |
重要 | 命名規則是否與所采用的操作系統或開發工具的風格保持一致? | 是 | |
標識符是否直觀且可以拼讀? | 否 | ||
標識符的長度應當符合“min-length && max-information”原則? | 是 | ||
重要 | 程序中是否出現相同的局部變量和全部變量? | 否 | |
類名、函數名、變量和參數、常量的書寫格式是否遵循一定的規則? | 是 | ||
靜態變量、全局變量、類的成員變量是否加前綴? | 否 | ||
表達式與基本語句 | |||
重要性 | 審查項 | 結論 | |
重要 | 如果代碼行中的運算符比較多,是否已經用括號清楚地確定表達式的操作順序? | 是 | |
是否編寫太復雜或者多用途的復合表達式? | 否 | ||
重要 | 是否將復合表達式與“真正的數學表達式”混淆? | 否 | |
重要 | 是否用隱含錯誤的方式寫if語句? 例如 | ||
(1)將布爾變量直接與TRUE、FALSE或者1、0進行比較。 | 否 | ||
(2)將浮點變量用“==”或“!=”與任何數字比較。 | 否 | ||
(3)將指針變量用“==”或“!=”與NULL比較。 | 否 | ||
如果循環體內存在邏輯判斷,並且循環次數很大,是否已經將邏輯判斷移到循環體的外面? | 否 | ||
重要 | Case語句的結尾是否忘了加break? | 否 | |
重要 | 是否忘記寫switch的default分支? | 否 | |
重要 | 使用goto 語句時是否留下隱患? 例如跳過了某些對象的構造、變量的初始化、重要的計算等。 | 無 | |
常量 | |||
重要性 | 審查項 | 結論 | |
是否使用含義直觀的常量來表示那些將在程序中多次出現的數字或字符串? | 否 | ||
在C++ 程序中,是否用const常量取代宏常量? | 否 | ||
重要 | 如果某一常量與其它常量密切相關,是否在定義中包含了這種關系? | 無 | |
是否誤解了類中的const數據成員?因為const數據成員只在某個對象 | 無 | ||
生存期內是常量,而對於整個類而言卻是可變的。 | 否 | ||
函數設計 | |||
重要性 | 審查項 | 結論 | |
參數的書寫是否完整?不要貪圖省事只寫參數的類型而省略參數名字。 | 是 | ||
參數命名、順序是否合理? | 是 | ||
參數的個數是否太多? | 否 | ||
是否使用類型和數目不確定的參數? | 否 | ||
是否省略了函數返回值的類型? | 否 | ||
函數名字與返回值類型在語義上是否沖突? | 否 | ||
重要 | 是否將正常值和錯誤標誌混在一起返回?正常值應當用輸出參數獲得,而錯誤標誌用return語句返回。 | 否 | |
重要 | 在函數體的“入口處”,是否用assert對參數的有效性進行檢查? | 無 | |
重要 | 使用濫用了assert? 例如混淆非法情況與錯誤情況,後者是必然存在的並且是一定要作出處理的。 | 無 | |
重要 | return語句是否返回指向“棧內存”的“指針”或者“引用”? | 無 | |
是否使用const提高函數的健壯性?const可以強制保護函數的參數、返回值,甚至函數的定義體。“Use const whenever you need” | 無 | ||
內存管理 | |||
重要性 | 審查項 | 結論 | |
重要 | 用malloc或new申請內存之後,是否立即檢查指針值是否為NULL?(防止使用指針值為NULL的內存) | 否 | |
重要 | 是否忘記為數組和動態內存賦初值?(防止將未被初始化的內存作為右值使用) | 否 | |
重要 | 數組或指針的下標是否越界? | 否 | |
重要 | 動態內存的申請與釋放是否配對?(防止內存泄漏) | 否 | |
重要 | 是否有效地處理了“內存耗盡”問題? | 否 | |
重要 | 是否修改“指向常量的指針”的內容? | 無 | |
重要 | 是否出現野指針?例如(1)指針變量沒有被初始化;(2)用free或delete釋放了內存之後,忘記將指針設置為NULL。 | 否 | |
重要 | 是否將malloc/free 和 new/delete 混淆使用? | 沒使用 | |
重要 | malloc語句是否正確無誤?例如字節數是否正確?類型轉換是否正 確? | 是 | |
重要 | 在創建與釋放動態對象數組時,new/delete的語句是否正確無誤? | 無 | |
C++ 函數的高級特性 | |||
重要性 | 審查項 | 結論 | |
重載函數是否有二義性? | 無 | ||
重要 | 是否混淆了成員函數的重載、覆蓋與隱藏? | 無 | |
運算符的重載是否符合制定的編程規範? | 無 | ||
是否濫用內聯函數?例如函數體內的代碼比較長,函數體內出現循環。 | 無 | ||
重要 | 是否用內聯函數取代了宏代碼? | 無 | |
類的構造函數、析構函數和賦值函數 | |||
重要性 | 審查項 | 結論 | |
重要 | 是否違背編程規範而讓C++ 編譯器自動為類產生四個缺省的函數: | ||
(1)缺省的無參數構造函數; | 無 | ||
(2)缺省的拷貝構造函數; | 無 | ||
(3)缺省的析構函數; | 無 | ||
(4)缺省的賦值函數。 | 無 | ||
重要 | 構造函數中是否遺漏了某些初始化工作? | 無 | |
重要 | 是否正確地使用構造函數的初始化表? | 無 | |
重要 | 析構函數中是否遺漏了某些清除工作? | 無 | |
是否錯寫、錯用了拷貝構造函數和賦值函數? | 無 | ||
重要 | 賦值函數一般分四個步驟: | ||
(1)檢查自賦值; | 是 | ||
(2)釋放原有內存資源; | 是 | ||
(3)分配新的內存資源,並復制內容; | 否 | ||
(4)返回 *this。是否遺漏了重要步驟? | 是 | ||
重要 | 是否正確地編寫了派生類的構造函數、析構函數、賦值函數? | 無 | |
註意事項: | |||
(1)派生類不可能繼承基類的構造函數、析構函數、賦值函數。 | 無 | ||
(2)派生類的構造函數應在其初始化表裏調用基類的構造函數。 | 無 | ||
(3)基類與派生類的析構函數應該為虛(即加virtual關鍵字)。 | 無 | ||
(4)在編寫派生類的賦值函數時,註意不要忘記對基類的數據成員重新賦值 | 無 | ||
類的高級特性 | |||
重要性 | 審查項 | 結論 | |
重要 | 是否違背了繼承和組合的規則? | ||
(1)若在邏輯上B是A的“一種”,並且A的所有功能和屬性對B而言都有意義,則允許B繼承A的功能和屬性。 | 無 | ||
(2)若在邏輯上A是B的“一部分”(a part of),則不允許B從A派生,而是要用A和其它東西組合出B。 | 無 | ||
其它常見問題 | |||
重要性 | 審查項 | 結論 | |
重要 | 數據類型問題: | ||
(1)變量的數據類型有錯誤嗎? | 否 | ||
(2)存在不同數據類型的賦值嗎? | 否 | ||
(3)存在不同數據類型的比較嗎? | 否 | ||
重要 | 變量值問題: | ||
(1)變量的初始化或缺省值有錯誤嗎? | 否 | ||
(2)變量發生上溢或下溢嗎? | 否 | ||
(3)變量的精度夠嗎? | 夠 | ||
重要 | 邏輯判斷問題: | ||
(1)由於精度原因導致比較無效嗎? | 否 | ||
(2)表達式中的優先級有誤嗎? | 否 | ||
(3)邏輯判斷結果顛倒嗎? | 否 | ||
重要 | 循環問題: | ||
(1)循環終止條件不正確嗎? | 否 | ||
(2)無法正常終止(死循環)嗎? | 否 | ||
(3)錯誤地修改循環變量嗎? | 否 | ||
(4)存在誤差累積嗎? | 否 | ||
重要 | 錯誤處理問題: | ||
(1)忘記進行錯誤處理嗎? | 是 | ||
(2)錯誤處理程序塊一直沒有機會被運行? | 是 | ||
(3)錯誤處理程序塊本身就有毛病嗎?如報告的錯誤與實際錯誤不一致,處理方式不正確等等。 | 否 | ||
(4)錯誤處理程序塊是“馬後炮”嗎?如在被它被調用之前軟件已經出錯。 | 否 | ||
重要 | 文件I/O問題: | ||
(1)對不存在的或者錯誤的文件進行操作嗎? | 無 | ||
(2)文件以不正確的方式打開嗎? | 無 | ||
(3)文件結束判斷不正確嗎? | 無 | ||
(4)沒有正確地關閉文件嗎? | 無 | ||
隊友代碼及優缺點評價
這次我就先來評價一下雷鎵同學的代碼吧,她的這次代碼是她數據結構課程設計的代碼,有關於最短路徑的求解。其實我們倆的課程設計題目還是挺像的,我是用普裏姆算法來求出以不同節點為起點構成的最小生成樹,而她是用弗洛伊德算法求出連接兩個城市的最短行走路線。
看她的代碼之前就需要首先了解弗洛伊德算法,感覺又重新回到了大一大二學數據結構與算法的那個時期了,真的是好久都沒有學過算法了,自我感嘆自己根本不像是一個計算機專業的學生了。
其實雷同學的這個代碼使用c語言寫的,所以參照代碼審查表中的一些c++的高級特性就沒有什麽意義了。她的這個小項目優點還是有很多的,比如算法寫的比較完善,部分代碼也還算比較整潔啊之類的,但是我認為還是有一些不可取的地方,在這裏我就毫不客氣地指出來啦。
1.從排版方面來看,
例如下面列舉的她代碼中的一個弗洛伊德函數,這塊代碼首先從排版上,我就看的比較難受,可能並不是只有我一個人強迫癥吧,大括號{}在有的循環中出現了,在有的循環中沒有出現。我個人的習慣是在一個函數中如果循環體中無論是有幾條語句,哪怕只有一句也要加上大括號的這種強迫星人。
說實話為了看懂這段代碼我確實真的是把它重新整理了一遍。
void Floyd(MGraph *L,int n)
{
int i,j,k;
for(i=1;i<=n;i++)
for(j=1;j<=n;j++){
if( L->arcs[i][j]!=Maxint)
p[i][j]=j;
else
p[i][j]=0;
D[i][j]=L->arcs[i][j];
}
for(k=1;k<=n;k++){
for(i=1;i<=n;i++)
for(j=1;j<=n;j++){
if(D[i][k]+D[k][j]<D[i][j]) {
D[i][j]=D[i][k]+D[k][j];
p[i][j]=p[i][k];
}
}
}
}
或許,改成這樣是不是能看的方便一點呢,當然,這只是我的個人見解。
void Floyd(MGraph *L,int n)
{
int i,j,k;
for(i=1;i<=n;i++)
{
for(j=1;j<=n;j++)
{
if( L->arcs[i][j]!=Maxint)
p[i][j]=j;
else
p[i][j]=0;
}
D[i][j]=L->arcs[i][j];
}
for(k=1;k<=n;k++)
{
for(i=1;i<=n;i++)
{
for(j=1;j<=n;j++)
{
if(D[i][k]+D[k][j]<D[i][j])
{
D[i][j]=D[i][k]+D[k][j];
p[i][j]=p[i][k];
}
}
}
}
}
2.關於變量的定義
雷同學的部分代碼變量定義的太過於簡單,簡單到我一眼看不出這是哪個變量...恩,其實我也是花費了一些時間來找。
比如說下面這兩句
MGraph *L;
int n,e,v,w,k;
不誇張地說,結合了後面的代碼我也是看了好久才懂,我覺得我以前也是這樣的,從大一開始學習c語言開始就一直用一些a、b、i、j、k、n之類的名字來定義變量。這其中有很多原因,其一是因為這些變量名字短,寫起來方便,其二是自己寫的程序都很短很簡潔,哪怕定義10個20個這種變量自己也還是能記住每個字母都對應著什麽意思,但是這就給別的閱讀代碼的人造成了困擾,如果是別人第一次閱讀你的代碼的話,我覺得他可能會崩潰。
我也是從大二開始寫代碼才想要用一些英文單詞來命名變量,有些不會的就上網查英文意思,選一個比較符合函數意義的單詞來命名。
在這裏她定義的n為頂點個數,e為邊數,v為起點,w為終點。如果我修改的話,可能會改為:
int vertex ,edges ,start_point ,end_point ;
3.關於malloc
代碼中使用了malloc來動態申請內存,以前我們編程的時候也沒有註意到這樣幾個問題。
內存真的申請成功了嗎?如果所申請的內存塊大於目前堆上剩余內存塊,則內存分配會失敗,函數返回NULL。那我們在使用指向這塊內存的指針時,必須用if(NULL!=p)語句來驗證內存確實分配成功了。
申請成功之後,既然有分配,那就必須有釋放。不然的話,有限的內存總會用光,而沒有釋放的內存卻在空閑。與malloc對應的就是free函數了。free函數只有一個參數,就是所要釋放的內存塊的首地址。在這裏雷鎵同學並沒有寫相應的釋放代碼。
而既然使用free函數之後,指針變量本身保存的地址並沒有改變,那我們就需要重新把p的值變為NULL。如果沒有把指針置NULL,這個指針就成為了“野指針”,這是很危險的,而且也是經常出錯的地方。所以free完之後,也一定要給指針置NULL。
這其實是一系列的內容,每次使用動態申請內存的時候都要考慮一些這些問題,這些也同樣是我以前沒有考慮到的。
4.有關註釋
雷鎵同學的代碼沒有註釋,幸好她的代碼不長,一百行多一點,而且函數只有三個,還都是無返回值的void函數,能從她的菜單中看出她的意圖,還是挺好的,但是希望以後稍微寫一些註釋,有助於別人閱讀她的代碼。
個人感想
??首次結對編程體驗雖然只是一個互相復審代碼的過程,但還是讓我有了很深刻的體會。結對編程?構建之法一書中說到結對編程是極限編程的一種,是比單人編程有很多可取之處的,其中一些優點就是兩個人能夠互相審查、共同進步。而我們寫代碼不能只自己看,自己偷偷藏著掖著自己隨便寫,以後也絕對是不可能的事情,一己之力做不成什麽大項目。為了讓別人能夠容易閱讀你的代碼,首先代碼所需要的就是簡明,易讀。代碼復審的過程中才知道原來小夥伴的代碼有很多優點,同時也暴露了很多缺點,而自己反觀自己的代碼,同樣慘不忍睹,年輕啊,以為寫代碼是多麽容易的事情,動動腦子,仔細研究研究算法就行了,事實根本不是這樣,如果真正落實到實際項目之中,那要考慮的太多了。
??還想了想,以後如果在公司實習或者工作,遇到的同事和夥伴看不懂你的代碼,那會是多麽尷尬的一件事情,這首先就證明了你寫的並不是很盡如人意,不是那麽通俗易懂。缺點在代碼復審中暴露出來了,下次真正做項目的時候要好好註意了。
結對編程初涉獵——結對夥伴的代碼復審