The Last Line Effect (日本語訳)

この文章は「 The Last Line Effect 」の日本語訳です。

Translated by Hideo Hattori

翻訳についての質問・改訳点等ありましたら <hhatto.jp@gmail.com> まで

• The Last Line Effect (日本語訳)
  • はじめに
  • ラストライン効果（The Last Line Effect）
  • 例
    • Source Engine SDK
    • Chromium
    • ReactOS
    • Multi Theft Auto
    • Source Engine SDK
    • Trans-Proteomic Pipeline
    • SeqAn
    • SlimDX
    • Qt
    • ReactOS
    • Mozilla Firefox
    • Quake-III-Arena
    • Clang
    • MongoDB
    • Unreal Engine 4
    • Qt
    • OpenSSL
  • おわりに

私はコピーアンドペースト(いわゆるコピペ)に起因するいくつかのエラーが 起こる事を学びました。 似たようなコードブロックの最後の行でプログラマがよくミスをするのを確信しました。 プログラミング書籍内でこの現象について書かれていた記憶が無いので、私が書く事にしました。 この現象を “ラストライン効果(last line effect)” と呼びます。

はじめに

私の名前はAndrey Karpovです。私は少し変わった仕事をしています。 具体的には、静的解析ツールを使用して様々なアプリケーションのコードを解析し、 エラーや欠陥について詳細をまとめています。 私は実用的かつ金銭的理由でこれを行なっています、なぜなら 私たちの会社のツールであるPVS-StudioとCppCatの広告のひとつとしてこれを行なっているのです。 これらの仕組みはとてもシンプルなものです。私がバグを発見します。それを文章にまとめます。 それらの文章は潜在的な顧客の注目を集めるのです。 利益にもなります。しかしこの文章はそうではありません。

様々なプロジェクトで解析を行なう場合、 私は特別なデータベースの中からコード片を見つけてバグの発生を阻止します。 ところで、興味を持っている人は誰でもこのデータベースを見ることができます。 私たちはバグをHTMLページにまとめ、ウェブサイトの ” Detected errors ” のセクションにそれらをアップロードしています。

このデータベースはかなりユニークなものになっています。 現在、エラーが発生している1500のコード片を含んでおり、 これらのエラーの中にある規則性を発見しようとしています。 これは今後の研究やマニュアル、文章の有用な根拠になるかもしれないのです。

私はまだ集められた資料の調査を行った事がありません。 しかしひとつのパターンが非常に明確にあらわれているため、それをより深く調査することにしました。 見てのとおり、この文章では「最終行に注目してください」というフレーズがよく出てきます。 この理由は後ほどわかるでしょう。

ラストライン効果（The Last Line Effect）

コードを書いている時、プログラマは一連の似たような構造で書こうとします。 同じコードを何度もタイプすることは退屈で非能率的です。 そのような場面でプログラマはコピペを使います。コードをコピペした後、それを編集します。 この方法が悪い事は誰でも知っていますが、ペーストした行を変更することで リスクをいとも簡単に忘れてしまいます。結果エラーをうむことになるのです。 運の悪い事にこのような方法に対するより良い選択肢が見つかっていないのです。

私はいくつかのパターンを見つけたのでそれについてお話します。 コピペした最終行で最もエラーが多い事を発見しました。

以下がシンプルで短い例です:

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ケースでこれらと同様の誤りが見られました。 1番目、2番目、5番目、あるいは10番目で似たようなコードブロックがあることに気づきました。 私たちは最後のブロックに誤りが非常に多く発生することに気づきました。

私は類似した5コードブロックにおける誤りの平均を調べました。

それまでは1コードブロックにつき10の誤りであったのですが、4コードブロックでは41の誤りがありました。

5コードブロックでは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のような高い技術力を要するプログラマが参加しているプロジェクトでさえ 同じような誤りが発生していることがわかったと思います。

私は私たちのバグデータベースに興味を持ってもらい、 プログラマがそれを役立ててくれることを願っています。 私はプログラマのためにエラーから多くの規則性のあるパターンを見つけ出して、 新しい助言として支援できると信じています。