整洁代码不代表不需要注释

精排版👈请戳这里

这篇文章的由来呢，源于最近在工作的过程中发现的一个非常有意思的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而不是必须这么写吗