Lec 3 代码审查

代码审查是一种对非原作者进行的、有条理且细致的源代码检查。类似对论文进行校对。代码审查的两个目的：1）改进代码——发现错误、预防潜在错误、检查代码清晰度、确保符合项目代码的编码风格；2）改进程序员——程序员相互学习的重要途径，在开源项目中，许多技术交流都发生在代码审查的过程中

为什么代码审查如此流行？在软件开发中，有许多流程方法（如 Agile、Scrum 等），但代码审查是少数被证明确实有效的实践之一。研究者 Hillel Wayne 总结了部分证据：代码审查可发现 70%-90% 的软件缺陷；Google 与 Microsoft 的工程师普遍认为代码审查非常有价值。

若你是 JavaScript 或 TypeScript 新手，可参考：

Idiomatic JavaScript（以可读性著称）
Google TypeScript Style Guide（针对 TypeScript 的规范）

带有异味的示例 #1

function dayOfYear(month: number, dayOfMonth: number, year: number): number {
    if (month === 2) {
        dayOfMonth += 31;
    } else if (month === 3) {
        dayOfMonth += 59;
    } else if (month === 4) {
        dayOfMonth += 90;
    } else if (month === 5) {
        dayOfMonth += 31 + 28 + 31 + 30;
    } else if (month === 6) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31;
    } else if (month === 7) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30;
    } else if (month === 8) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31;
    } else if (month === 9) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31;
    } else if (month === 10) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30;
    } else if (month === 11) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30 + 31;
    } else if (month === 12) {
        dayOfMonth += 31 + 28 + 31 + 30 + 31 + 30 + 31 + 31 + 30 + 31 + 31;
    }
    return dayOfMonth;
}

接下来的几节内容和练习会指出这段代码中具体的“坏味道”。

不要重复自己（DRY）

重复的代码会威胁程序的安全性。如果在两个地方存在完全相同或非常相似的代码块，那么最根本的风险是：这两个地方可能都有相同的 bug，而维护者修复了其中一个，却忘了另一个。

要像避免不看路就过马路那样去避免重复代码。复制粘贴虽然非常诱人，但每当你使用它时，都应该感到一丝危险的预感。复制的代码块越长，风险越高。

“不要重复自己”（Don’t Repeat Yourself，简称 DRY）已成为程序员的座右铭。

到目前为止，你应该已经学到了一些让代码“更干净”的方法，包括（但不限于）：

用命名良好的局部变量来替代重复出现的表达式；
用循环替代在同一位置多次出现的相似代码块；
用辅助函数替代在不同位置重复的代码块。

在需要的地方注释

优秀的软件开发者会在代码中合理地添加注释。好的注释应当让代码更容易理解、减少出错的风险（因为重要假设已被记录下来），并且让代码更容易修改。

一种重要的注释是规格说明（specification），通常出现在函数或类的上方，用于描述它们的行为。在 TypeScript 中，规范性注释通常写成文档注释（documentation comment），以 /** 开始，并使用 @ 语法（例如函数的 @param 和 @returns）。例如：

/**
 * 计算冰雹数列（Hailstone sequence）。
 * 参见 https://en.wikipedia.org/wiki/Collatz_conjecture#Statement_of_the_problem
 * @param n 数列的起始值；要求 n 为大于 0 的整数。
 * @returns 从 n 开始并以 1 结尾的冰雹数列。
 *          例如，hailstone(3) = [3, 10, 5, 16, 8, 4, 2, 1]。
 */
function hailstoneSequence(n: number): Array<number> {
    ...
}

另一种重要的注释是来源说明，即记录一段代码的出处或改编来源。这对职业开发者来说极为重要，也是 6.102 课程中关于合作政策的要求——当你改写或借鉴了网上的代码时，必须标明来源。例如：

// 反转数字字符串
// 参见 https://stackoverflow.com/questions/1611427/reversing-a-string-in-javascript
const digitsReversed = digits.split('').reverse().join('');

记录来源的原因之一是避免版权问题。Stack Overflow 上的小代码片段一般属于公共领域（public domain），但其他网站的代码可能受版权或特定开源协议约束，使用时更受限制。另一个原因是被借用的代码可能有 bug 或已经过时。例如上面那段反转字符串的代码，在原帖下的评论区中就有人指出，它无法正确处理包含非 ASCII（即非英文）字符的字符串。

当然，也有一些糟糕且多余的注释。比如只是机械地把代码翻译成英语，这对理解毫无帮助，因为你应假设读者至少懂 TypeScript：

while (n !== 1) { // 检查 n 是否等于 1   （不要写这种注释！）
   ++i; // 自增 i
   l.push(n); // 把 n 加入列表 l
}
// 但如果代码不直观、难以理解，就应该加注释：
const sum = n*(n+1)/2;  // 高斯求和公式：1 到 n 的和

// 这里我们使用 sin x ≈ x 的近似，这在 x 很小时成立
const moonDiameterInMeters = moonDistanceInMeters * apparentAngleInRadians;

另外，把代码分成“段落”（每组几行代码完成一个小目标）也很有帮助。每个段落前可以加上一句注释，说明其目的，就像写论文一样：

// 准备火箭
...
...

// 发射火箭
...
...

// 进入轨道
...
...

当你以这种方式思考代码时，可能会发现这些段落其实可以提炼成独立的辅助函数，而原本的注释可以转化为有意义的函数名

prepareRocket(...);
launchRocket(...);
enterOrbit(...);

一个清晰的函数名往往就能替代一条注释。dayOfYear 这段代码就需要加上一些注释——例如，你会在什么地方说明 month 是从 0 到 11，还是从 1 到 12？

快速失败

“快速失败”意味着代码应尽早暴露出自己的错误。问题被发现得越早（越接近其根本原因），就越容易定位和修复。静态检查比动态检查“失败得更早”，而动态检查又比“继续产生错误结果、并可能污染后续计算”更早暴露问题。

dayOfYear 函数并不是一个“快速失败”的例子——如果你传入的参数顺序错误，它不会报错，而是悄悄返回错误结果。实际上，以这种方式设计函数非常容易让非美国用户（习惯用日-月-年顺序）传错参数。因此，这个函数需要更多的检查机制——无论是静态检查还是动态检查。

避免魔法数字

有个计算机科学的笑话说：计算机科学家真正理解的数字只有 0、1，以及有时的 2。所有其他常量都被称为“魔法数字”，因为它们凭空出现、毫无解释。解释一个数字的方法之一是加注释，但更好的做法是定义一个具名常量（named constant），并给它一个清晰、准确的名字。

dayOfYear 中充斥着魔法数字，恰好说明了为什么应该避免这种情况：

数字的可读性不如名字。 在 dayOfYear 里，月份 2 到 12 如果改成 FEBRUARY、MARCH、…、DECEMBER，可读性会大大提高。
常量可能需要在未来修改。用具名常量而不是在多处硬编码数字，能让修改更方便。
常量之间可能存在依赖关系。在 dayOfYear 中，像 59 和 90 这样的数字尤其糟糕。它们没有任何注释或说明，实际上是程序员手动计算出来的——通过加总前几个月的天数。这种做法既不直观，也难以维护，还容易出错。不要手动算出数字再硬编码进去；应该用具名常量，并通过表达式清晰地显示它们之间的关系。

当你的代码中充满魔法数字时，这往往意味着你应该后退一步，把这些数字视为数据而非常量，并用数据结构存储它们。比如在 dayOfYear 中，月份天数（30、31、28 等）如果存入列表或映射（如 monthLength[month]），代码会更简洁、可读性更强，也更符合 DRY 原则。

每个变量只负责一个目的

在 dayOfYear 里，参数 dayOfMonth 被重复利用来计算完全不同的值（即函数返回值，而不再是“某月的第几天”）。

不要复用参数，也不要复用变量。编程中变量并不是稀缺资源，尽管有时候程序员会像吝啬内存一样吝啬变量。应该自由地引入新变量，取一个有意义的名字，并在不需要时停止使用它。如果一个变量在几行之后突然变了意义，读代码的人会非常困惑。这不仅是可理解性问题，也是安全性和可维护性问题。

尤其是函数参数，一般不应被修改（保持“原始输入”能被后续逻辑访问）。在 TypeScript 中，建议尽量使用 const 来声明局部变量，因为编译器会静态检查防止其被重新赋值。遗憾的是，TypeScript 目前还不支持将函数参数声明为 const。

带有异味的示例 #2

dayOfYear 函数中其实还有一个隐藏的 bug：它根本没处理闰年。假设我们写一个判断闰年的函数：

function leap(y:number):boolean {
    let tmp=y.toString();
    if(tmp[2]==='1'||tmp[2]==='3'||tmp[2]==='5'||tmp[2]==='7'||tmp[2]==='9') {
        if(tmp[3]==='2'||tmp[3]==='6') return true; /*R1*/
        else
            return false; /*R2*/
    }else{
        if(tmp[2]==='0' && tmp[3]==='0') {
            return false; /*R3*/
        }
        if(tmp[3]==='0'||tmp[3]==='4'||tmp[3]==='8') return true; /*R4*/
    }
    return false; /*R5*/
}

使用良好命名

好的函数名和变量名应当长而自描述。合理命名往往能替代注释，使代码更清晰。示例

// 不要写
const tmp = 86400;
// 应改为
const secondsPerDay = 86400;

像 tmp、temp、data 这样的变量名都很糟糕——它们是“极度懒惰的程序员”症状。因为每个变量都是“临时的”，每个变量也都是“数据”，这些名字毫无意义。请使用更长但更具描述性的名字，让代码能自我说明。

命名还应遵循语言的词法规范。

在 Python 和 TypeScript 中，类名通常以大写字母开头。
函数名和变量名以小写字母开头。
Python 使用 snake_case（如 starts_with），TypeScript 使用 camelCase（如 startsWith）。

类似 Java 的语言还区分全局常量和局部变量：

全局常量用 ALL_CAPS_WITH_UNDERSCORES。
局部变量和局部常量则用 camelCase（如 secondsPerDay）。

通常：

函数名是动词短语（如 getDate、isUpperCase），
变量和类名是名词短语。

命名要简洁，但不要使用缩写。例如： message 比 msg 更清晰，word 比 wd 好得多。要记住，你的同事（或同学）可能不是英语母语者，缩写对他们更难理解。

除非是约定俗成的情况，否则避免单字母变量名。 x、y 用于坐标，i、j 用于循环计数是可以的。但如果你的代码中到处是 e, f, g, h，只因为“字母还没用完”，那可读性就非常差。

给变量起好名字值得投入时间——因为代码被阅读的次数远多于被编写的次数。现代编辑器的自动补全功能让输入长名字几乎没有成本。

leap 函数的命名就不好：函数名本身和局部变量名 tmp 都模糊不清。你会如何改写它？例如：

function isLeapYear(year: number): boolean {
    const yearString = year.toString();
    ...
}

使用空白和标点帮助读者

保持一致的缩进。前面的 leap 示例在这方面做得不好；而 dayOfYear 示例则好得多。事实上，dayOfYear 甚至将所有数字排成整齐的列，这使得人类读者更容易比较和检查。那是对空白的非常好的使用方式。

在代码行中使用空格，使其更容易阅读。leap 示例中的很多行都挤在一起，非常难读：

if(tmp[2]==='1'||tmp[2]==='3'||tmp[2]==='5'||tmp[2]==='7'||tmp[2]==='9') {

在二元操作符（如 === 和 ||）的两边添加空格，使它们更醒目；并且在多行上保持对齐，使相似点和差异点一目了然：

if (   tmp[2] === '1'
    || tmp[2] === '3'
    || tmp[2] === '5'
    || tmp[2] === '7'
    || tmp[2] === '9' ) {

永远不要使用制表符（tab 字符）来缩进，只使用空格字符。意思是说你的编辑器在你按下 Tab 键时，不能真的插入一个 tab 字符。原因是不同的工具对 tab 字符的处理方式不同——有的把它扩展为 4 个空格，有的 2 个，有的 8 个。如果你在命令行运行 git diff，或者在别的编辑器中查看源代码，缩进可能会完全乱掉。

所以，只使用空格。当你按下 Tab 键时，让编辑器插入空格字符。这是 Visual Studio Code 的默认设置。

在使用 TypeScript（或 JavaScript、C、C++、Java）等类 C 语言编写代码时，在每条语句末尾都加上分号。虽然 TypeScript/JavaScript 有“自动分号插入”机制，会自动补上缺失的分号，但它的行为并不总是符合你的预期。要保持一致，为每条语句加上分号。

不要使用全局变量

一个全局变量是：

变量：即一个值可以被改变的名字
全局的：可以在程序的任何地方被访问和修改

Global Variables Are Bad 这篇文章总结了全局变量的各种风险。

在 TypeScript 中，全局变量是使用 let 或 var 在任何函数定义之外声明的变量。不过，如果它是用 const 声明的，并且它的类型是不可变的，那么这个名字就变成了全局常量。全局常量可以在任何地方被读取，但不能被重新赋值或修改，因此就不会有那些风险。全局常量是常见且有用的。

一般来说，应当把全局变量转换为参数和返回值，或者把它们放入某个对象中，通过调用对象的方法来访问。后面的内容中我们会看到很多这样做的技巧。

快照图的变量种类

在绘制程序的快照图（snapshot diagram）时，区分不同种类的变量非常重要：

函数内部的局部变量
对象实例中的实例变量
- 实例变量也有不同的称呼：在 TypeScript/JavaScript 中叫 property（属性），在 C++ 中叫 member variable（成员变量），在 Python 中叫 attribute（属性），在 Java 中叫 field（字段）。
函数或对象之外的全局变量

局部变量在函数被调用时产生，在函数返回时消失。如果同时有多个对同一函数的调用（例如递归），那么每次调用都会拥有自己独立的一份局部变量。

实例变量在用 new 创建对象时产生，在对象不再被访问并被垃圾回收（garbage-collected）后消失。每个对象实例都有自己独立的实例变量。

全局变量在声明语句执行时产生，并在整个程序生命周期中一直存在。

函数应该返回结果，而不是打印结果

countLongWords 这个函数没有做到“对变化有准备”。它通过 console.log() 将部分结果直接输出到控制台。这样一来，如果你想在别的场景中使用这个函数——例如把结果用于计算而不是展示给人看——就不得不改写代码。

一般来说，只有程序中最高层级的部分（例如主函数、用户界面逻辑）才应该与用户或控制台交互。较低层的部分应该通过参数接收输入，通过返回值输出结果。

唯一的例外是调试输出，当然可以打印到控制台。但这类输出不应属于程序设计的一部分，只是开发时用于调试的辅助手段。

避免写特殊情况代码

程序员经常会忍不住为“特殊情况”写特殊处理代码，比如参数为 0、空数组、空字符串等。countLongWords 就犯了这种错误，它专门为空数组单独写了一段逻辑：

if (words.length === 0) {
    console.log("0");
    return;
}
let n = 0;
longestWord = "";
for (let word of words) {
    if (word.length > LONG_WORD_LENGTH) ++n;
    if (word.length > longestWord.length) longestWord = word;
}
console.log(n);

开头的 if 判断其实是多余的。如果删除它，当 words 为空时，for 循环不会执行任何操作，打印出来的依然是 0。事实上，这种“特殊处理”反而可能引入潜在的错误——空数组的处理方式和没有长单词的非空数组不同。

要主动抵制写特殊情况代码的冲动。当你发现自己在为某个“特殊情况”写 if 时，先停下来想一想：

一般情况的代码是否已经能正确处理这个“特殊情况”？（很多时候其实可以！）
如果不能，能否稍微调整一般逻辑，让它自然地覆盖这个情况？

如果你还没写一般情况的代码就急着处理“简单情况”，那顺序就反了。应该先写好一般情况。写出通用的代码带来很多好处：

函数更短、更易理解，减少 bug 的藏身处
更少的假设，让代码更安全
更容易修改，因为逻辑集中、改动范围小

有些程序员为特殊情况单独写代码，是因为他们误以为这样能提升性能。例如写排序算法时，会想先判断列表长度是否为 0 或 1，从而“直接返回”，甚至对长度为 2 的情况单独处理。这些优化可能有意义——但只有当你有证据表明它真的能显著提升性能时才值得写。

如果排序函数几乎从不遇到这些情况，那这些“优化”只会增加复杂度、增加错误风险、却没有实际性能收益。先写一个干净、通用的算法；等确实需要优化时，再优化。

代码长度要适中

就像自然语言中过长的句子或段落难以阅读一样，源代码也如此。过长的代码通常是一种“坏味道”，提示你应该重新组织。

重构

在代码评审（code review）后，你经常需要修改和改进代码——即使它功能上已经正确。这种在不改变程序行为的前提下改进代码结构的过程就叫做重构（refactoring）。

重构的范围可以很小，也可以很大：

小重构：改进注释、重命名变量。
大重构：拆分函数、调整参数设计。

安全重构的步骤：

小步前进。不要一次性重写整个代码，而是分步进行。例如在拆分函数时，一次只提取一个辅助函数，每完成一步就检查是否正常运行。
临时注释旧代码。对于逻辑较复杂的改动，可以先把旧版本注释掉而不是删除，这样方便对照新旧逻辑。
利用静态检查。在 TypeScript 这类静态类型语言中，类型检查器能帮你发现改动影响的地方。例如增加了函数参数后，编译器会自动提示所有未更新的调用处。
每次改动后运行测试。每完成一个小步骤就运行测试，确保行为未被改变。改动越小，定位新引入的错误就越容易。
删除旧代码并提交版本控制。当一个步骤完成且验证正确后，删除旧代码并立即提交到版本控制系统。

总结

代码评审（code review）是一种通过人工检查来提升软件质量的常用方法。它能发现多种问题，这里总结了一些良好代码的通用实践：

不重复自己（DRY）
在需要时写注释
尽早失败（Fail Fast）
避免魔法数字
每个变量只承担一个用途
使用好的命名
用空白和标点帮助阅读
不使用全局变量
函数返回结果而非打印输出
避免特殊情况代码
保持合适的代码长度

这些主题与“好软件”的三大特性之间的关系如下：

安全（Safe from bugs）

代码评审帮助发现错误。
DRY 减少重复，从而只需在一个地方修复错误。
清晰的注释帮助他人避免误解。
“Fail Fast” 原则让错误尽早暴露。
避免全局变量能限制变量修改范围，使问题更易定位。

易于理解（Easy to understand）

评审让他人检验代码是否清晰。
适当的注释、良好的命名、干净的结构都能提升可读性。

易于修改（Ready for change）

有经验的评审者能预见未来变化并建议改进。
DRY 的代码修改成本更低。
返回值而非打印输出使代码更灵活，易于复用。

Lec 3 代码审查 ​

带有异味的示例 #1 ​

不要重复自己（DRY） ​

在需要的地方注释 ​

快速失败 ​

避免魔法数字 ​

每个变量只负责一个目的 ​

带有异味的示例 #2 ​

使用良好命名 ​

使用空白和标点帮助读者 ​

不要使用全局变量 ​

快照图的变量种类 ​

函数应该返回结果，而不是打印结果 ​

避免写特殊情况代码 ​

代码长度要适中 ​

重构 ​

总结 ​