關於Code review這檔事

說到code review這件事，相信這年頭凡是吃碼農這行飯的，多多少少已經成為工作的一部分了( 啥？貴公司沒在做這件事甚至反對這件事？塊陶啊～～～)

如果估狗『how to code review』、『code review guidelines』等關鍵字，很容易就可以找到許多不錯的參考文章，但很多都是超過十項的點檢表，我實在不擅長記憶這麼多項目，常常邊看邊忘，在腦內只留下了浪漫模糊的輪廓。

點檢表是非常有用的，特別是在初期對一個code base還很不熟悉的時候。例如咱家WordPress VIP code review guidelines、WordPress theme development checklist、這篇Kevin London的Code Review Best Practices也曾被轉載一陣。待對code base熟悉到一個程度以上後，記憶幾個好記的思考源作起點再從之延伸，對我來說比較容易掌握：

1. 我了解整個patch / PR的目的嗎？
2. 我知道該怎麼測試嗎？
3. 每一行變更的意義我都完全了解嗎？
4. 我們能做得更好嗎？

從『目的』延伸

從『了解目的』的延伸較單純，主要就是要求用意清楚、單一，這通常代表了這個patch / PR不能太大、盡可能只有單一個目的、說明必須清楚。不論對reviewer或作者來說，這些都是在進入程式碼前就該做到的事情。

從『測試』延伸

有unit test就要跑、有相關integration test要跑，如果是跟使用者情境有關的，一定要去實際用用看；用潮一點的話來說，就是要做e2e test。作者應當要把整個測試計畫寫清楚，力求reviewer可以挖著鼻孔傻傻跟著步驟做就可以完全測試完。此一基本要求做到後，才是reviewer把手指擦乾淨，該發揮點創意的時候了：想辦法把測試弄壞。

對unit test或automated integration test來說比較簡單，挑其中幾項把input弄成『理應出錯』的狀態，新加的測試尤其要試，確認這些測試不是在那邊擺好看的。這件事在php或js這類語言上尤其重要，因為語言本身能給的正確性保證實在是少得可憐。舉個例，下面是我最近碰到的一個case的簡化版：

describe( 'just a example', () => {
    it( 'func a should have been called.', () => {
        const func = sinon.spy();
        somethingThatCallsFuncOnEven( 2, func );
        expect( func ).to.have.beenCalled();
    } );
} );

假設somethingThatCallsFuncOnEven是個當第一個參數是偶數時會call func的函式，上面這個測試理當改傳入1, 3, 5等奇數就會報錯，但它還是過了，為什麼？因為beenCalled永遠都是true，正確的呼叫法是been.called()。我沒去研究為什麼會有這麼奇怪的行為，但這是我真切碰到的案例。

使用測試嘛…好像就沒什麼系統化的方法，我個人的方法是『想像我沒看過程式碼』。我以前在鈊X工作的時候有幸認識幾位這方面非常有天份的測試員，他們總是能做出我想都沒想過的動作把程式搞爆；這邊連按幾下、那幾個鍵一起按按看、這邊好像可點耶~點點看…啊，爆炸了 ^.< 諸如此類。從他們身上，我發現我作為『知道實作』的人，太過清楚『預期中的使用流程』了，很容易有盲點。 當然我不可能完全沒看到程式碼，不然怎麼做code review? 但在做這部分測試時，適時把視點切換、改從使用情境著眼，我的經驗上非常有幫助。

了解每一行的意義

特地強調『每一行』是為了從一開始就有精度上的心理準備。我覺得code review非常忌諱『大致上看起來對』，尤其當你的同事們都是高水平的碼農時，如果不拿放大鏡看，很難找到隱藏的瑕疵。搞不清楚的時候，想想自己可能會怎麼做是不錯的方法，常常這樣一來一往的思考下就通了。當然，最後的一招就是直接抓作者本人來問。

我們可以做得更好嗎？

與其說關鍵字是『更好』，其實是『我們』。強調『我們』，是因為我認為code review是合作學習，而非老師審閱學生。有了這一層認識後，再來談改善就會自然、輕鬆許多。

實作方法符合best practices嗎？程式碼能再精簡嗎？命名直白嗎？效能有進步空間嗎？有適當的文件嗎？精微之處有註解嗎？如果手邊有公司要求的落落長的checklist(例如前面提過的WordPress VIP checklist)，這是個讓它重新出場的好時機。

總之，快樂code review！

由於個人不擅長記憶太瑣碎的項目，又不喜歡隨時拿張表來檢查，以上四項不論我在哪個code base工作，都是我用來review的基本起點，目前為止至少在我個人還運作得不錯。魯莽deploy真的是很危險的事情，雖然一開始進展很快很有快感，但積累太多髒污最後一口氣噴發的時候，可是會讓覺得自己好棒棒的那些愉悅都打成笑話一樁的。以前亂上程式碼發生過不少慘事，有人曾形容中國近年經濟的問題就是『步子邁太大，扯到蛋了』，我的狀況差不多就是那樣。與其說嚴謹的code review表面上讓進展變慢、長期上是變快，我覺得倒不如說這才是正常的步調，省去之後的快本來就是假象。