整潔代碼不代表不需要注釋

文章目錄

• 優秀的程式猿時刻謹記重構
• 莫名其妙的Incident原因
• 沒有注釋的大坑
• 整潔代碼中好的注釋
• 總結

精排版👈請戳這裡

這篇文章的由來呢，源于最近在工作的過程中發現的一個非常有意思的Bug！

而正是因為這個Bug，我們團隊還搞了個Incident😭😭！也是以，給了我一個非常深刻的教訓，是以我決定将整個事件還原，記錄下來！

下面，我就來詳細介紹介紹這個Bug吧！

出于對客戶資訊安全的考慮，以下所有代碼均為模拟代碼，僅用于示範，不含真實源碼！

優秀的程式猿時刻謹記重構

事件源于一個Production issue(細節就不說了)，而為了修複該issue，團隊内的小紅對API Service做了一些修改，如下：

......

var filteredList = list
  .Where( a == null || (a.jobId == b.jobId || (a.jobId == null && b.jobId == null)) )
  .ToList();

......
           

從PR👆的對比來看，改動隻有一處，就是加了一個

a == null

的判斷，而這個新增的判斷則是為了将資料篩選的範圍放大一些，以便修複上述的Production issue。

而在看到這個PR後，團隊内的小明很敏銳地發現了一個非常不合理的地方👇：

a.jobId == b.jobId || (a.jobId == null && b.jobId == null)
           

這個表達式乍一看，應該是能簡化成

a.jobId == b.jobId

的！

但由于對C#不是很熟悉(小明寫過Java，目前是在搞JS&Node)，是以小明特意找了個線上編輯器試了下：

确認沒問題，于是小明向小紅提出了重構這個表達式的想法，而小紅也欣然接受了，于是重構後的代碼變成了這樣⬇️：

......

var filteredList = list
  .Where( a == null || a.jobId == b.jobId )
  .ToList();

......
           

Approve後，代碼被成功合并到了master，再接着，上了Production！

莫名其妙的Incident原因

代碼改動上線後的第二天早上，客戶方突然抛出了一個Incident，說是某個資料頁面的資料顯示不全，影響到了一部分資料展示及部分使用者，同時初步将Incident定級為P3！

接到消息的小明和小紅等人立即開始和客戶溝通Incident的現象，同時開始對代碼進行排查，看這個Incident是最近的改動造成的還是一直存在的！

然而很不幸，經過排查發現，導緻這個Incident的正是之前的改動！

但是，看着之前的改動，團隊内的幾個開發統統陷入了沉默，從代碼⬇️來看，改動是完全沒有問題的，可是怎麼會導緻出錯了呢？

......

var filteredList = list
  .Where( a == null || a.jobId == b.jobId )
  .ToList();

......
           

于是，為了将錯誤範圍再次縮小(此時已經和客戶溝通清楚了Incident的具體表現)，小紅等人對幾個判斷條件一一進行了測試，同時得到了測試結果：

判斷條件	測試結果
a.jobId == b.jobId || (a.jobId == null && b.jobId == null)	資料未缺失，但前一個Production issue未修複
a == null || (a.jobId == b.jobId || (a.jobId == null && b.jobId == null))	資料未缺失，且前一個Production issue被修複
a == null || a.jobId == b.jobId	資料缺失，但前一個Production issue被修複
a == null || (a.jobId == null && b.jobId == null)	資料缺失大部分，但前一個Production issue被修複
a.jobId == b.jobId	資料缺失，且前一個Production issue未修複

經過測試及對測試結果⬆️的比較，團隊得出，導緻該Incident的原因就是👉：删除

(a.jobId == null && b.jobId == null)

導緻部分資料未被篩選到，進而導緻資料顯示不全！

可是，這個原因是讓小紅等人難以接受的，明明還特意測試過了，在C#中，

null == null

的結果就是

True

啊！

a.jobId == b.jobId || (a.jobId == null && b.jobId == null)

完全可以被簡化為

a.jobId == b.jobId

啊，怎麼會導緻錯誤呢？

除非……

null == null

的結果是

False

！

沒有注釋的大坑

可是，小明分明測試過了，在C#中，****

null == null

的結果就是

True

，但是，從測試的結果來看，卻得到了一個相反的結果：

null == null

的結果是

False

！

那麼，就隻有一種可能👉**

a.jobId == null && b.jobId == null

**這個判斷條件不是C#語句！

這句話被小明說了出來，然後突然，團隊内的小李(團隊内唯一會C#的開發)說道：“這個語句确實不是C#，這個代碼塊的作用是将C#語句轉換為SQL Server查詢語句！”

納尼？

小明和小紅仿佛明白了什麼，悶頭就是一通搜尋！幾乎沒花費什麼時間，小明找到了一篇文章：Why does NULL == NULL evaluate to false in SQL server

雖然不是官方的解釋，但是看到這篇文章标題的時候，小明其實就已經明白了。

好心重構，結果辦了壞事！

知道如何修複，搞清楚了原因，幾人趕緊送出了改動，并将原因和客戶方做了解釋。

松下一口氣後的幾人開始反思，到底是什麼原因導緻這個錯誤的修改被“順理成章”地merge了呢？

最終經過讨論，小紅、小明、小李幾人一緻認為，這裡應該有一個注釋，表明**這裡的****

null == null

**并非C#語句，而是被轉換成了SQL Server查詢語句，而在SQL Server中， NULL ≠ NULL！

也就是說，正确的代碼應該是這樣的⬇️：

......

// 請勿删除條件 (a.jobId == null && b.jobId == null)
// 該語句将被轉換為SQL Server查詢語句，而在SQL Server中，NULL != NULL
// 參考：https://stackoverflow.com/questions/1843451/why-does-null-null-evaluate-to-false-in-sql-server
var filteredList = list
  .Where( a == null || (a.jobId == b.jobId || (a.jobId == null && b.jobId == null)) )
  .ToList();

......
           

整潔代碼中好的注釋

之前在看《Clean Code》的時候，書中有強調，不要寫無用的注釋，應該用代碼來闡述意圖，比如：👇

// return user name
function getName() {
  return this.name;
}

// 👎 Bad
           

function getName() {
  return this.name;
}

// 👍 Good
           

第一個代碼片段顯然是一個非常糟糕的注釋，毫無用處，通過方法名，我可以百分百地了解到代碼的意圖，這樣的注釋就是“脫褲子放屁”！

對于這樣的代碼來說，最好的注釋就是不要注釋！

同時，由于工作經曆的緣故，我的上一個項目非常注重代碼的整潔，我們幾乎每天都會留出30min～1h來做Code Review，而我們經常說的一句話是，能用函數名說明目的的代碼，就不要加注釋！

可能是由于這種種因素結合吧，我幾乎已經下意識地認為代碼中不應該寫注釋了(API及SDK除外)，但通過這次的Incident，我突然發現，對于一些比較容易造成誤解的代碼，注釋真的可以避免很多損失！

......

var filteredList = list
  .Where( a == null || (a.jobId == b.jobId || (a.jobId == null && b.jobId == null)) )
  .ToList();

......

// 👎 Bad
           

......

// 請勿删除條件 (a.jobId == null && b.jobId == null)
// 該語句将被轉換為SQL Server查詢語句，而在SQL Server中，NULL != NULL
// 參考：https://stackoverflow.com/questions/1843451/why-does-null-null-evaluate-to-false-in-sql-server
var filteredList = list
  .Where( a == null || (a.jobId == b.jobId || (a.jobId == null && b.jobId == null)) )
  .ToList();

......

// 👍 Good
           

同時，在寫下這些的同時，我還想到了我所負責的上一個新功能，由于新功能的開發是在老功能的基礎上做一些添加，但是由于老功能非常複雜，計算過程非常難以了解(梳理計算過程花費了兩天)，是以，導緻新功能的計算過程同樣非常難以了解，幾乎不可能維護，屬于那種别人不敢改的代碼！

但是，新功能還必須上(它是為了彌補老的架構的不足的一個緩沖方案)，于是，一個對新功能代碼的解釋迫在眉睫！

我們當時的解決方案是寫了一個文檔，裡面包含了幾部分内容：

• 老架構的不足
• 新功能的必要性
• 老功能及代碼意圖的解釋
• 新功能及代碼意圖的解釋

最後，我們将對代碼的解釋文檔的連結放在了代碼注釋裡！如👇：

// https://confluence.atlassian.com/example/explain-of-new-feature.html
ExampleModule module = generateNewModule(oldModule);

...
           

總結

整潔代碼不代表不需要注釋，而是需要一些有價值、有意義的注釋！

1. 不要寫無用的注釋，應該用代碼來闡述意圖
2. 能用函數名說明目的的代碼，就不要加注釋
3. 對于一些比較容易造成誤解的代碼，注釋真的可以避免很多損失
4. 将對代碼的解釋文檔的連結放在代碼注釋裡
5. 對于一些看起來很明顯的傻B代碼，重構前先思考這個代碼真的是傻B而不是必須這麼寫嗎