Categories
程式開發

Code Review怎麼做


Code Review現在基本成為了一種共識,再忙的團隊,也會抽出時間來做Code Review。至於怎麼做Code Review,每個團隊的情況都可能不太一樣。甚至每個人的習慣都不太一樣。不得不承認的是,Code Review做得好不好和Reviewer個人的水平有很大的關係。一個好的Reviewer,反饋意見又快又好,而且反饋的意見一針見血,直至代碼的最核心問題。

看到一個MR之後,我腦袋裡最開始出現的幾個問題是:

代碼是否足夠簡單,是否容易閱讀,是否容易理解代碼有沒有陌生的或者不符合常人思維的概念代碼的排版是否符合編程規範,風格是否和現有代碼一致代碼是否容易測試,是否有相應的測試代碼代碼是否有明顯的重複,是否拷貝了其他模塊的代碼

上面5個問題有了答案之後,基本上可以對這份代碼有個初步的判斷了。如果都沒有大的問題,說明這份代碼是經過深思熟慮的,是用心打磨過的,不是隨意提交的代碼。那麼,我們可以把更多的精力放到更深層次的問題。深層次的問題,分為宏觀和微觀兩個方面入手。

宏觀上,我們主要關注以下幾個方面:

新增的代碼和已有的架構設計是否一致。做一個新的特性,不僅僅要看這個特性本身是否能夠基於現有的架構裡實現,還要看是否突破了原有設計的邊界和破壞了原有的設計原則。破壞遠比建設來得容易,架構能否適應業務的發展,也是需要持續演進,但是我們要知道演進的方向和原則,控制系統的邊界。上下文的切分是否合理。特性的粒度或大或小,上下文的切分都同等重要。切分上下文既是為了隔離變化,也是為了保留可拓展性。不合理的上下文切分,往往要在業務發生變化的時候要付出數倍的代價。有的時候,為了重新劃分上下文,與其重構,不如重寫。依賴是否正交。切分上下文是從模塊劃分的角度分離業務,我們還是需要分層的視角來進一步隔離變化。對於復雜的業務,分層的思想很重要。一個複雜的問題,可以通過分層來分解成幾個更容易解決的問題,同時問題也變得更容易理解。方案的理解成本更低,才可能寫出更容易理解的代碼。

如果一個MR,宏觀上沒有太大的問題,那麼我們就可以集中精力好好審視和推敲微觀的問題了。微觀的問題一般比較瑣碎,而且都是相對比較獨立的問題,所有經驗很重要。經驗豐富的reviewer會有更出色的嗅覺,更靈敏地感覺到代碼的壞味道。我一般是從以下幾類常見的問題入手:

語言最佳實踐。雖然現在的IDEA很強大了,但是還是會有不少最佳實踐是無法通過IDEA自動識別出來的。比如Java裡面的函數式的寫法,try-with-resource,單例模式等。算法的時間或者空間複雜度。大多數不是專門研究算法的工程師,對算法複雜度的敏感度是比較低的,大多數時候會忽略算法的複雜度。設計模式的使用。對於一些有定式的問題,使用設計模式會大幅降低理解和溝通成本。如果不使用設計模式,理論上也是可以實現的。但是設計模式的好處在於,把更多的信息用代碼表達出來,而且是形式化的代碼。這些信息比文字、語言還要高效。安全問題。安全問題平時關注度不高,一旦出現,代價高昂。比如常見的安全問題有:明文密碼、SQL注入、shell注入、提權漏洞等等。系統性的安全問題,可以在設計階段識別出來。代碼實現的安全問題就比較隱蔽了,需要我們帶著攻擊者的思維去推演、挖掘。

以上就是我工作這幾年總結的代碼檢視的思路。代碼檢視的質量取決於committer投入的時間和自身的水平。單個committer很難全面的發現問題,一個可以互相學習的committer群體對提高代碼質量和團隊水平至關重要。