說透代碼評審

代碼如熵，不加外力很容易就會隨着代碼的不斷堆積而發生腐爛和潰敗。咱們不是不知道代碼問題，而是對既成事實難有改變。可是若是站在迭代的角度思考下一次升級，如何確保新的代碼質量就顯得頗有必要，特別是你的代碼須要重寫的時候，我想你必定會遇到和我同樣的問題，咱們究竟應該如何保證咱們的代碼的質量？因而就有了這篇工具型的文章。

​ 如下內容架構是我摘錄以爲比較重要的緯度，不少方法論借鑑了以上幾篇文章思想，對代碼評審的各類狀況基本都談得比較到位了，反覆精讀基本上就能夠採用拿來主義，但願對你的團隊評審有所幫助。

爲何要評審

不審查的壞處

　　代碼的質量反應了咱們的產品質量，產品不穩定、總是出現BUG，直接影響客戶滿意度和口碑。

　　同時，代碼的好壞決定了將來運維的成本，若是由於一時疏忽和妥協，回頭又沒有及時修改，中間又出現人員變更，那麼這份代碼的後患是無窮的。

　　由於不規範，可讀性差，對交接人來講從心態上是本能反抗的，可是又不得不改，因而就一通亂改，能貼膏藥就貼膏藥，能運行就能夠，管他規範不規範。這樣致使的後果是，代碼從不規範走向更加不規範，很難想象通過5-10年鍥而不捨的不規範，這個產品還能活着。

　　技術債務的危害怎麼形容都不爲過，輕則系統局部異常，中等的會致使修改困難，嚴重的須要推翻重來。

　　從物理學上看，熵讓咱們理解了一件事，若是不施加外力影響，事物永遠向着更混亂的狀態發展，因此規範和審查就顯得彌足珍貴了。

　　從軟件設計看，軟件設計要關注長期變化，須要應對需求規模的膨脹。若是腐爛的代碼日積月累，這些在不斷流變腐爛的東西又怎能支撐起長期的變化呢！

　　作產品，不審查不足以長久。

評審的好處

　　在軟件工程師平常的開發工做中，若是要挑出一項極其重要，卻又很容易被忽視的工做，在我看來代碼審查幾乎是無可爭議的第一。代碼審查是一個低投入、高產出的開發活動，就我的而言，從其中學到的習慣、方法和知識，獲益匪淺。

　　代碼評審的做用不少，主要表如今 6 個方面。

　　好處 1，儘早發現 Bug 和設計中存在的問題。咱們都知道，問題發現得越晚，修復的代價越大。代碼審查把問題的發現儘可能提早，天然會提升效能。

　　好處 2，提升我的工程能力。不言而喻，別人對你的代碼提建議，天然能提升你的工程能力。事實上，僅僅由於知道本身的代碼會被同事審查，你就會注意提升代碼質量。

　　好處 3，團隊知識共享。一段代碼入庫以後，就從我的的代碼變成了團隊的代碼。代碼審查能夠幫助其餘開發者瞭解這些代碼的設計思想、實現方式等。另外，代碼審查中的討論記錄還能夠做爲參考文檔，幫助他人理解代碼、查找問題。

　　好處 4，針對某個特定方面提升質量。一些比較專業的領域，好比安全、性能、UI 等，能夠邀請專家進行專項審查。另外，一些核心代碼或者高風險代碼，也能夠經過團隊集體審查的方式來保證質量。

　　好處 5，統一編碼風格。這，也是代碼審查的一個常見功能，但最好能經過工具來實現自動化檢查。

　　好處 6，社會性功用。若是你在編程，並且知道必定會有同事將檢查你的代碼，那麼你編程的姿式和心態就會徹底不一樣。這之間的微妙差別正是在於會不會有人將對你的代碼作出反饋與評價。

評審的困境和爭議

　　大多數的爭議，都不是在否定代碼審查的好處，而是聚焦在不進行代碼審查的那些「緣由」 或者 「藉口」上。

1）代碼評審費時費力：

　　項目期限 Deadline 已定，時間緊迫，每天加班忙成狗了，誰還願意搞代碼評審？這是一個最多見的客觀阻礙因素，由於 Deadline 不少時候都不是咱們本身能肯定和改變的。

改來改去無非是一些格式、註釋、命名之類不痛不癢的問題。

2）代碼審查不利於團建：

　　由於常常有程序員由於觀點不一樣在代碼審查的時候吵起來。

3）主觀意願

　　評審人用本身主觀意見看待別人的代碼。以爲別人的代碼風格不夠好，或者把我的的喜愛強加到別人身上。

4）團隊人員結構搭配不合理。

　　新人沒經驗的多，有經驗的少。新人交叉評審可能效果很差，總是安排經驗多的少數人幫助 Review 多數新人的代碼，新人或有收穫，但對高級或資深程序員又有多大裨益？

　　一個好的規則或制度，老是須要既符合多方參與者的個體利益又能知足組織或團隊的共同利益，這樣的規則或制度才能更順暢、有效地實施和運轉。

5）有人就是不喜歡別人 Review 他的代碼，他會感受是在找茬。

　　好比，團隊中存在一些自信超強大的程序員，以爲本身寫的代碼絕對沒問題，不須要別人來給他 Review。

6）效果很是差、形式主義

　　代碼評審作得很差確實像形式主義，純粹走個過場。

搞清楚評審形式

　　落地以前，咱們先搞清楚評審形式是什麼樣子的。

　　通常來講人工檢查才叫審查，機器進行的檢查通常叫做代碼檢查或者代碼自動檢查。常見的辦法是人工評審和機器檢查同時進行。

　　代碼評審形式能夠分爲：

• 機器檢查

• 人工評審

• • 純線上評審
  • 線下投屏評審(推薦)

　　這裏推薦作法是線下投屏評審，或許傳統、保守，可是利大於弊。根據個人經驗，線上問題處理和後續工做計劃調整，所產生的一次性成本遠遠大於一次性線下評審。

評審對象有哪些

　　咱們應該約定如下幾種操做都要進行評審，通常變更比較大的須要多人評審，通常性修改只須要其餘同事二次檢查便可。

　　

評審參與人員

　　在這個問題上，原則是業務密切相關的人員都要參加，好比說：

1. 開發人員的導師
2. 負責這個業務流轉鏈路的下一個環節的同事

　　你們參與進來可以及時同步信息，避免信息的不對稱，也就是說能夠避免其餘環節須要人員兼容改動的，但是實際上沒有人考慮到，而致使漏改問題。

　　問題：爲何說要堅持密切相關的人員都參加這個原則？

　　答案：大部分人對於本身不感興趣或者聽不懂的事務時，很是容易走神。因此組織業務密切相關的一線開發，組長，及領導參與評審就足夠了。

評審應該關注哪些

　　最後還有一個須要咱們搞懂的問題，弄清楚以後，咱們才能很好的把代碼評審落地。

• 代碼評審容易犯的錯誤：評審人用本身主觀意見看待別人的代碼。

　　以爲別人的代碼風格不夠好，或者把我的的喜愛強加到別人身上，有這種想法當然好，若是以爲在團隊內應該統一使用這種風格，最好申請把他歸入到團隊的代碼規範文檔中，以免代碼評審的重點產生偏移。

• 代碼評審的目的：最大化維護團隊的利益。

　　爲了軟件的良好發展和團隊的高效協做，每一個人的代碼最好看上去都差很少，這樣修改別人的代碼就比較親切。

• 代碼評審應該關注的重點：

• • 是否明顯的邏輯錯誤
  • 是否落實了代碼規範
  • 代碼的可讀性和可維護性是否良好
  • 代碼是否有違背基本的設計模式理念

• 代碼評審不該該過多關注：

• • 代碼是否能正常運行
  • 代碼是否知足業務需求
  • 是否覆蓋業務場景

　　這些應該由編寫代碼的人和測試人員共同保證

評審的流程有哪些

　　每一個想要高效工做的程序員，都不但願本身的計劃被頻繁打斷，大多數狀況下循序漸進，跟着規劃走纔是最穩妥的。

　　下面是評審中的六個流程：

　　

• 約定規範文檔

　　爲了不審查當中的主觀喜愛，制定有章可循的約定是前提條件。

• 制定排期：

　　咱們能夠約定每週一發起代碼評審，由提交人根據邏輯變更狀況，給出一個評審大概須要花費的時間，同時結合需求的提測時間，上線時間，肯定好何時進行評審，避免過多雜亂的時間線。

• 補充資料

　　在評審工期申請後，還須要補充資料，交代清楚需求背景、編碼內容以及功能影響的範圍，評審人就能夠根據這些資料，評估應該重點關注哪些代碼隱患。

好比需求涉及到金錢往來，那麼評審人就應該更加仔細留意每一行代碼，檢查是否有明顯的邏輯錯誤，避免上線後致使用戶或者公司的利益受到損害。

• 代碼評審

　　代碼評審中發現的問題，根據嚴重性決定是否進行二次評審。

• 完善必備要素

　　發現問題並不可怕，可怕的是相同的問題不一樣的同事重複犯，因此必須在評審後完善代碼評審必須具有的元素清單，好比監控、註釋等。若是不知足任何一個，直接駁回，畢竟重複說起相同的問題，會你們參與者的積極性。

• 完善代碼規範約定

　　另外也須要按期回顧以前的代碼評審，完善團隊代碼規範約束，讓軟件質量和團隊能力都想着更好的方向走。

審查步驟和方法

　　從個人經驗來看，要成功引入代碼審查，首先要在團隊內達成一些重要的共識，而後選擇試點團隊實行，最後選擇合適的工具和流程。

　　我的以爲不是全部的團隊都適合作代碼審查，必定要慎用。特別是那種項目類型的團隊，救火類型的團隊，團隊規模很小，業務優先的團隊。

一、代碼審查應該計入工做量

　　代碼審查若是沒有爲它預留時間，結果是你們沒有時間作審查，效果天然也就很差。因而，造成惡性循環，代碼審查要麼被逐漸廢棄，要麼流於形式。

　　預估工做量的時候須要考慮代碼審查的時間。好比，平均天天大約預留 30-60分鐘用於代碼審查，大概佔寫代碼總時間的 1/5。

　　通常的代碼審查速度約是一小時 150 行，對於一些關鍵的軟件，一小時數百行代碼的審查速度太快，可能沒法找到程序中的問題。

　　同時，代碼審查的狀況會做爲績效考評的一個重要指標。

　　另外，平時須要給審查者關於審查質量的實時反饋。好比，剛加入公司的時候，對代碼審查不夠重視，作得不夠好。主管應該對給反饋意見，讓新人提升審查質量。

　　其次，讓新人多給同事作審查，另外，是讓新人多給一些結構上的建議，不用過重視語法細節。

　　總之，管理者要明確代碼審查是開發工做的重要組成部分，並記入工做量和績效考評。這，是成功引入代碼審查的前提，再怎麼強調都不爲過。

二、選擇試點團隊

　　爲何要選擇試點團隊，說白了就是要避免紙上談兵的不足，在小範圍內作實驗能夠有效下降風險，儘量得收集負面效果，並及時改進。不然大規模出現負面效果是很影響你們信心，甚至出現反面效果。

三、選擇工具，融合機器審查和人工審查

　　關於工具，若是你的團隊原本已經在使用 GitLab、GitHub、Gerrit、Phabricator 管理代碼的話，那麼很容易上手代碼審查。由於，GitHub、GitLab 有基於 PR 的審查。而 Gerrit 和 Phabricator 自己就主打代碼審查的功能。

　　第一，將代碼提交到本地 Git 倉庫或者用於審查的遠端 Git 服務器的分支上；第二，把 commit 提交給代碼審查工具；第三，代碼審查工具開始進行機器審查和人工審查；第四，若是審查通不過就打回重作，開發者修改後從新提交審查，直到審查經過後代碼入庫。

　　

　　關於工具集的適用：使用 GitLab、Jenkins 和 SonarQube 進行配置。具體使用 GitLab 管理代碼，代碼入庫後經過鉤子觸發 Jenkins，Jenkins 從 GitLab 獲取代碼，運行構建，並經過 Sonar 分析結果。這裏有一篇不錯的文章供你參考

評審的關鍵操做

• 提升提交的原子性

　　每次提交的代碼粒度相當重要，咱們能夠反過來思考：

• • 若是提交的是半個功能的代碼會怎麼樣？
  • 若是提交的是一週的代碼會怎麼樣？

因此原子性就是合適的粒度，大功能要拆分來提交，一週的代碼的代碼要按天來提交。不然對於評審人員來講是很反感的，由於只會增長審查的難度。

• 提升提交說明的質量

　　咱們應該常常見到不少的提交是這樣描述的：

• • BUG
  • FIX
  • 更新

　　下面是葛俊給出的三個改進步驟：

　　第一步，規定提交說明必定要包括標題、描述和測試狀況三部分，但暫時還不具體要求必須寫多少字。好比，測試部分能夠簡單寫一句「沒有作測試」，但必定要寫。若是格式不符合要求，審查者就直接打回。這個格式要求工做量很小，比較容易作到，兩個星期後整個團隊就習慣了。雖然只是在提交說明裏增長了簡單描述，也已經爲審查者和後續工做中進行問題排查提供一些必要信息，因此你們也比較承認這個操做。

　　第二步，要求提交說明必須詳細寫明測試狀況。若是沒有作測試必定要寫出具體理由，不然會被直接打回。這樣作，不但爲審查者提供了方便，還促進了開發人員的自測。整個團隊在一個多月後，也養成了詳細描述測試狀況的習慣。

　　第三步，逐步要求提交的原子性。我要求每個提交要在詳細描述部分描述具體實現了哪些功能。若是一個提交同時實現了多個功能，那就必須解釋爲何不能拆開提交；若是解釋不合理的話，提交直接被打回。

　　提交說明是提升代碼審查的利器，好的格式應該包含如下幾個方面：

　　標題，簡明扼要地描述這個提交。這部分最好在 70 個字符以內，以確保在單行顯示的時候可以顯示完整。好比，在命令行經常使用的 git log --oneline 輸出結果要能顯示徹底。

　　詳細描述，包括提交的目的、選擇這個方法的緣由，以及實現細節的總結性描述。這三個方面的內容最能幫助審查者閱讀代碼。

　　測試狀況，描述的是你對這個提交作了什麼樣的測試驗證，具體包括正常狀況的輸出、錯誤狀況的輸出，以及性能、安全等方面的專項測試結果。這部份內容，能夠增長審查者對提交代碼的瞭解程度以及信心。

　　與其餘工具和系統相關的信息，好比相關任務 ID、相關的衝刺（sprint，也可翻譯爲「迭代」）連接。這些信息對工具的網狀互聯提供基礎信息，很是重要。這裏還有一個 Git 的技巧是，你可使用 Git 的提交說明模板（Commit Message Template），來幫助團隊使用統一的格式。

評審的關鍵原則

　　雖然說評審須要規範文檔作指導，可是不少規範其實沒法作的那麼細，特別是規範早期，不少規範是缺失的。在評審過程當中不免會出現爭論或者相持不下的狀況。這個時候有必要把評審的原則提早作一個說明，能夠消除將來沒必要要的麻煩。

• 相互尊重

　　從編碼做者的角度來看，審查者要花費時間去閱讀他並不熟悉的代碼，來幫助你提升，應該儘可能爲審查者提供方便。

• 好比，提升提交說明的質量，就是對審查者最基本的尊重。
• 還有，若是你的代碼都沒有進行自測就提交審查，你以爲審查者內心會怎麼想呢？
• 又如，若是你提交的一個審查有一萬行代碼，讓審查者怎麼看呢？

　　因此，代碼做者必定要提審查者着想，幫助審查者可以比較輕鬆地完成審查。

　　從審查者的角度來看，在提出建議的時候，必定要考慮代碼做者的感覺。最重要的一點是，不要用一些主觀標準來評判別人的代碼。

• 基於討論

　　代碼審查的目的是討論，而不是評判，這個原則相當重要。

　　討論的心態，有助於放下沒必要要的自尊心，從而順利地進行技術交流，提升審查效率。

　　另外，討論的心態也能促進你們提前發出審查，從而儘早發現結構設計方面的問題。

　　另外，我還有一些關於討論的建議：審查者切記不要說教，說教容易讓人反感，不是討論的好方法。

　　審查者提意見便可，不必定要提供解決方法。給定答案也可能會引發沒必要要的反感。

代碼評審的常見問題

　　這些是代碼評審過程當中發現的共同的問題，能夠一塊兒放在代碼規範文檔中。

缺乏註釋和變動說明

　　好比下面三個方法名稱，參數，返回值類似，爲何會出現這種狀況？

　　大半是開發者發現原來的代碼沒有註釋，因此不敢修改，因而拷貝一份後只是稍微修改了一下，這樣纔出現了類似冗餘的代碼。重複代碼最可怕的地方就是錯該或者漏改。

publi Article GetArticleById(long id)
{
  return null;
}
publi MaterialPO GetMaterialById(long id)
{
  return null;
}
publi ArticleVO GetArticleByIdWithoutStatus(long id)
{
  return null;
}

過渡相信第三方

• 對請求參數沒有限制
• 對請求參數沒有校驗

//錯誤示範
Boolean SaveError(List<ShareDetail> list)
{
   //錯誤緣由：只判斷非空，可是沒法保證list中的detail是非空的
   if(list.IsEmpty())
   {
      return false;
   } 
   //錯誤緣由：沒法保證list中的id都是同樣的
   Int id = list.get(0).getId();
   //保存數據
   return Save(id,list)
}
 
 
//正確示範
Boolean SaveRight(Int id,List<ShareDetail> list)
{
   //深度判斷非空狀況
   if(id==null || CollectionUtil.hasNull(list))
   {
      return false;
   } 
   //id從參數中獲取，避免二義性
   return Save(id,list)
}

變量做用域過大

ublic static void main(Sstring[] args){
  ShareDetail shareDetail1 =new ShareDetail();
  //錯誤示例：對象在多個方法間進行值地址引用傳遞
  varErrorStep1(shareDetail1); 
  //正常示例
  ShareDetail shareDetail2=varRight();
}
 
 
public static varErrorStep1(ShareDetail shareDetail)
{
   //其餘複雜操做
  shareDetail.setId(111);
  var ErrorStep2(shareDetail);
}
 
 
public static void varErrorStep2(ShareDetail shareDetail)
{
  //其餘複製操做
  shareDetail.setName("hello");
}
 
 
public static ShareDetail varRight()
{
  ShareDetail shareDetail =new ShareDetail();
  shareDetail.setId(111);
  shareDetail.setName("hello");

缺乏階段性結果

　　對於過多的if-else判斷，優化方法：先進性異常判斷，若是有異常就快速返回結果。

//錯誤示範
public void SetpError()
{
  //標記位
  Boolean flag=false;
  List<Integer> list=new ArrayList<>();
  for(Integer detail:list)
  {
    //若是列表中有值大於10則將標記設置爲true
    if(detail>0)
      {
        flag=true;
      }
    }
  }
  //
  if(flag)
  {
    return；
  }
}
//正確示範
public void SetpRight()
{
  //步驟一：校驗參數
  List<Integer> list=new ArrayList<>();
  for(Integer detail:list)
  {
    //若是列表中有值大於10則將標記設置爲true
    if(detail>0)
      {
        return; //移除多餘的標記，直接中斷；
      }
    }
  }
  //步驟二：
   
  //步驟三：
 
 
}

　　同時爲了增長代碼的層次感，可讓代碼階段性的輸出結果，好比編寫時註釋好每個步驟要作什麼。

日誌打印問題

//錯誤示範
public Boolean logError(Integer id,List<ShareDetail> list)
{
  if(id==null || CollectionUtil.hasNull(list))
  {
      return false;
  }
  logger.info("logError run");
   
  try{
    return shareDao.save(id,list);
  }catch(Exception e)
  {
    //錯誤一：不記錄異常上下文
   logger.error("save error");
   //錯誤二：只記錄了有限的上下文
   logger.error("save error e:{0}",e.getMessage());
  }
  return false;
}
 
 
//正確示範
public Boolean logError(Integer id,List<ShareDetail> list)
{
  if(id==null || CollectionUtil.hasNull(list))
  {
      return false;
  }
  //info級別在非線上環境很容易
  logger.info("logError run");
   
  try{
    return shareDao.save(id,list);
  }catch(Exception e)
  {
    //正確：充分記錄錯誤上下文
    logger.error("save error");
    //正確：規避記錄日誌時出現控制着，規避內存OOM
    logger.error("save error id:{},e:{},list:{},id,list==null?list:JSONObject.toJSON(list),e);
  }
  return false;
}

總結

　　想讓團隊從內心接受代碼評審的理念，承認它是平常開發過程必不可少的步驟，那麼就要提升代碼評審的質量，不然評審的時候，你們都在搞私事，容易流於形式，而要提升代碼評審的質量，咱們必須明確代碼評審的形式，對象，參與者，關注點，流程，常見問題。

　　我但願你們能記住一件事，代碼評審要從團隊利益出發，讓每一個人都可以從裏面持續獲得正反饋，這纔是最重要的。若是時間充足的話最好邀請測試人員一塊兒參與，讓測試人員充分了解代碼改動點，有助於對測試場景邊界的把控。

> 來源：https://www.cnblogs.com/jackyfei/p/13299877.html

歡迎關注公衆號 【碼農開花】一塊兒學習成長 我會一直分享Java乾貨，也會分享免費的學習資料課程和麪試寶典 回覆：【計算機】【設計模式】【面試】有驚喜哦