Ефект останнього рядка

    Copy-Paste
Я вивчив безліч помилок, що виникають в результаті копіювання коду. І стверджую, що найчастіше помилки допускають у пізньому фрагменті однотипного коду. Раніше я не зустрічав в книгах опису цього феномена, тому й вирішив про нього написати. Я назвав його «ефектом останнього рядка».
 
 Введення
Мене звати Андрій Карпов. Я займаюся незвичайним заняттям. Я досліджую програмний код додатків за допомогою статичних аналізаторів і описую знайдені помилки і недоліки. Займаюся я цим через прагматичних і корисливих спонукань. Так наша компанія рекламує інструменти PVS-Studio і CppCat. Знайшли помилки. Описали в статті. Привернули увагу. Profit. Але сьогодні стаття не про аналізатори.
 
У процесі аналізу проектів я зберігаю помічені помилки і відповідні фрагменти коду в спеціальній базі. До речі, з вмістом цієї бази може познайомитися будь-хто. Ми перетворюємо її в набір html-сторінок і викладаємо їх на сайті в розділі " Виявлені помилки ".
 
База унікальна! Зараз вона містить близько 1500 фрагментів коду з помилками. Вона чекає людей, які зможуть виявити в цих помилках закономірності. Це може послужити основою багатьох досліджень і матеріалів для книг і статей.
 
Спеціально я не робив ніяких досліджень накопиченого матеріалу. Проте одна закономірність так явно проявляє себе, що я вирішив вивчити її детальніше. У статтях мені часто доводиться писати «зверніть увагу на останній рядок». Я вирішив, що це не спроста.
 
 Ефект останнього рядка
У вихідному коді програми часто потрібно написати кілька однотипних конструкцій, що йдуть підряд. Набирати кілька разів однотипний код нудно і не ефективно. Тому використовується метод Copy-Paste. Фрагмент коду копіюється кілька разів, після чого робляться правки. Всі знають, що цей метод поганий. Легко забути щось поміняти і в результаті код буде містити помилку. На жаль, часто хорошою альтернативи немає.
 
Тепер про закономірності. Я з'ясував, що найчастіше помилка допускається в самому останньому скопійованому блоці коду.
 
Простий короткий приклад:
 
inline Vector3int32& operator+=(const Vector3int32& other) {
  x += other.x;
  y += other.y;
  z += other.y;
  return *this;
}

Зверніть увагу на рядок «z + = other.y;». У неї забули поміняти 'y' на 'z'.
 
Приклад здається штучним. Але цей код узятий з реального програми. Далі я переконливо покажу, що це дуже часта і поширена ситуація. Саме так і виглядає «ефект останнього рядка». Людина найчастіше допускає помилку в самому кінці внесення однотипних виправлень.
 
Я десь чув, що часто альпіністи зриваються на останніх десятках метрах підйому. Не тому, що вони втомилися. Просто вони радіють, що залишилося зовсім небагато. Вони передчувають солодкий смак перемоги над вершиною. У результаті вони послаблюють увагу і допускають фатальну помилку. Мабуть, щось таке трапляється і з програмістами.
 
Тепер трохи цифр.
 
Вивчивши базу помилок, я виявив 84 фрагмента коду, які, як мені здається, були написані з використанням Copy-Paste. З них 41 фрагмент містить помилку десь у середині скопійованих блоків. Ось приклад:
 
strncmp(argv[argidx], "CAT=", 4) &&
strncmp(argv[argidx], "DECOY=", 6) &&
strncmp(argv[argidx], "THREADS=", 6) &&
strncmp(argv[argidx], "MINPROB=", 8)) {

Довжина рядка «THREADS =» не 6, а 8 символів.
 
В інших 43 випадках помилка була знайдена в самому останньому скопійованому блоці.
 
На перший погляд, число 43 зовсім небагато більше 41. Але врахуйте, що однотипних блоків буває достатньо багато. І помилка може бути в першому, другому, п'ятому чи навіть у десятому блоці. Отримуємо відносно рівне розподіл помилок у блоках та різкий стрибок в кінці.
 
У середньому я взяв, що кількість однотипних блоків дорівнює 5.
 
Виходить, що на перші 4 блоки припадає 41 помилка. Або близько 10 помилок на один блок.
 
На останній п'ятий блок припадає 43 помилки!
 
Для наочності можна побудувати ось такий приблизний графік:
 
 Ð Ð¸ÑÑƒÐ½Ð¾Ðº 1. Приблизительный график количества ошибок в пяти блоках похожего кода.
 
Малюнок 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' буде завжди дорівнює одиниці. Останній рядок повинна бути такою:
 
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» не 3, а 7 символів.
 
На цьому зупинимося. Думаю, наведених прикладів більш ніж достатньо.
 
 Висновок
У цій статті ви дізналися, що при використанні Copy-Paste ймовірність допустити помилку в останньому скопійованому блоці в 4 рази вище, ніж в будь-якому іншому.
 
Ця особливість психології людини, а не його професійних навичок. У статті ми побачили, що до тяги до помилок в кінці схильні навіть висококваліфіковані розробники таких проектів, як Clang або Qt.
 
Сподіваюся моє спостереження виявиться корисним. І, можливо, підштовхне людей до дослідження накопиченої бази помилок. Думаю, вона дозволить знайти багато цікавих закономірностей і сформулювати нові рекомендації для програмістів.
 
 цю статтю англійською
Якщо хочете поділитися цією статтею з англомовною аудиторією, то прошу використовувати посилання на переклад: Andrey Karpov. The Last Line Effect .
 
 Прочитали статтю і є питання? Часто до наших статей ставлять одні і ті ж питання. Відповіді на них ми зібрали тут: Відповіді на питання читачів статей про PVS-Studio і CppCat, версія 2014 . Будь ласка, ознайомтеся зі списком.
 
    
Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.