之前寫過一篇《寫代碼的四個(gè)境界》,那個(gè)時(shí)候结啼,大部分時(shí)候我還是愉快地寫著自己的代碼掠剑。Code review 也是每天工作的一部分,但是相對(duì)而言花的時(shí)間還是有限的郊愧。
最近一是因?yàn)榻巧D(zhuǎn)換朴译,二是突然來了很多新人井佑。花在 code review 上的時(shí)間比寫代碼多出了好多眠寿,也有一些心得和感觸躬翁,隨便寫寫吧。
總的說來盯拱,硅谷稍具規(guī)模的公司 code review 的流程都是比較規(guī)范的盒发。模式也差不多。一來所有的 PR 都必須有至少一個(gè)人 stamp狡逢,才能 merge宁舰。如果改的東西涉及到多個(gè)項(xiàng)目,則需要每個(gè)項(xiàng)目都有人 stamp 才行甚侣。還有一些特別關(guān)鍵的代碼明吩,比如支付相關(guān)的,通常也會(huì)需要支付組的人 stamp 才行殷费。
NOTE: PR (pull request, a set of changes someone tries to push to a GitHub repository. Once a PR is sent, interested parties (peer engineers) can review the set of changes, discuss potential modifications, and even push follow-up commits if necessary.
在 stamp 前印荔,通常是 github page 上可以給出各種 comments,page 是共享的详羡,所以誰都能看到仍律。有些 comments 是詢問,那么代碼作者直接回復(fù)或解釋就行实柠。有些 comments 是指出代碼的問題水泉,這樣代碼作者可能會(huì)依此改動(dòng);也可能不同意窒盐,那就回復(fù) comments草则,有的時(shí)候關(guān)于一個(gè)實(shí)現(xiàn)細(xì)節(jié)琉兜,討論的 comments thread 可以多達(dá)十幾條拍皮。這樣對(duì)于大家達(dá)成共識(shí)很有幫助链快。
以前遇到有朋友說:
“看到代碼寫的太爛帽氓,覺得來回扯皮效率不高缭受,干脆撲上去自己寫须眷∷屎冢”
曾經(jīng)我也有過類似的想法枝秤。不過最近遇到的一些事讓我慢慢地意識(shí)到這種想法很要不得嗽交。
首先就是從對(duì)方的角度來說卿嘲。對(duì)方寫不好,可能是對(duì)業(yè)務(wù)不熟悉夫壁,可能是對(duì)語言不熟悉拾枣,也可能是對(duì)公司代碼的大架構(gòu)不熟悉。如果你是幫他/她 “寫” 而不是耐心指出哪里有問題盒让,那么下一次放前,他/她可能還是不知道忿磅。不僅無益于別的人成長(zhǎng),有的時(shí)候甚至?xí)寗e人有挫敗感凭语。
再然后就是這種方式的可擴(kuò)展性很差葱她。即使 code review 會(huì)花掉哪怕十倍于你自己寫的時(shí)間和精力,但它會(huì)讓人明白代碼應(yīng)該怎么寫似扔,從長(zhǎng)遠(yuǎn)來看吨些,這其實(shí)是在一定程度上 “復(fù)制” 你的生產(chǎn)力。你不能什么都自己寫炒辉。尤其是你慢慢開始帶項(xiàng)目豪墅、帶新人。每天 review 五個(gè)人的代碼黔寇,和寫五個(gè)人的代碼偶器,你覺得長(zhǎng)期而言哪個(gè)更合算?
此外缝裤,如果說寫代碼是一個(gè)學(xué)習(xí)過程屏轰,怎么做一個(gè)好的代碼審核人更是一個(gè)學(xué)習(xí)和成長(zhǎng)的過程。自己繞過一個(gè)坑不難憋飞,難的是看到別人那么走霎苗,遠(yuǎn)遠(yuǎn)地你就能告訴他/她那里有個(gè)坑。而他/她在經(jīng)你指出多次后榛做,下一次他/她也會(huì)幫著指出別人的類似的問題唁盏。
這一點(diǎn)最近感觸尤為深刻。前一陣組里咔咔咔接二連三來了好多新人检眯,老大說厘擂,你少寫點(diǎn)代碼,多做點(diǎn) review锰瘸。于是那幾天我?guī)缀豕ぷ鞯囊话霑r(shí)間都用來看代碼驴党,寫 comments』癫纾可是最近就會(huì)發(fā)現(xiàn),他/她們相互之間大部分時(shí)候已經(jīng)可以很好的相互 review 了倔既,于是我又騰出好一些時(shí)間來做別的工作恕曲。
Code review 做多了,發(fā)現(xiàn)其實(shí)也是有一定套路的渤涌,試著歸納如下:
代碼格式方面佩谣。很多公司都有 coding style guideline。大家的約定俗成实蓬,避免公司的代碼風(fēng)格不一致茸俭,也避免一些不不要的為了 “要不要把閉括號(hào)另起一行” 而無謂地爭(zhēng)論吊履,除非是不小心,通常大家都不會(huì)弄錯(cuò)调鬓。但是新員工往往會(huì)在這方面還不太熟悉艇炎。這一類問題也比較容易指出。
代碼可讀性方面腾窝。這包括一個(gè)函數(shù)不要太長(zhǎng)缀踪,太長(zhǎng)就 break down。所有的變量名盡量能夠說明它的用意和類型虹脯。比如 hosting_address_hash驴娃,一看就知道是房東地址,而且是個(gè) hash 類型循集。不要有嵌套太多層的條件語句或者循環(huán)語句唇敞。不要有一個(gè)太長(zhǎng)的 boolean 判斷語句。如果一個(gè)函數(shù)咒彤,別人需要看你的長(zhǎng)篇注釋才能明白疆柔,那這個(gè)函數(shù)就一定有重構(gòu)的空間。另外蔼紧,如果不可避免有一些注釋婆硬,則一定要保證注釋準(zhǔn)確且與代碼完全一致。
幫代碼作者想想他/她有沒有漏掉任何 corner case奸例。很多時(shí)候這是業(yè)務(wù)邏輯相關(guān)的彬犯,尤其需要比較老的工程師幫助指出需要處理的所有情況。
Error handling查吊。這是最常見也是代碼審核最容易幫別人看出的問題谐区。舉個(gè)例子,下面一段簡(jiǎn)單到不能再簡(jiǎn)單的代碼就至少有三個(gè)潛在的問題:params 里面需要 validate 是不是有 user_id 和 new_name 這兩個(gè) key逻卖;能不能找到這個(gè) user_id 對(duì)應(yīng)的 user宋列;save 的時(shí)候會(huì)不會(huì)有 DB level 的 exception,應(yīng)該怎么處理评也。
測(cè)試?yán)头揽印?/b>測(cè)試?yán)挥谜f了炼杖,每段代碼都應(yīng)該有測(cè)試?yán)5菍?duì)于一些你能預(yù)見如果別人改動(dòng)代碼會(huì)引起可能問題的情況盗迟,一定要額外的加測(cè)試?yán)乐惯@種事情的發(fā)生坤邪。這一點(diǎn)沒有例子參考也不太好說。怎么寫好測(cè)試?yán)B疲旧砭椭档脤懸黄恼铝恕?/p>
小架構(gòu)艇纺。什么意思呢,就是一個(gè)文件或者類內(nèi)部的代碼組織。比如如果有重復(fù)的代碼段黔衡,就應(yīng)該提取出來公用蚓聘。不要在代碼里隨意設(shè)常數(shù),所有的常數(shù)都應(yīng)該文件頂部統(tǒng)一定義盟劫。哪些應(yīng)該是 private夜牡,等等
大架構(gòu)。這個(gè)就更廣了捞高。包括文件組織氯材,函數(shù)是不是應(yīng)該抽象到 lib 或者 helper 文件里;是不是應(yīng)該使用繼承類硝岗;是不是和整個(gè)代碼庫(kù)的風(fēng)格一致氢哮,等等。
當(dāng)然型檀,視具體情況可能還有很多別的需要在 code review 中注意的冗尤。但是如果按照這個(gè) checklist 過一遍,一些明顯的問題就可以避免個(gè)八九不離十了胀溺。
另外裂七,代碼 review 和寫代碼是我們每個(gè)工程師都應(yīng)該致力的兩個(gè)方面,缺一不可仓坞”沉悖可能在工作中的某個(gè)階段或者某個(gè)項(xiàng)目里,你會(huì)花更多的時(shí)間在某一面无埃,但長(zhǎng)久看來徙瓶,兩個(gè)技能缺一不可。
代碼審核是工作嫉称,不要抱有情緒化侦镇。我曾經(jīng)干過一件事,一個(gè)外組同事的代碼织阅,別人已經(jīng) stamp 了壳繁,可是我覺得有問題,于是把 stamp revert 掉了荔棉,在 comments 里寫了為什么有問題闹炉,和建議的改法。當(dāng)時(shí)心里還有點(diǎn)覺得好像怪得罪人的润樱≡ィ可是后來發(fā)現(xiàn)同事反而很客氣地接受并道謝了。心里也是很感激有這樣的工作環(huán)境祥国。
另外,經(jīng)常會(huì)遇到有些 review 的 comments 里出現(xiàn)不同意見,其實(shí)是兩個(gè)人在編程習(xí)慣和約定俗成上沒有共識(shí)舌稀。如果在不違背公司 style guideline 的情況下啊犬,沒必要一定讓對(duì)方和你有一樣的習(xí)慣。如果你真的覺得這樣更好壁查,通常我也只會(huì)寫 “I personally would prefer A over B, but no strong opinion.” 或者 “How about change it to A, since ... However, this is not a merge blocker. ”
人都有不周到的時(shí)候觉至。多幾雙眼睛一起幫你看一遍,就可以很大程度地減少代碼里的錯(cuò)誤睡腿。另外语御,相互 review 的過程中還能從彼此那里學(xué)到很多編程的小技巧和好習(xí)慣。細(xì)想來席怪,很多 Java 和 Ruby 的特別好用的 feature应闯,我都是在 code review 的過程中學(xué)到的。