Skip to content
This repository has been archived by the owner on May 9, 2023. It is now read-only.

Latest commit

 

History

History
440 lines (285 loc) · 27 KB

9.md

File metadata and controls

440 lines (285 loc) · 27 KB

第9节 代码审查

❤️💕💕包含软件工程、算法与架构：设计模式、软件架构、协同开发、质量保障。更多关注我的博客：Myblog:http://nsddd.top

[TOC]

代码审查

代码审查由非原始代码作者的人对源代码进行仔细、系统的研究。这类似于校对论文。

代码审查实际上有两个目的：

  • 改进代码。 查找错误，预测可能的错误，检查代码的清晰度，并检查与项目风格标准的一致性。
  • 改进程序员。 代码审查是程序员相互学习和教授新语言特性、项目设计或其编码标准的变化以及新技术的重要方式。特别是在开源项目中，很多对话都发生在代码审查的上下文中。

无论是Go语言或者是Java 、 C、python，都是有自己的编程规范，甚至于每个企业，组织也都是有自己的规范，对于J~ava来说，项目选择maven或许有着更好的移植性。

代码审查在 Apache 和Mozilla等开源项目中得到广泛应用。代码审查在行业中也得到了广泛的应用。例如，在Google 的代码审查流程中，您不能将任何代码推送到主存储库中，直到另一位工程师在代码审查中签字同意。

谷歌的代码审查 — 代码审查应该关注：

  • 设计：代码是否经过精心设计并适合您的系统？
  • 功能：代码的行为是否符合作者的预期？代码的行为方式是否对用户有利？
  • 复杂性：代码可以更简单吗？将来遇到此代码时，其他开发人员是否能够轻松理解和使用此代码？
  • 测试：代码是否具有正确且设计良好的自动化测试？
  • 命名：开发人员是否为变量、类、方法等选择了清晰的名称？
  • 评论：评论（注释）是否清晰有用？
  • 风格：代码是否遵循我们的 风格指南？
  • 文档：开发者是否也更新了相关文档？

谷歌的风格语言包含C++ 样式指南、C# 样式指南、 Swift 样式指南、Objective-C 样式指南、 Java 样式指南、Python 样式指南、R 样式指南、 Shell 样式指南、HTML/CSS 样式指南、 JavaScript 样式指南、TypeScript 样式指南、AngularJS 样式指南、 Common Lisp 样式指南和Vimscript 样式指南。

更新文档很重要，对于一个开源项目来说，如果文档更新的不及时，可能引发的问题很严重，严重影响使用者和其他开发者的效率和体验。

对于Go语言来说，谷歌有专门的要求可以参考Go样式指南

代码审查可以发现 70-90% 的软件缺陷，谷歌和微软的绝大多数软件工程师都认为它很有价值，值得去做。

风格标准

大多数公司和大型项目都有编码风格标准。这些可以变得非常详细，甚至可以指定空格（缩进的深度）以及大括号和圆括号的位置。这类问题通常会导致圣战，因为它们最终成为品味和风格的问题。

在 6.031 中，我们没有正式的样式指南。如果您是 Java 新手并希望获得推荐，Google Java Style被广泛使用并因其可读性而吸引人，尤其是以下规则：

if (isOdd(n)) {
    n = 3*n + 1;
}
  • 关键字 ( if) 后有空格，但函数名 ( isOdd)后没有空格
  • {在一行的末尾，单独}在一行上
  • {…}围绕所有块，甚至是空块或单语句块

Eclipse 有一个自动格式化程序（Source → Format），其默认规则类似于 Google 风格。

但我们不会规定在本课程中将花括号放在何处。这是每个程序员都应该做出的个人决定。然而，自洽是很重要的，遵循你正在从事的项目的约定也很重要。如果你是一个程序员，会重新格式化你接触的每个模块以匹配你的个人风格，你的队友会恨你，这是正确的。成为团队合作者。

但是有一些规则是相当明智的，它们针对我们的三大属性，以比放置花括号更强大的方式。本阅读材料的其余部分将讨论其中的一些规则，至少是与本课程目前相关的规则，我们主要讨论的是编写基本的 Java。这些是您在审查其他学生的代码以及查看自己的代码以进行改进时应该开始寻找的一些东西。但是，不要将其视为代码样式指南的详尽列表。在整个学期中，我们将讨论更多的东西——规范、具有表示不变量的抽象数据类型、并发性和线程安全——然后它们将成为代码审查的素材。

臭名昭著的例子#1

程序员经常将糟糕的代码描述为具有需要去除的“难闻气味”。“代码卫生”是另一个词。让我们从一些臭代码开始。

public static int dayOfYear(int month, int dayOfMonth, int year) {
    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;
}

（干）避免重复

重复的代码是安全风险。如果您在两个地方有相同或非常相似的代码，那么根本风险是两个副本中都存在错误，并且一些维护人员在一个地方修复了该错误，但在另一个地方没有修复。

避免重复，就像避免不看就过马路一样。复制粘贴是一种极具诱惑力的编程工具，每次使用它时，你都应该感到一种危险的感觉。您复制的块越长，风险就越大。

不要重复自己，或者干燥简而言之，已经成为程序员的口头禅。

该dayOfYear示例充满了相同的代码。你会怎么把它弄干？

需要的地方注释

优秀的软件开发人员在他们的代码中编写注释，并且明智地这样做。好的注释应该使代码更容易理解，更安全地避免错误（因为重要的假设已经记录在案），并且更易于更改。

一种重要的注释是规范，它出现在方法或类之上，并记录方法或类的行为。在 Java 中，这通常被编写为 Javadoc 注释，这意味着它以 - 语法开头/**并包含@- 语法、like@param和@returnfor 方法。这是规范的示例：

/**
 * Compute the hailstone sequence.
 * See http://en.wikipedia.org/wiki/Collatz_conjecture#Statement_of_the_problem
 * @param n starting number of sequence; requires n > 0.
 * @return the hailstone sequence starting at n and ending with 1.
 *         For example, hailstone(3)=[3,10,5,16,8,4,2,1].
 */
public static List<Integer> hailstoneSequence(int n) {
    ...
}

规范记录假设。我们已经多次提到规格，在以后的阅读中会有更多关于它们的内容。

另一个重要的评论是指定从其他地方复制或改编的一段代码的出处或来源。这对于实践的软件开发人员来说至关重要，并且当您改编您在 Web 上找到的代码时，这是6.031 协作策略所要求的。这是一个例子：

// read a web page into a string
// see http://stackoverflow.com/questions/4328711/read-url-to-string-in-few-lines-of-java-code
String mitHomepage = new Scanner(new URL("http://www.mit.edu").openStream(), "UTF-8").useDelimiter("\\A").next();

记录来源的原因之一是避免侵犯版权。Stack Overflow 上的小片段代码通常位于公共领域，但从其他来源复制的代码可能是专有的或被其他类型的开源许可证所涵盖，这些许可证更具限制性。记录来源的另一个原因是代码可能会过时。自从首次回答以来，该代码所来自的Stack Overflow 答案在这些年里发生了显着变化。

有些评论是不好的和不必要的。例如，将代码直接音译成英文对提高理解没有任何帮助，因为您应该假设您的读者至少了解 Java：

while (n != 1) { // test whether n is 1   (don't write comments like this!)
   ++i; // increment i
   l.add(n); // add n to l
}

但是晦涩的代码应该得到注释：

int sum = n*(n+1)/2;  // Gauss's formula for the sum of 1...n

// here we're using the sin x ~= x approximation, which works for very small x
double moonDiameterInMeters = moonDistanceInMeters * apparentAngleInRadians;

dayOfYear代码需要一些注释——你会把它们放在哪里？例如，您将在哪里记录month运行是从 0 到 11 还是从 1 到 12？

编译错误

编译错误意味着代码应该尽早揭示它的错误。越早发现问题（越接近其原因），就越容易发现和修复。正如我们在第一篇文章中看到的那样，静态检查比动态检查失败得更快，而动态检查比产生可能破坏后续计算的错误答案更快地失败。

该dayOfYear函数不会很快失败——如果你以错误的顺序传递参数，它会悄悄地返回错误的答案。事实上，这种方式dayOfYear是设计好的，非美国人很有可能会以错误的顺序传递论点！它需要更多检查——静态检查或动态检查。

避免幻数

有一个计算机科学笑话，计算机科学家理解的唯一数字是 0、1，有时是 2。

所有其他常量都称为魔法因为它们似乎是凭空出现的，没有任何解释。

解释数字的一种方法是使用注释，但更好的方法是将数字声明为具有良好、清晰名称的命名常量。

dayOfYear充满了神奇的数字，这说明了应该避免使用它们的一些原因：

  • 数字不如名称可读。 在dayOfYear, 月份值 2, ..., 12 将更易读，因为FEBRUARY, ..., DECEMBER。
  • 将来可能需要更改常量。 使用命名常量，而不是在各个地方重复文字数字，更易于更改。
  • 常量可能依赖于其他常量。 在dayOfYear中，神秘的数字 59 和 90 是特别有害的例子。它们不仅没有注释和文档，而且实际上是程序员手工计算的结果——对某些月份的长度求和。这不太容易理解，不太容易改变，而且绝对不那么安全。不要对手动计算的数字进行硬编码。使用一个命名常量，它可以根据其他命名常量直观地计算关系。

那些看起来恒定且永恒的常数呢，比如 π，或者三角形的总角度，或者万有引力常数G？是否有必要为不依赖于其他常数的数学和物理学的基本常数命名？首先，数字本身可能表达起来很复杂，因此多次重复它并不安全。很容易判断哪个是正确的：3.14159265358979323846还是3.1415926538979323846？其次，即使是这些数字也编码了未来可能会改变的设计决策，例如精度位数或测量单位。当这些设计决策发生变化时，命名常量更容易发生变化。

如果您的代码中有大量幻数，这可能表明您需要退后一步，将这些数字视为数据而不是命名常量，并将它们放入允许更简单代码的数据结构中。在dayOfYear中，如果将月份的长度（30、31、28 等）存储在数组、列表或映射等数据结构中，例如MONTH_LENGTH[month].

每个变量的一个目的

在dayOfYear示例中，该参数dayOfMonth被重用于计算一个非常不同的值——函数的返回值，它不是月份中的哪一天。

不要重用参数，也不要重用变量。变量不是编程中的稀缺资源。自由地介绍它们，给它们起个好听的名字，当你不再需要它们时就停止使用它们。如果一个曾经意味着一件事的变量突然在几行之后开始意味着不同的东西，你会让你的读者感到困惑。

这不仅是一个易于理解的问题，而且还是一个防止错误和准备更改的问题。

尤其是方法参数，通常应保持不变。（这对于准备好更改很重要——将来，方法的其他部分可能想知道方法的原始参数是什么，所以在计算时不应该把它们吹走。）使用final方法参数和尽可能多的其他变量是个好主意。final关键字表示永远不应该重新分配变量，Java 编译器会静态检查它。例如：

public static int dayOfYear(final int month, final int dayOfMonth, final int year) {
    ...
}

臭名昭著的例子#2

中存在一个潜在的错误dayOfYear。它根本没有处理闰年。作为解决这个问题的一部分，假设我们编写了一个闰年方法。

public static boolean leap(int y) {
    String tmp = String.valueOf(y);
    if (tmp.charAt(2) == '1' || tmp.charAt(2) == '3' || tmp.charAt(2) == 5 || tmp.charAt(2) == '7' || tmp.charAt(2) == '9') {
        if (tmp.charAt(3)=='2'||tmp.charAt(3)=='6') return true; /*R1*/
        else
            return false; /*R2*/
    }else{
        if (tmp.charAt(2) == '0' && tmp.charAt(3) == '0') {
            return false; /*R3*/
        }
        if (tmp.charAt(3)=='0'||tmp.charAt(3)=='4'||tmp.charAt(3)=='8')return true; /*R4*/
    }
    return false; /*R5*/
}

这段代码中隐藏了哪些错误？我们已经讨论过哪些风格问题？

使用好名字

好的方法和变量名称很长且具有自我描述性。通常可以通过使代码本身更具可读性来完全避免注释，并使用更好的名称来描述方法和变量。

例如，您可以重写

int tmp = 86400;  // tmp is the number of seconds in a day (don't do this!)

作为：

final int secondsPerDay = 86400;

一般来说，像tmp,temp和之类的变量名data很糟糕，是程序员极度懒惰的症状。每个局部变量都是临时的，每个变量都是数据，所以这些名称通常没有意义。最好使用更长、更具描述性的名称，这样您的代码就可以自己清楚地阅读。

遵循语言的词汇命名约定。在 Python 和 Java 中，类通常以大写字母开头，变量名和方法名以小写字母开头。但是这两种语言在如何将多词短语呈现为方法或变量名称方面有所不同。Python使用snake_case（下划线分隔短语的单词），而Java使用camelCase（在第一个单词之后大写每个单词，如in startsWithor getFirstName）。

Java 还使用大写来区分全局常量 ( public static final) 与变量和局部常量。 ALL_CAPS_WITH_UNDERSCORES用于static final常量。但是在方法中声明的局部变量，包括secondsPerDay上面的局部常量，使用 camelCaseNames。

在任何语言中，方法名通常是动词短语，例如getDateor isUpperCase，而变量和类名通常是名词短语。选择简短的单词，简洁，但避免缩写。例如，message比 更清晰msg，并且word比 好得多wd。请记住，您在课堂上和现实世界中的许多队友都不会以英语为母语，而且对于非母语人士来说，缩写可能会更加困难。

完全避免使用单字符变量名，除非它们易于按照惯例理解。例如，对于笛卡尔坐标有意义，并且x作为循环中的整数变量。但是，如果您的代码中充满了 、 、 和 等变量，因为您只是从字母表中挑选它们，那么它将非常难以阅读。y``i``j``for``e``f``g``h

Effectively Naming Software Thingies对命名有一些很好的建议。强烈推荐Robert Martin 的Clean Code （第 2 章）。

该leap方法有错误的名称：方法名称本身和局部变量名称。你会怎么称呼他们？

使用空格来帮助读者

使用一致的缩进。这个leap例子在这方面很糟糕。这个dayOfYear例子要好得多。事实上，dayOfYear将所有数字很好地排列成列，使人类读者易于比较和检查。这是对空白的极大使用。

在代码行中放置空格以使其易于阅读。跳跃的例子有一些被打包在一起的行——放在一些空格里。

切勿使用制表符进行缩进，只能使用空格字符。请注意，我们说的是字符，而不是键。我们并不是说您永远不应该按 Tab 键，只是说您的编辑器永远不应该将制表符放入源文件以响应您按 Tab 键。这条规则的原因是不同的工具对待制表符的方式不同——有时将它们扩展为 4 个空格，有时扩展为 2 个空格，有时扩展为 8 个。如果你在命令行上运行“git diff”，或者如果你在一个不同的编辑器，那么缩进可能完全搞砸了。只需使用空格。始终将编程编辑器设置为在按 Tab 键时插入空格字符。6.031 Eclipse 安装已经以这种方式设置。

臭名昭著的例子#3

这是第三个臭代码示例，它将说明本文的其余要点。

public static int LONG_WORD_LENGTH = 5;
public static String longestWord;

public static void countLongWords(String text) {
    String[] words = text.split(' ');
    if (words.length == 0) {
        System.out.println("0");
        return;
    }
    int n = 0;
    longestWord = "";
    for (String word: words) {
        if (word.length() > LONG_WORD_LENGTH) ++n;
        if (word.length() > longestWord.length()) longestWord = word;
    }
    System.out.println(n);
}

不要使用全局变量

避免全局变量。让我们分解一下我们的意思全局变量. 一个全局变量是：

  • 一个变量，一个可以改变其值的名字
  • 这是全局的，可从程序中的任何位置访问和更改的。

Global Variables Are Bad 很好地列出了全局变量的危险。

在 Java 中，声明了一个全局变量public static。public修饰符使它可以在任何地方访问，并且意味着static变量只有一个实例。

但是，如果有一个额外的修饰符 ，并且如果变量的类型是不可变的，那么名称就变成了public static **final**全局常数. 全局常量可以在程序的任何地方读取，但永远不会重新分配或变异，因此风险消失了。全局常量是常见且有用的。

通常，将全局变量转换为参数和返回值，或者将它们放在您正在调用方法的对象中。我们将在未来的阅读中看到许多这样做的技术。

快照图中的变量种类

当我们绘制快照图时，区分不同类型的变量很重要：

  • 一个局部变量在方法内部

  • 一个

    实例变量

    在对象的实例内

    • 实例变量也可以称为场地（特别是在 Java 中），一个财产（TypeScript/JavaScript），一个成员变量（C++），或属性（Python）。

  • 一个静态变量与一个类相关联

局部变量在调用方法时出现，然后在方法返回时消失。如果对同一方法的多次调用正在进行（例如由于递归），那么每个调用都将有自己独立的局部变量。

实例变量在使用 创建对象时存在new，然后在对象不再可访问并被垃圾回收时消失。每个对象实例都有自己的实例变量。

静态变量在程序启动时（或者更准确地说是在首次加载包含该变量的类时，因为这可能会延迟）就存在，并且在程序的剩余生命周期中都存在。

这是一段使用所有三种变量的简单代码：

class Payment {
    public double value;
    public static double taxRate = 0.05;
    public static void main(String[] args) {
        Payment p = new Payment();
        p.value = 100;
        taxRate = 0.05;
        System.out.println(p.value * (1 + taxRate));
    }
}

在接下来的两个练习中，您将构建一个显示该程序状态的快照图。

绘制堆

绘制堆栈

方法应该返回结果，而不是打印出来

countLongWords还没有准备好改变。它将一些结果发送到控制台，System.out. 这意味着，如果你想在另一个环境中使用它——在这个环境中需要这个数字用于其他目的，比如计算而不是人眼——就必须重写它。

通常，只有程序的最高级别部分应该与人类用户或控制台交互。较低级别的部分应将其输入作为参数并将其输出作为结果返回。这里唯一的例外是调试输出，当然可以打印到控制台。但是这种输出不应该是您设计的一部分，而只是您调试设计的一部分。

避免特殊情况代码

程序员经常想编写特殊代码来处理看似特殊的情况——例如，参数为 0、空列表或空字符串。该示例通过专门countLongWords处理一个空列表落入了这个陷阱：words

    if (words.size() == 0) {
        System.out.println(0);
        return;
    }
    int n = 0;
    longestWord = "";
    for (String word: words) {
        if (word.length() > LONG_WORD_LENGTH) ++n;
        if (word.length() > longestWord.length()) longestWord = word;
    }
    System.out.println(n);
}

起始if语句是不必要的。如果它被省略了，并且words列表是空的，那么for循环就什么也不做，并且0无论如何都会被打印出来。事实上，单独处理这种特殊情况导致了一个可能的错误——空列表的处理方式与碰巧没有长词的非空列表的处理方式不同。

积极抵制特殊情况分开处理的诱惑。 如果您发现自己正在if为特殊情况编写语句，请停止您正在做的事情，而是更加仔细地考虑一般情况代码，或者确认它实际上已经可以处理您担心的特殊情况（即通常是真的！），或者多花点精力让它处理特殊情况。如果您甚至还没有编写通用案例代码，而只是尝试先处理简单案例，那么您的操作顺序错误。先处理一般情况。

编写更广泛的通用代码是有回报的。它导致方法更短，更容易理解，并且隐藏错误的地方更少。它可能对错误更安全，因为它对正在使用的值做出的假设更少。而且它更易于更改，因为当方法的行为发生更改时，需要更新的地方更少。

一些程序员认为单独处理特殊情况是合理的，他们相信通过立即返回特殊情况的硬编码答案来提高方法的整体性能。例如，在编写排序算法时，在方法的最开始检查列表的大小是 0 还是 1 可能很诱人，因为您可以立即返回而根本不需要排序。或者如果大小为 2，只需进行比较和可能的交换。这些优化可能确实有意义——但直到你有证据表明它们实际上对程序的速度很重要！如果在这些特殊情况下几乎从不调用 sort 方法，那么为它们添加代码只会增加复杂性、开销和 bug 的隐藏位置，而不会对性能进行任何实际改进。写一个干净，简单，只有当它真的有帮助时。

隐藏在特殊情况下的错误

概括

代码审查是一种广泛使用的技术，用于通过人工检查来提高软件质量。代码审查可以检测代码中的多种问题，但作为初学者，这篇阅读文章谈到了好代码的这些一般原则：

  • 不要重复自己（干）
  • 需要的地方评论
  • 快速失败
  • 避免幻数
  • 每个变量的一个目的
  • 使用好名字
  • 使用空格来帮助读者
  • 不要使用全局变量
  • 方法应该返回结果，而不是打印出来
  • 避免特殊情况代码

今天阅读的主题与我们优秀软件的三个关键属性有关，如下所示：

  • 远离错误。 通常，代码审查使用人工审查员来查找错误。DRY 代码让您只在一个地方修复错误，而不必担心它已经传播到其他地方。用清晰的注释记录你的假设可以降低其他程序员引入错误的可能性。Fail Fast 原则尽可能早地检测到错误。避免使用全局变量可以更容易地定位与变量值相关的错误，因为非全局变量只能在代码中的有限位置进行更改。
  • 容易理解。 代码审查确实是找到晦涩难懂或令人困惑的代码的唯一方法，因为其他人正在阅读它并试图理解它。使用明智的注释、避免使用幻数、为每个变量保留一个目的、使用好名称以及使用好空格都可以提高代码的可理解性。
  • 准备好改变。 代码审查由经验丰富的软件开发人员完成，他们可以预测可能发生的变化并提出防范方法。DRY 代码更易于更改，因为只需在一个地方进行更改。返回结果而不是打印结果可以更容易地使代码适应新的目的。

代码审查并不是唯一具有强有力支持证据的软件开发实践。另一个是：睡眠。

询问有关阅读的问题

对阅读或阅读练习有疑问？在 Piazza 上发布您的问题，并包含一个链接，该链接可直接跳转到您所询问的阅读部分或阅读练习。您可以通过将鼠标光标移动到左边距来获取该链接，直到#您要引用的内容旁边出现一个 octothorpe 链接。单击它#，您的浏览器的地址栏现在有了您想要的链接。

END 链接