Code Review 時，曾被我忽視的 3 件重要小事
共 6379字，需瀏覽 13分鐘
·
2024-04-22 12:00
來源：piglei
Code Review（代碼評審）是一種流行的軟件開發(fā)實踐。通過在代碼合入主分支前引入人工評審，能有效促進(jìn)成員間的知識交流，提升軟件質(zhì)量。
我以評審者的身份參與過大量代碼評審。在評審一份代碼時，有些事項長期處在我的關(guān)注榜頭部，比如設(shè)計是否考慮到了邊界情況、代碼是否有合理的單測覆蓋。也有一些事項，因看似無關(guān)痛癢一直未引起足夠重視，直到最近，我才漸漸發(fā)現(xiàn)它們的重要性。
以下是曾被我忽視的 3 件重要的小事。
1. 命名
小女孩千尋誤入湯婆婆為神明開設(shè)的浴場。為了留在浴場內(nèi)工作，千尋與湯婆婆簽訂了一份協(xié)議，但協(xié)議并非重點(diǎn)，重點(diǎn)是另一件看似無關(guān)緊要的小事——湯婆婆給千尋改了個名：從“千尋”改為“千”。一旦失去了原本的名字，人們便失去了逃離浴場所在的異世界的能力，甘心永世被湯婆婆所奴役。
——電影《千與千尋》
程序員們對“命名”的關(guān)注程度似乎呈一個“倒 U 形”曲線。缺乏經(jīng)驗時，對命名的關(guān)注度很低，代碼中充斥著各類不準(zhǔn)確、不精確的名字，無法有效描述各種抽象概念。
下面這段代碼中的命名就存在不少問題：
def get_var(u):"""獲取環(huán)境變量列表"""data1 = UserVarsManager.get(u)data2 = SiteVarsManager.get(u.site)return data1 + data2
隨著經(jīng)驗逐漸增加，大家對命名的關(guān)注度逐步提升。項目中的名字開始變得更具有描述性，含糊不清的名字漸漸絕跡。名字至少不會成為他人理解代碼時的屏障。
這個階段，代碼會逐漸演變成像是這樣：
def list_environment_vars(user):"""獲取環(huán)境變量列表"""items_user = UserVarsManager.get(user)items_site = SiteVarsManager.get(user.site)return items_user + items_site
在絕大多數(shù)評審中，這絕對算是一份合格的代碼，至少不大可能因為命名應(yīng)發(fā)爭議。
自此之后，大部分程序員們對命名的關(guān)注度進(jìn)入“倒 U 形”曲線的后半段：不再如從前那般關(guān)注命名，名字只要有一定描述性，不造成歧義就足夠。我也曾經(jīng)是其中一員。
但不應(yīng)在這個階段停留太久，作為代碼評審人，我們應(yīng)該不斷提升自己對于名字的敏感度。比方說，對于前面那份代碼，也許應(yīng)該提出以下評審建議：
def list_environment_vars(user): # 1"""獲取環(huán)境變量列表"""items_user = UserVarsManager.get(user) # 2items_site = SiteVarsManager.get(user.site)return items_user + items_site
?評論 1： 項目中對于“環(huán)境變量”的統(tǒng)一縮寫是 env_variables /env_vars，此處應(yīng)保持一致，使用 list_env_variables 或 list_env_vars。?評論 2： UserVarsManager.get 的命名可優(yōu)化，因為 Manager 是一個“萬金油”名詞，雖然放在各種場景下都不違和，但也是以損失名字（等同于“職責(zé)”）的精確指向性為代價，此處可考慮改用一個更精確的名字，比如：UserEnvVarsRetriever.get(user)；SiteVarsManager 同理。
雖然只是兩處小改進(jìn)，但是積少成多。
每一次代碼評審，必定涉及到許多新名字。但名字并非生來平等，不是所有名字都值得我們花費(fèi)時間，應(yīng)當(dāng)盡量把關(guān)注點(diǎn)聚焦在那些最常被使用、最靠近用戶的名字上，比如 URL 路徑的資源名、數(shù)據(jù)庫模型與字段名、工具函數(shù)（類方法）名，等等。
此外，與業(yè)務(wù)直接相關(guān)的領(lǐng)域詞匯重要程度極高。評審時，每一個關(guān)鍵的領(lǐng)域詞匯都值得仔細(xì)斟酌、反復(fù)推敲。舉個例子，開發(fā)一個影評功能，”用戶評分“、“媒體評分”、“平均分”分別該用哪些名字表示？你絕不會想要在一個文件里看到 movie_score，在另一個文件里看到 movie_rating。
命名這件小事，雖然看似不起眼，但項目規(guī)模越大、所跨越的時間維度越長，在名字質(zhì)量上的細(xì)微差別就越容易累加出不可估量的巨大影響。
2. 指引性注釋
夏洛已經(jīng)在網(wǎng)上織出了光彩照人四個大字，威爾伯站在金色的陽光里，真是光彩照人。自從蜘蛛開始扶助它，它就盡力活得跟它的名聲相襯。夏洛的網(wǎng)說它是王牌豬，威爾伯盡力讓自己看上去是只王牌豬；夏洛的網(wǎng)說它了不起，威爾伯盡力讓自己看上去了不起；現(xiàn)在網(wǎng)上說它光彩照人，它盡力讓自己光彩照人。
——《夏洛的網(wǎng)》
關(guān)于注釋，我向來信奉 Bob 大叔在《代碼整潔之道》[1]里的觀點(diǎn)：“注釋的恰當(dāng)用法是彌補(bǔ)我們在用代碼表達(dá)意圖時遭遇的失敗?！?/strong> 這就是說，好代碼應(yīng)該總是能清晰說明自身意圖，無需注釋再來畫蛇添足，注釋只應(yīng)該被用來描述那些代碼之外的信息，比如解釋“為什么”。
正因如此，注釋總是應(yīng)該被謹(jǐn)慎使用。假如一段代碼很難理解，第一反應(yīng)不應(yīng)該是補(bǔ)注釋，而是應(yīng)該去追求用一種更易理解的方式重寫它。
但隨著時間的推移，我漸漸意識到，事情不能一概而論。“指引性注釋”，或者說常被人們詬病為“近乎復(fù)述代碼意圖”的描述性文字，也有著不可替代的重要作用。
Redis 的作者 antirez 就是“指引性注釋”的忠實擁護(hù)者，他曾寫過一篇文章[2]詳細(xì)分析過指引性注釋在 Redis 項目中的應(yīng)用。下面這段代碼摘自 Redis 源碼，里面就有不少“指引性注釋”：
/* Call the node callback if any, and replace the node pointer* if the callback returns true. */if (it->node_cb && it->node_cb(&it->node))memcpy(cp,&it->node,sizeof(it->node));/* For "next" step, stop every time we find a key along the* way, since the key is lexicographically smaller compared to* what follows in the sub-children. */if (it->node->iskey) {it->data = raxGetData(it->node);return 1;}
在這段代碼中，兩段注釋并未提供任何在代碼之外的新信息。所以，好處是什么？
最直觀的好處，就是這些注釋讓代碼變得更容易理解了，它們極大地降低了人們閱讀代碼時所需付出的心智成本。同樣一份代碼，在缺少指引性注釋的情況下，完全理解它的行為可能得花費(fèi) 10 分鐘，而有了注釋的幫助，時間也許就能縮短到 5 分鐘甚至更短。
當(dāng)新開發(fā)者加入項目時，這些指引性注釋也能助力他們更快上手。
正因如此，在評審一份代碼時，我常常會在一段復(fù)雜的代碼邏輯上評論：“Nit：考慮增加一小段指引性注釋，幫助理清代碼行為?！保∟it=nitpick，表示“雞蛋里挑骨頭”式的并不強(qiáng)烈要求修改的意見）。
此外，如果一段代碼曾在評審過程中引發(fā)過一些深度討論，那么那些討論內(nèi)容，也許很適合被二次加工后，作為指引性注釋加入代碼中。對于理解代碼來說，它們有時有奇效。
不過，在追求“指引性注釋”的路上，也要避免踩入以下幾個陷阱：
?簡單復(fù)述代碼：指引性注釋雖然是一種幫助理解代碼的輔助性文字，但絕不能只是復(fù)述代碼而已，簡單來說，你可以這么理解兩者在傳遞信息方面的風(fēng)格差異：代碼是一本厚厚的權(quán)威科學(xué)教材，指引性注釋則是一小冊面向中學(xué)生的科學(xué)啟蒙讀物?追求“注釋率”：不要在“代碼注釋率”指標(biāo)上設(shè)置硬性要求，指引性文字也需要講究質(zhì)量，盲目追求數(shù)量只會適得其反?不注重時效性：過時的注釋比代碼危害更大，要及時修改或刪除已經(jīng)過期的指引性注釋
總而言之，你可以把指引性注釋當(dāng)成有針對性的代碼“教學(xué)文本”。審閱代碼時，如果你發(fā)現(xiàn)一段邏輯理解起來很吃力，而代碼本身也沒有太多優(yōu)化空間，請不要遲疑，勇敢表達(dá)出你對于“教學(xué)文本”的需求吧！
3. 溝通方式
“我因為魯思和薩拉不得不離開我們而痛苦萬分。而令我感覺更加痛苦的是我當(dāng)時以為自己是完全孤立無援的?！?/strong>
“說真的，肯頓小姐……”我端起那個我用來放使用過的瓷器的托盤?！皩δ菢拥慕夤臀易匀皇菢O不贊同的。我還以為那是不言自明的?！?/strong>
——《長日將盡》
時至今日，仍有許多人認(rèn)為軟件開發(fā)是一種單打獨(dú)斗的工作。一位程序員撿起一塊鍵盤，就能源源不斷地產(chǎn)出代碼，根本不需要其他人。但事實是，程序員單打獨(dú)斗的黃金時代早已過去，現(xiàn)代軟件開發(fā)已演變成一種多人參與的協(xié)作事務(wù)。正因如此，程序員的日常充斥著各類溝通工作，參與代碼評審正是其中之一。
在代碼評審時，評審者的工作內(nèi)容似乎一句話就可簡單概括：指出他人代碼中的不足。 這聽起來易如反掌，對不對？我曾經(jīng)正是這么以為，所謂評審，只要做完下面的“123”即可：
1.找出所有可優(yōu)化的點(diǎn)（有事說事?。?/span>2.等提交者完成改動，或在討論后確認(rèn)維持原狀（就事論事?。?/span>3.合并代碼（大功告成?。?/span>
而現(xiàn)實總是和理想相去甚遠(yuǎn)，代碼評審很少會像上面這樣順利。因為一旦涉及到人與人之間的溝通，尤其其中一方還在給另外一方“挑毛病”，事情又能簡單到哪兒去呢？
人類是一種神奇的智慧生物，閱讀一段文字，不僅能從中獲取到信息，更能從字里行間感受到情緒，有時，這份情緒甚至?xí)w過信息，影響他們做出判斷。因此，當(dāng)你在參與評審時，請謹(jǐn)記這一點(diǎn)：保持謙遜、尊重他人，無論對方的經(jīng)驗或背景如何。優(yōu)秀的表達(dá)，能做到內(nèi)容即使在批評，也能讓對方感受到自己仍是被尊重的。
讓我來舉一個例子。團(tuán)隊內(nèi)來了一位新人，用他不太熟悉的 Python 語言提交了一個 PR。作為 PR 的評審人，你在代碼里發(fā)現(xiàn)了一段冗長的循環(huán)代碼，于是寫下評論：
代碼比較啰嗦，建議改成列表推導(dǎo)式。雖然你的觀點(diǎn)沒錯，但這種表達(dá)方式值得商榷。下面是這條評論的另一種寫法：
這里的循環(huán)體較簡單，只有過濾和轉(zhuǎn)換邏輯，很適合改成列表推導(dǎo)式，代碼更精簡。舉個例子：items = [to_item(obj) for obj in objs if obj.is_valid()]參考：https://docs.python.org/3/tutorial/datastructures.html#list-comprehensions
比起第一條評論，后面這條顯然更不容易引起新人的抵觸情緒，更可能被采納。這就像那句老話所說：“你表達(dá)的方式跟所要表達(dá)的內(nèi)容同樣重要，如果不是更重要的話。”
除了保持謙遜和尊重，還有一些其他值得采用的評審溝通小技巧：
?一例勝千言：有時用文字洋洋灑灑寫一大堆，不如直接寫幾行代碼，舉一個實際的代碼樣例?見什么人說什么話：對于加入團(tuán)隊一個月和一年的開發(fā)者，你在評審時可以（也應(yīng)該）區(qū)別對待；對缺乏經(jīng)驗的新人，組織語言時要謹(jǐn)慎，盡量避免讓對方感覺不被尊重，產(chǎn)生太多的挫敗感；對熟稔的老人，語言風(fēng)格就可以相對隨意，言簡意賅即可，不必過于啰嗦
我相信也許大部分人在心底都認(rèn)同：代碼評審是一個“對事不對人”的過程，不應(yīng)該把對代碼的批評當(dāng)成對人的否定。但這和提倡“好好說話”并不沖突。一次讓雙方滿意的溝通，幾乎等同于一次更高效的溝通。所以，改善溝通方式就能提升工作效率，你又何樂而不為呢？
總結(jié)
代碼評審作為保障軟件質(zhì)量的重要手段，是大型軟件開發(fā)中不可或缺的重要一環(huán)。本文總結(jié)了我作為評審的參與者，在命名、指引性注釋和溝通方式三個方面的一些思考，要點(diǎn)如下：
?程序員們應(yīng)當(dāng)不斷提高在代碼評審時對于命名的敏感度?檢查命名的兩個技巧：同類名詞保持一致、用更精確的詞代替那些“萬金油”名字?對待名字不要一視同仁，多多關(guān)注那些最重要的名字?對任何一個項目，領(lǐng)域（業(yè)務(wù)）相關(guān)的名字最重要，值得仔細(xì)斟酌、反復(fù)推敲?指引性注釋雖不提供太多代碼之外的信息，但也有著不可替代的作用?指引性注釋能大大降低人們?yōu)槔斫獯a所付出的心智成本?留意指引性注釋的幾個陷阱：簡單復(fù)述代碼、追求“注釋率”、不注重時效性?評審時，要勇于對復(fù)雜的代碼邏輯提出補(bǔ)充指引性注釋的請求?軟件開發(fā)中包含許多與溝通有關(guān)的事項，代碼評審正是其中之一?理想的評審是“有事說事，就事論事”的，但正因涉及人際溝通，導(dǎo)致現(xiàn)實往往偏離理想?文字不光能傳達(dá)信息，更是情緒的載體，而情緒往往會影響溝通的效果?在評審中，永遠(yuǎn)保持謙遜、尊重他人，無論對方的經(jīng)驗或背景如何?對于有著不同經(jīng)驗的待評審者，應(yīng)當(dāng)采取不同的溝通風(fēng)格
代碼評審是一項涉及多人協(xié)作的復(fù)雜事務(wù)，里面藏著許許多多的學(xué)問。質(zhì)量高的評審，對于提升質(zhì)量和塑造團(tuán)隊氛圍有著不可替代的作用。質(zhì)量低下的評審，則可能淪落為形式主義，甚至讓團(tuán)隊內(nèi)部滋生矛盾和不滿。
而影響評審質(zhì)量的因素，往往藏在那些不起眼的小細(xì)節(jié)、小事情中。以上這些關(guān)于“小事情”的經(jīng)驗之談，希望能對你的工作有所啟發(fā)。
題圖來源: Photo by Helena Lopes on Unsplash
References
[1] 《代碼整潔之道》: https://book.douban.com/subject/4199741/[2] 一篇文章: http://antirez.com/news/124
這是一個專為國內(nèi) Python 開發(fā)者量身打造的資訊平臺，為你挑選最值得分享的文章、教程、開源項目、軟件工具、播客和視頻、熱門話題等內(nèi)容。