代码审查是指阅读代码来检查源代码与编码标准的符合性以及代码质量的活动。现在,越来越多的团队倡导要进行代码审查活动,而本文作者通过一幅漫画,来诠释其对代码审查的理解,观点可能不符合大家的常规心理。以下为译文:
一天早晨,本文作者在Twitter上面看到这样一条推文:
Why code reviews are important? pic.twitter.com/8KyMo7Syis — Arun Gupta (@arungupta) August 28, 2016
该条推文下面放了Oppressive Silence网站上的一组漫画:
通过代码审查解决?
我想问的主要问题:
代码审查期间真的会发现问题或bug吗?
我的回答可能不会惊艳到你,下面我从四个方面来回答这个问题,至少这样的问题不应该发生在我(或者任何其它有经验的程序员)身上。
➤静态变量/全局状态(Static variables/global state)
第一个非常明显的错误是把变量isCrazyMurderingRobot设置成static并且是可变的。程序员在下面就很难推论和跟踪这个变量,任何程序员都可以给它赋值,这是完全不可接受的,没有任何理由把该变量设置成可变的静态类型。
重要提示:如果你要设置变量,特别是可变变量,那么,你应该把它的作用域范围最小化。
➤Final方法参数
如果你把全局状态的问题解决了,那么代码很有可能是这样的(转换成Java代码):
private void interact_with_humans(boolean isCrazyMurderingRobot) {
if(isCrazyMurderingRobot = true) {
kill(humans);
} else {
be_nice_to(humans);
}
}
任何时候,只要把一个参数传递到一个方法中,都应该把该参数设置成final。方法里的参数应该是不可变的。这可能意味着,方法应该拥有属于自己的(作用域)可变变量。
下面的代码变化:
private void interact_with_humans(final boolean isCrazyMurderingRobot) {
if(isCrazyMurderingRobot = true) {
// Compilation error: 'Cannot assign a value to final variable 'isCrazyMurderingRobot'
在告诉我错误之前,这段代码根本不会编译,好了,世界得救了!
➤静态代码分析
这样的bug在我的代码中根本不会发生,因为我们在整个项目中都启用了静态代码分析。例如FindBugs这款软件,它会立即揪出这种bug。
看一下FindBugs里的特定检查:
QBA: Method assigns boolean literal in boolean expression (QBA_QUESTIONABLE_BOOLEAN_ASSIGNMENT)
This method assigns a literal boolean value (true or false) to a boolean variable inside an if or while expression. Most probably this was supposed to be a boolean comparison using ==, not an assignment using =.
在现代编程中,静态代码分析工具是必不可少的。
➤Yoda conditions
另外一个好习惯使我的代码中永远都不会发生这样的bug,采用Yoda模式来编写If语句。原因是它会在cartoon里完全阻止这个bug。
在cartoon中,这个if语句会由“疯狂谋杀机器是真的吗?”转变为:“真的要疯狂谋杀机器人吗?”
如果这样做,代码应该是:
private void interact_with_humans(boolean isCrazyMurderingRobot) {
if(true = isCrazyMurderingRobot) {
// Compilation error: 'Variable expected'
结论/拯救世界
所以,你认为Arun的观点是正确的吗?代码审查会发现这个bug吗?对还是不对呢?
人类擅长推理、思考、富有创造性、并且发现错误,但语法编译对人类来说是一件可怕的事情。在代码审查期间,你可能仅仅发现一些拼写或基础错误(typo)但并未真正发现bug。难道第二双眼睛就可以发现了吗?