作者:
2014 年 5 月 31 日 (最后更新:2014 年 5 月 31 日)

最后一行效应

评分:4.3/5 (159 票)
*****

我研究了由于使用复制粘贴方法而导致的许多错误,并可以向您保证,程序员最常犯的错误是在同质代码块的最后一部分。我从未在编程书籍中见过这种现象的描述,所以我决定自己写一篇关于它的文章。我称之为“最后一行效应”。

简介

我叫 Andrey Karpov,我做一份不寻常的工作——我通过静态分析器分析各种应用程序的程序代码,并撰写我发现的错误和缺陷的描述。我这样做是出于务实和功利的原因,因为我的工作是我们公司宣传其工具 PVS-Studio 和 CppCat 的方式。这个模式很简单。我找到 Bug。然后我描述它们。文章吸引了我们潜在客户的注意力。盈利。但今天的文章不是关于分析器。

在进行各种项目的分析时,我会将我发现的 Bug 和相应的代码片段保存在一个特殊的数据库中。顺便说一下,任何有兴趣的人都可以看看这个数据库。我们将其转换为一系列 html 页面,并将其上传到我们网站的“检测到的错误”部分。

这个数据库确实是独一无二的!它目前包含 1500 个带有错误的 कोड 片段,并等待程序员学习它并揭示这些错误中的某些规律。这可以为未来的许多研究、手册和文章提供有用的基础。

我从来没有对收集到的材料进行过任何特别的调查。然而,一个模式非常清晰,以至于我决定深入研究一下。您知道,在我的文章中,我经常不得不写“注意最后一行”这句话。我想到一定有什么原因。

最后一行效应

在编写程序代码时,程序员经常需要编写一系列相似的结构。多次输入相同的代码既乏味又效率低下。这就是为什么他们使用复制粘贴方法:代码片段被复制并粘贴几次,然后进行进一步编辑。每个人都知道这种方法的坏处:您很容易忘记在粘贴的行中更改某些内容,从而产生错误。不幸的是,通常没有更好的替代方法。

现在我们来谈谈我发现的模式。我发现错误最常发生在最后一个粘贴的代码块中。

这是一个简单而短的例子

inline Vector3int32& operator+=(const Vector3int32& other) {
  x += other.x;
  y += other.y;
  z += other.y;
  return *this;
}

注意“z += other.y;”这一行。程序员忘记将 'y' 替换为 'z'。

您可能会认为这是一个人为的示例,但它实际上来自一个真实应用程序。在本文的后面,我将说服您这是一个非常频繁和常见的问题。这就是“最后一行效应”的样子。程序员最常在相似编辑序列的末尾犯错。

我曾经在哪里听说过登山者在攀登的最后几十米常常会摔下来。不是因为他们累了;他们只是对几乎到达顶峰感到过于高兴——他们期待胜利的甜蜜滋味,注意力变得不那么集中,然后犯下一些致命的错误。我想程序员也发生了一些类似的事情。

现在说点数字。

在研究了 Bug 数据库后,我挑出了 84 个我发现是通过复制粘贴方法编写的代码片段。其中,41 个片段在复制粘贴块的中间包含错误。例如

strncmp(argv[argidx], "CAT=", 4) &&
strncmp(argv[argidx], "DECOY=", 6) &&
strncmp(argv[argidx], "THREADS=", 6) &&
strncmp(argv[argidx], "MINPROB=", 8)) {

“THREADS=”字符串的长度是 8 个字符,而不是 6 个。

在其他 43 个案例中,错误发现在最后一个复制的代码块中。

好吧,43 这个数字看起来只比 41 稍微大一点。但请记住,同质块可能有很多,所以错误可能出现在第一个、第二个、第五个,甚至第十个块中。这样我们就得到了一个错误在块中相对平滑的分布,以及一个在结尾的尖锐峰值。

我平均将同质块的数量视为 5。

所以,看起来前 4 个块包含 41 个错误,分布在它们之中;这使得每个块大约有 10 个错误。

剩下 43 个错误留给第五个块!

所以我们得到的是这样的模式

在最后粘贴的代码块中犯错的概率是其他任何代码块的 4 倍。

我不会从中得出任何宏大的结论。这只是一个有趣的观察,了解它可能很有用——当您编写最后代码片段时,您会保持警惕。

例子

现在我只需要说服读者,这一切都不是我的奇思妙想,而是真实的趋势。为了证明我的话,我将向您展示一些例子。

我当然不会引用所有例子——只会引用最简单或最有代表性的。

Source Engine SDK

inline void Init( float ix=0, float iy=0,
                  float iz=0, float iw = 0 ) 
{
  SetX( ix );
  SetY( iy );
  SetZ( iz );
  SetZ( iw );
}

SetW() 函数应该在最后调用。

Chromium

if (access & FILE_WRITE_ATTRIBUTES)
  output.append(ASCIIToUTF16("\tFILE_WRITE_ATTRIBUTES\n"));
if (access & FILE_WRITE_DATA)
  output.append(ASCIIToUTF16("\tFILE_WRITE_DATA\n"));
if (access & FILE_WRITE_EA)
  output.append(ASCIIToUTF16("\tFILE_WRITE_EA\n"));
if (access & FILE_WRITE_EA)
  output.append(ASCIIToUTF16("\tFILE_WRITE_EA\n"));
break;

最后一个块和前一个块是相同的。

ReactOS

if (*ScanString == L'\"' ||
    *ScanString == L'^' ||
    *ScanString == L'\"')

Multi Theft Auto

class CWaterPolySAInterface
{
public:
    WORD m_wVertexIDs[3];
};
CWaterPoly* CWaterManagerSA::CreateQuad (....)
{
  ....
  pInterface->m_wVertexIDs [ 0 ] = pV1->GetID ();
  pInterface->m_wVertexIDs [ 1 ] = pV2->GetID ();
  pInterface->m_wVertexIDs [ 2 ] = pV3->GetID ();
  pInterface->m_wVertexIDs [ 3 ] = pV4->GetID ();
  ....
}

最后一行是机械粘贴的,是多余的。数组只有 3 个元素。

Source Engine SDK

intens.x=OrSIMD(AndSIMD(BackgroundColor.x,no_hit_mask),
                AndNotSIMD(no_hit_mask,intens.x));
intens.y=OrSIMD(AndSIMD(BackgroundColor.y,no_hit_mask),
                AndNotSIMD(no_hit_mask,intens.y));
intens.z=OrSIMD(AndSIMD(BackgroundColor.y,no_hit_mask),
                AndNotSIMD(no_hit_mask,intens.z));

程序员忘记在最后一个块中将“BackgroundColor.y”替换为“BackgroundColor.z”。

Trans-Proteomic Pipeline

void setPepMaxProb(....)
{  
  ....
  double max4 = 0.0;
  double max5 = 0.0;
  double max6 = 0.0;
  double max7 = 0.0;
  ....
  if ( pep3 ) { ... if ( use_joint_probs && prob > max3 ) ... }
  ....
  if ( pep4 ) { ... if ( use_joint_probs && prob > max4 ) ... }
  ....
  if ( pep5 ) { ... if ( use_joint_probs && prob > max5 ) ... }
  ....
  if ( pep6 ) { ... if ( use_joint_probs && prob > max6 ) ... }
  ....
  if ( pep7 ) { ... if ( use_joint_probs && prob > max6 ) ... }
  ....
}

程序员忘记将最后一个条件中的“prob > max6”替换为“prob > max7”。

SeqAn

inline typename Value<Pipe>::Type const & operator*() {
  tmp.i1 = *in.in1;
  tmp.i2 = *in.in2;
  tmp.i3 = *in.in2;
  return tmp;
}

SlimDX

for( int i = 0; i < 2; i++ )
{
  sliders[i] = joystate.rglSlider[i];
  asliders[i] = joystate.rglASlider[i];
  vsliders[i] = joystate.rglVSlider[i];
  fsliders[i] = joystate.rglVSlider[i];
}

rglFSlider 数组应该在最后一行中使用。

Qt

if (repetition == QStringLiteral("repeat") ||
    repetition.isEmpty()) {
  pattern->patternRepeatX = true;
  pattern->patternRepeatY = true;
} else if (repetition == QStringLiteral("repeat-x")) {
  pattern->patternRepeatX = true;
} else if (repetition == QStringLiteral("repeat-y")) {
  pattern->patternRepeatY = true;
} else if (repetition == QStringLiteral("no-repeat")) {
  pattern->patternRepeatY = false;
  pattern->patternRepeatY = false;
} else {
  //TODO: exception: SYNTAX_ERR
}

'patternRepeatX' 在最后一个块中丢失。正确的代码如下

pattern->patternRepeatX = false;
pattern->patternRepeatY = false;

ReactOS

const int istride = sizeof(tmp[0]) / sizeof(tmp[0][0][0]);
const int jstride = sizeof(tmp[0][0]) / sizeof(tmp[0][0][0]);
const int mistride = sizeof(mag[0]) / sizeof(mag[0][0]);
const int mjstride = sizeof(mag[0][0]) / sizeof(mag[0][0]);

“mjstride”变量将始终等于一。最后一行应该这样写

const int mjstride = sizeof(mag[0][0]) / sizeof(mag[0][0][0]);

Mozilla Firefox

if (protocol.EqualsIgnoreCase("http") ||
    protocol.EqualsIgnoreCase("https") ||
    protocol.EqualsIgnoreCase("news") ||
    protocol.EqualsIgnoreCase("ftp") ||          <<<---
    protocol.EqualsIgnoreCase("file") ||
    protocol.EqualsIgnoreCase("javascript") ||
    protocol.EqualsIgnoreCase("ftp")) {          <<<---

结尾有一个可疑的字符串“ftp”——它已经被比较过了。

Quake-III-Arena

if (fabs(dir[0]) > test->radius ||
    fabs(dir[1]) > test->radius ||
    fabs(dir[1]) > test->radius)

dir[2] 单元格的值未经验证。

Clang

return (ContainerBegLine <= ContaineeBegLine &&
        ContainerEndLine >= ContaineeEndLine &&
        (ContainerBegLine != ContaineeBegLine ||
         SM.getExpansionColumnNumber(ContainerRBeg) <=
         SM.getExpansionColumnNumber(ContaineeRBeg)) &&
        (ContainerEndLine != ContaineeEndLine ||
         SM.getExpansionColumnNumber(ContainerREnd) >=
         SM.getExpansionColumnNumber(ContainerREnd)));

在块的末尾,“SM.getExpansionColumnNumber(ContainerREnd)”表达式与自身进行比较。

MongoDB

bool operator==(const MemberCfg& r) const {
  ....
  return _id==r._id && votes == r.votes &&
         h == r.h && priority == r.priority &&
         arbiterOnly == r.arbiterOnly &&
         slaveDelay == r.slaveDelay &&
         hidden == r.hidden &&
         buildIndexes == buildIndexes;
}

程序员在最后一行忘记了“r.”。

Unreal Engine 4

static bool PositionIsInside(....)
{
  return
    Position.X >= Control.Center.X - BoxSize.X * 0.5f &&
    Position.X <= Control.Center.X + BoxSize.X * 0.5f &&
    Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f &&
    Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f;
}

程序员忘记在最后一行进行 2 项编辑。首先,“>=”应替换为“<=;”其次,减号应替换为加号。

Qt

qreal x = ctx->callData->args[0].toNumber();
qreal y = ctx->callData->args[1].toNumber();
qreal w = ctx->callData->args[2].toNumber();
qreal h = ctx->callData->args[3].toNumber();
if (!qIsFinite(x) || !qIsFinite(y) ||
    !qIsFinite(w) || !qIsFinite(w))

在对函数 qIsFinite 的最后一次调用中,变量“h”应作为参数使用。

OpenSSL

if (!strncmp(vstart, "ASCII", 5))
  arg->format = ASN1_GEN_FORMAT_ASCII;
else if (!strncmp(vstart, "UTF8", 4))
  arg->format = ASN1_GEN_FORMAT_UTF8;
else if (!strncmp(vstart, "HEX", 3))
  arg->format = ASN1_GEN_FORMAT_HEX;
else if (!strncmp(vstart, "BITLIST", 3))
  arg->format = ASN1_GEN_FORMAT_BITLIST;

“BITLIST”字符串的长度是 7 个字符,而不是 3 个。

我们就到这里吧。我希望我展示的例子已经足够多了。

结论

从这篇文章中,您了解到使用复制粘贴方法,在最后粘贴的代码块中犯错的概率比在其他任何代码片段中犯错的概率高 4 倍。

这与人的心理特点有关,而不是专业技能。我在本文中向您展示了,即使是像 Clang 或 Qt 这样的项目中的技术娴熟的开发人员,也倾向于犯此类错误。

我希望我的观察对程序员有用,并可能促使他们研究我们的 Bug 数据库。我相信这将有助于揭示许多错误规律,并制定新的程序员建议。