Categories
程式開發

代碼審查普遍存在的6大問題


最近某寶彈窗事件導致其APP被大量用戶刪除,影響極其惡劣。我在想,如果他們的內部代碼審查更加嚴格一點,少走形式,就能將隱患扼殺在搖籃裡了。基於此,我們部門專門成立了由小組長和核心成員組成的代碼評審組,將以前的代碼評審模式進行了一些優化,加強了管控。我在以往代碼評審過程中發現了一些普遍存在的問題,總結如下。

1、缺少變更的說明

通常情況下我不寫註釋的原因只有一個,這幾行代碼過於簡單,其意思可由代碼翻譯得到,但是有些人寫的代碼里通篇沒有註釋,我就很納悶了,難道是我太菜了嘛?

說實在的,註釋和變更說明真的是一對難兄難弟,我們經常對他們視而不見,完全沒有在開源框架中註意到他們。其實不難發現框架的代碼註釋和變更說明是非常多的,包括使用示例、開始支持的版本號等等,同時也不難發現我們的代碼庫提交變更註釋滿屏都是”update”,滿懷期待地打開下一頁,終究還是被打臉了。或許背後是規範問題,或許是因為我們還沒有開始問候前人。

2、濫用異常捕獲

我在不少項目中發現一些奇怪的騷操作,每個控制器方法都使用了全局的try-catch異常捕獲,然後在其catch方法中處理可用率下降埋點。編程容易產生模板化思維,可能自己不覺得有什麼不妥,但是這種方式實在不優雅。我們完全可以使用全局切面進行統一異常處理,根據返回的錯誤碼進行可用率下降埋點。假如控制器方法返回不是統一的規範結果對象,那就停下手中的工作開始重構吧!另外優化前的方法,很容易讓我們對中間結果放鬆警惕,畢竟有這麼大而全的異常捕獲,是不是非空判斷或者非法判斷統統都可以省略了? !

上面說的問題完全不會影響代碼的運行,但是場景切換到事務操作,就不得不小心了,因為生吞異常會導致事務回滾沒有機會執行。所以,如果調用者關心異常,就盡情往上層拋吧!

3、過度相信第三方

過度相信第三方的意思,包括前端傳過來的參數沒有進行校驗、調用上游接口返回的結果沒有進行判斷、不進行參數校驗就調用其他團隊提供的寫數據接口,最後一個示例有可能導致SQL漏洞攻擊,雖然主要責任不是自身。不過,我在真實開發中就因為過度相信第三方差點體驗了一把刪庫跑路,我從類中拷貝了幾行代碼,當參數非空時就執行更新操作,但是當前端沒有傳該參數時,該參數類型為非包裝類也就是默認非空,從而覆蓋數據庫記錄。這種錯誤也是新手處理空值判斷容易犯的。

4、變量的作用域過大

我們會不經意間開始隨意玩弄變量的作用域,比如把變量的創建和賦值操作分別由兩個不同的方法實現,顯然給系統增加了不確定性。另外,變量太多容易淹沒主幹邏輯。如果它們有聯繫或者會產生協作,應該共用一個方法專門管理。

5、處理過程缺少階段性結果

沒有程序員喜歡閱讀大量的判斷語句,而提前異常判斷快速返回結果是一個基本的優化方法,比如當校驗不合法時直接返回,再比如移除控制標記,來減少循環嵌套。語義再拔高點不就是處理過程要有階段性結果了嘛,這樣代碼更加有層次感、整體流程更加清晰。

6、日記打印問題

我相信大部分公司都採用了微服務的架構,這種架構的難點之一就是問題的快速排查,特別是涉及到跨團隊。而封裝日記信息就是我們唾手可得的利器,記錄參數、中間結果、返回結果、異常信息,有了這些信息後,找上游反饋問題就更加理直氣壯了,而不需要修改代碼添加日記後重新上線。所以,盡量多記錄一些異常信息,盡量多添加一些INFO級別的日記。不過,日記記錄畢竟是旁路邏輯,因為記錄而產生的空指針異常或者內存溢出就得不償失了,所以記錄時要避免序列化空對象,避免使用無界的異步隊列。

到這裡本文就要結束了,總結一下,我在文中和你分享了六種容易犯的錯誤,包括缺少變更的說明、濫用異常捕獲、過度相信第三方、變量的作用域過大、處理過程缺少階段性結果、日記打印問題。希望對你有所幫助,歡迎分享。