編程中的“末行效應”

jopen 10年前發布 | 14K 次閱讀 編程

我研究過數百個因“拷貝-粘貼”導致的錯誤。可以肯定的是,程序員常常會在一大段代碼的最后一段里犯錯。好像還沒有任何編程書討論過這種現象,因此我決定自己寫點什么。我稱之為“末行效應”。

拷貝粘貼

我叫Andrey Karpov,我的工作有點不尋常:我借助靜態分析工具研究各種應用程序代碼,并描述從中找到的錯誤或者缺陷。我這么做既有實際效益也因為工作需要。使用的方法正是基于我們公司所推廣的PVS-Studio和CppCat工具的原理。套路很簡單:找bug,然后寫文章分析bug,文章吸引到潛在用戶的注意,接著就是收益。但今天這篇文章不是介紹這些工具的。

在分析各種軟件項目的過程中,我把找到的bug以及相關代碼存入一個特殊的數據庫。順便說一下,有興趣的話各位可以看一看這個數據庫。我們把它轉換成網頁格式并上傳到了公司網站的“Detected errors”欄下。

這個數據庫獨一無二!目前它收錄了1500塊問題代碼片,正等著程序員們去研究,從中總結出特定規律。為將來的研究,手冊和文章奠定一個基礎。

我還沒認真地分析過目前搜集到的材料。但是過程中我發現有一個明顯的模式反復出現,決定深入研究一下。你大概看到了,文中我反復使用短語“注意最后一行”。在我看來,這一定有某種規律。

末行效應

編程的時候,程序員常常需要寫一系列相似的結構。逐行敲鍵盤輸入無聊且低效。這就是為什么他們會使用奧義-“拷貝-粘貼”大法:一段代碼被拷貝粘貼幾次,然后修改。誰都知道這樣做的壞處:你很容易在粘貼后忘記修改某些內容最后滋生出問題。不幸的是,常常找不到比這更好的方法。

那么我發現了什么規律呢?我發現錯誤常常發生在最后的一塊粘貼代碼里。

下面是一個簡短的例子:

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

注意這一行:”z += other.y;”。程序員忘記把‘y’替換成‘z’了。

也許你以為這是個假設的例子,然后它其實來自一個真實的應用程序。接下來,我會讓你相信這是高頻常見的一種錯誤。程序員們經常在一連串相似操作的結尾犯這種錯誤。

我聽說攀巖者常常在最后的幾十米中滑落下來。并不是因為他們累了,而正是由于他們對即將到達的終點過于興奮,他們想象著成功后的喜悅,變得疏忽大意,最后失足。我猜想程序員們也是這樣的。

接下來看一組數據。

研究了數據庫后,我分離出了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個錯誤!

下面的分布概圖凸顯出這個現象:

圖1. 五塊類似代碼段中的錯誤分布概圖

因此我們可以總結出一個規律:

在最末的粘貼代碼塊中出錯的概率是其他代碼塊的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’永遠等于1。最后一行應該是:

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;
}

最后一行中,程序員忘記了兩個地方。首先,“>=”應改為“<=”,其次,減號應改為加號。

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))

最后一個qlsFinite中,傳入參數應該是‘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數據庫。相信如此有助于在這些錯誤中發現新的規律并總結出新的編程建議。

本文轉載自: 外刊IT評論 http://www.vaikan.com/the-last-line-effect/

 本文由用戶 jopen 自行上傳分享,僅供網友學習交流。所有權歸原作者,若您的權利被侵害,請聯系管理員。
 轉載本站原創文章,請注明出處,并保留原始鏈接、圖片水印。
 本站是一個以用戶分享為主的開源技術平臺,歡迎各類分享!