Code Review 常見的5個錯誤模式

peida發表於2020-05-13

原作者:Trisha Gee

Code Review 的時候,每個人都會關心最佳實踐,但最壞的實踐有時可能會更有啟示意義。

Code Review是研發團隊必不可少的,但並不總是正確的。這篇文章指出了所有開發者在Code Review時或提交拉取請求時可能都會遇到的一些常見的錯誤模式,並對這些錯誤模式進行了總結:

錯誤模式:挑毛病

想象一下下面的場景。程式碼作者花了幾個小時,甚至幾天的時間來建立他們認為最有效的解決方案。他們考慮了多種設計方案,並選擇了看起來最相關的路徑。他們考慮了現有應用程式的架構,並在適當的地方進行了修改。然後,他們將自己的解決方案以拉動請求的形式提交,或者開始了代Code Review 過程,他們收到的專家反饋是:

  • "你應該使用標籤,而不是空格。"
  • "我不喜歡這部分的大括號在哪裡。"
  • "你的檔案末尾沒有空行。"
  • "你們的詞庫是大寫的,應該用句子大寫。"

雖然新的程式碼要與現有程式碼的風格保持一致是很重要的,但這些東西幾乎不需要人工稽核員來完成。人工稽核員的成本很高,而且可以完成計算機無法完成的事情。檢查是否符合風格標準是計算機可以輕鬆完成的事情,這就分散了程式碼稽核的真正目的。

如果開發人員在程式碼稽核過程中看到很多這樣的註釋,說明這個團隊要麼是沒有風格指南,要麼是有了風格指南,但檢查風格還沒有實現自動化。解決的辦法是使用checkstyle等工具來確保風格指南已經被遵循,或者使用sonarqube來識別常見的質量和安全問題。而不是依靠人工稽核員來警告這類問題,持續整合環境可以做到這一點。

有時,如果沒有程式碼指南,或者內部程式碼風格隨著時間的推移而變化,在不同的部分有不同的風格,那麼這種自動檢查可能會很困難。在這種情況下,有一些方法可以應用自動檢查。例如,一個團隊可以同意做一個單一的提交,應用約定的程式碼風格,並且不包含其他的更改。或者一個團隊可以約定,當一個檔案因為bug或功能而被更改時,該檔案也會被更新到新的樣式,而自動工具可以被配置為只檢查更改過的檔案。

如果一個團隊有多種程式碼樣式,而它沒有辦法自動檢查樣式,也容易落入下一個陷阱。

錯誤模式:不一致的反饋

每一個被邀請審閱程式碼的開發者,至少要多邀請一個意見,而且可能更多。每個人都可以同時持有不止一種意見。有時,Code Review 可能會陷入審查者之間關於不同方法的爭論,比如說是使用流還是經典的for迴圈最好。如果團隊成員對同一段程式碼有不同的意見,那麼開發人員應該如何進行修改,結束審閱,並將程式碼推送到生產中?

即使是一個審稿人的想法也很容易發生變化,可能是在一次審稿中,也可能是在一系列的審稿中。在一次審閱中,一個審閱者可能會催促作者確保使用O(1)讀操作的資料結構,而在下一次審閱中,審閱者可能會問為什麼不同的用例會有幾個資料結構,並建議通過單一結構進行線性搜尋來簡化程式碼。

當一個團隊對自己的 "最佳實踐 "是什麼樣子的沒有一個明確的想法,當團隊還沒有弄清楚自己的優先順序是什麼的時候,這種情況就會出現:

  • 程式碼是否應該向著更現代的Java風格發展?還是更重要的是程式碼的一致性,因此,繼續到處使用 "經典 "構造?
  • 在系統的所有部分中,對資料結構進行O(1)讀操作是否重要?還是有些部分的O(n)可以接受?

幾乎所有的設計問題都可以用 "這要看情況 "來回答。為了對答案有一個更好的想法,開發人員需要了解他們的應用和團隊的優先順序。

錯誤模式:最後一分鐘的設計變更

開發者在Code Review 過程中最讓人士氣低落的反饋是:當評審者從根本上不同意方案的設計或架構,並強行完全重寫程式碼時,要麼通過一系列的評審來逐步完成(見下一節),要麼粗暴地拒絕程式碼,讓作者重新開始。

Code Review 不是評審設計的正確時機。如果團隊按照經典的 "閘道器式 " Code Review ,那麼在最後一步讓另一個開發人員看程式碼之前,程式碼應該是可以工作的,所有的測試都應該是通過的。在這一點上,幾個小時、幾天,甚至可能是幾周(雖然我真的希望不是幾周;Code Review 應該是小事一樁,但這是另一個話題)的努力已經花在了被審查的程式碼上。在Code Review 中指出底層設計是錯誤的,這是在浪費大家的時間。

Code Review 可以作為設計審查,但如果這是Code Review 的意圖,那麼審查應該在實現之初就進行。然後,在開發人員還沒有走得太遠之前,他們可以把自己的想法勾勒出來,也許會有一些存根類和方法,以及一些有意義的名稱和步驟的測試,也許還可以提交一些文字或圖表,以便讓團隊成員對將要採取的方法進行反饋。

如果團隊成員在關口審查中發現了真正的展示性設計問題(也就是說,當程式碼完成並執行時),團隊應該更新流程,以便更早地定位這些問題。這可能意味著要做其他型別的審查,比如上一段中建議的審查,白板上的想法,配對程式設計,或者與技術負責人討論建議的解決方案。在最後的Code Review 中發現設計問題是對開發時間的浪費,也是對程式碼作者的極大打擊。

錯誤模式:乒乓球 Reviews

在一個理想的世界裡,作者會提交程式碼進行評審,評審人員會提出一些明確的解決方案的意見,作者提出修改建議並重新提交程式碼,評審結束,程式碼就會被推送。但如果這樣的事情經常發生,誰還能說得清 Code Review 的過程是有道理的呢?

在現實生活中,經常出現的情況是這樣的:

  1. 一個Code Review開始了。
  2. 一些審稿人提出了幾個建議:有的小而容易,有的蓬頭垢面,沒有明顯的解決方案,有的複雜。
  3. 作者做了一些修改:至少是簡單的修改,或者說是幾處修改,力求讓審稿人滿意。作者可能會向審稿人提出問題來澄清一些事情,或者作者可能會提出意見,解釋為什麼沒有做出特定的修改。
  4. 審稿人回來後,接受一些更新,對其他的修改提出進一步的意見,找到他們不喜歡的地方,回答問題,並在審稿中與其他審稿人或作者爭論他們的意見。
  5. 程式碼作者做更多的修改,增加更多的評論和問題,以此類推。
  6. 審稿人檢查修改,提出更多的意見和建議,以此類推。
  7. 步驟5和6重複進行,或許永遠都是這樣。

在這個過程中,理論上來說,修改和批註應該向著零的方向下降,直到程式碼準備好為止。最鬱悶的情況是,每一次迭代都會帶來至少和已經結束的舊問題一樣多的新問題。在這種情況下,團隊就進入了 "Code Review 的無限迴圈"。發生這種情況的原因有很多:

  • 如果審稿人吹毛求疵,如果審稿人給出的反饋不一致,就會出現這種情況。對於陷入這些習慣的審稿人來說,有無限多的問題需要找出,有無限多的意見需要提出。

  • 當審稿時沒有明確的審稿目的,或者審稿時沒有準則可循,就會出現這種情況,因為這樣一來,每個審稿人都會覺得每一個可能出現的問題都必須找出來。

  • 當不清楚審稿人的評論對程式碼作者的要求是什麼時就會發生。是不是每一條評論都意味著必須要進行修改?所有的問題是否都暗示著程式碼沒有自證,需要改進?還是有些評論僅僅是為了教育程式碼作者下一次,而提出問題只是為了幫助審稿人理解和學習?

評論應該被理解為阻止者或不是阻止者,如果審稿人決定程式碼需要修改,他們需要明確說明程式碼作者應該採取什麼行動。

同樣重要的是,要明白由誰來決定稽核是否 "完成"。這可以通過任務清單上的檢查專案來實現,也可以由個人授權說 "足夠好 "來完成。通常需要有一個人能夠打破僵局,解決分歧。這個人可能是高階開發人員、領導或者是架構師,甚至是團隊中的程式碼作者,因為在團隊中,他們之間的信任度很高。但是,在某些時候,需要有人說 "評審結束了 "或者 "當這些步驟完成後,評審就結束了。"

錯誤模式:幽靈審查

在這裡我承認我最容易犯的反常的地方:幽靈化。無論我是審閱者還是程式碼作者,在程式碼審閱中都會出現一個點(有時就在開始的時候!),在審閱過程中,我根本就沒有回應。也許有一個重要或有趣的功能被要求我審閱,所以我決定把它留到 "更好的時候",等我可以 "真正好好看一看 "的時候再做。又或許是Review的量大,我想留出充足的時間。又或許是我是作者,在迭代(或二十次)後,我就是無法面對閱讀和回覆評論了,所以我決定等 "等我的腦袋想好了再來"。

聽起來是不是很熟悉?

不管是什麼原因,有時在審查過程中有人根本沒有反應。這可能意味著在這個人看完程式碼之前,審查工作就已經死氣沉沉了。這是一種浪費。即使有人在建立一個資產(新程式碼)上投入了時間,但在它投入生產之前,它並沒有增加價值。事實上,當它在程式碼庫中越來越落後於其他程式碼庫的時候,它很可能已經腐爛了。

有幾個因素會導致幽靈審查。龐大的程式碼稽核量是一個因素,因為誰願意去翻閱幾十個或幾百個修改過的檔案?不重視Code Review 是另一個因素,因為不重視Code Review 是真正的工作或交付成果的一部分。困難的或令人沮喪的Code Review 經歷是另一個主要因素。沒有人願意停止編碼(開發人員通常喜歡的東西),去參加一項耗費時間和破壞靈魂的活動。

以下是解決幽靈審查的建議:

  • 確保Code Review 的規模要小。每個團隊都要制定出自己的定義,但這將是幾個小時或幾天的複審工作,而不是幾周的時間。
  • 確保Code Review 的目的很明確,審查人員應該找的東西很清楚。當範圍是 "找到程式碼中可能存在的任何問題 "時,很難激勵自己去做一件事。"
  • 在開發過程中留出時間來做Code Review 。

最後一點可能需要團隊的紀律性,或者團隊可能希望通過(例如)通過目標或任何用來決定開發人員的生產力的機制來獎勵良好的Code Review 行為來鼓勵允許時間。

你的團隊能做什麼?

對於研發團隊,專注於建立一個行之有效的Code Review流程。我在我的部落格上寫過這方面的內容,但想在這裡分享一下這個過程的一部分:

  • 在進行Code Review 時,有很多事情需要考慮,如果開發人員在每次Code Review 時都擔心所有的事情,那麼任何程式碼都幾乎不可能通過評審流程。要實現一個適合所有人的Code Review 流程,最好的方法是考慮以下問題。
  • 團隊為什麼要做審閱?當有一個明確的目的時,審查員的工作會更容易,程式碼作者也會從審查過程中減少討厭的驚喜。
  • 團隊成員要找的是什麼?當有了目的,開發人員可以在審閱程式碼時建立一套更有針對性的東西來檢查。
  • 誰來參與?誰來做評審,誰負責解決意見衝突,誰最終決定程式碼是否合格?
  • 團隊何時進行復審,複審何時完成?稽核可以在開發人員在程式碼工作的時候迭代進行,也可以在流程結束時進行。如果沒有明確的指導,一個評審可能會一直進行下去,如果沒有明確的指導,程式碼最終什麼時候可以進行。
  • 團隊在哪裡做評審?Code Review 不需要特定的工具,所以審查可能就像作者在辦公桌上帶領同事看他們的程式碼一樣簡單。

一旦回答了這些問題,你的團隊就應該能夠建立一個執行良好的Code Review 流程。記住:Code Review 的目的應該是讓程式碼投入生產,而不是證明開發人員有多聰明。

結論

Code Review 的錯誤模式可以通過建立一個明確的Code Review 流程來消除,或者至少是緩解。許多團隊認為他們應該進行Code Review ,但他們沒有明確的準則,為什麼要進行Code Review 。

不同的團隊需要不同型別的Code Review ,就像不同的應用程式有不同的業務和效能要求一樣。第一步是弄清楚團隊為什麼需要審閱程式碼,然後團隊就可以著手於:

  • 自動化的簡易檢查(例如,檢查程式碼樣式,識別常見的BUG,發現安全問題)。
  • 就審查的時間、審查的內容以及審查結束後由誰決定等問題制定明確的準則
  • 將Code Review 作為開發過程的一個關鍵工作內容

專注於為什麼要進行Code Review ,將幫助團隊建立Code Review 流程的最佳實踐,這樣就更容易避免Code Review 的錯誤模式。

原文:https://blogs.oracle.com/javamagazine/five-code-review-antipatterns

相關文章