Эффект последней строчки
При написании кода программистам часто приходится писать серии одинаковых конструкций. Писать почти одно и то же несколько раз подряд утомительно и неэффективно. Именно потому люди и используют метод копирования-вставки: фрагмент кода копируется и вставляется несколько раз подряд, а потом немного редактируется. Любой понимает опасности этого метода: легко можно забыть внести нужные изменения, что повлечет за собой ошибку. К сожалению, вменяемой альтернативы этому методу в таком случае я не знаю.
Теперь поговорим о закономерности, которую я обнаружил. Я обратил внимание на то, что большинство ошибок допускается в последнем скопированном и вставленном участке кода.
Вот простой пример:
|
1
2
3
4
5
6
|
inline Vector3int32& operator+=(const Vector3int32& other) {
x += other.x;
y += other.y;
z += other.y;.
return *this;
}
|
Обратите внимания на строчку z += other.y;. Программист забыл исправить y на z.
Можно подумать, что это искусственный пример, но нет. Я взял его из реального приложения. В этой статье я собираюсь доказать, что на самом деле это очень распространенная и важная ошибка. Программисты чаще всего ошибаются в самом конце последовательности однотипных действий.
Где-то я слышал, что альпинисты часто срываются на последних десятках метров своего восхождения. Не потому, что они устали: они просто слишком обрадованы тем, что вершина почти достигнута — они уже чувствуют сладкий вкус победы, теряют концентрацию и совершают фатальную ошибку. Я думаю, что с программистами случается примерно то же самое.
Вот еще примеры.
Пролистывая свою базу багов, я нашел 84 фрагмента кода, ошибки в которых вызваны многострадальным копипастом. Из них в 41 фрагменте ошибки содержатся где-то в середине блока скопированных и вставленных строк:
|
1
2
3
4
|
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. Но тут важно учитывать: однородных блоков много. Во всем 41 случае они могут находиться в первом, втором, пятом или десятом блоке. Получается, что мы получаем относительно равномерное распределение ошибок по всей длине блока и резкий пик в конце.
Допустим, что длина нашего однородного блока равна 5. Получается, что в первых четырех блоках будет 41 ошибка — в среднем по 10 на блок. И только на один последний блок придется 43 ошибки!
Итак, какую закономерность мы вывели:
Вероятность совершить ошибку в последнем блоке в 4 раза больше, чем в любом из предыдущих.
Я не раздуваю из этого грандиозных выводов. Это просто очень интересное наблюдение, о котором следует знать по практическим причинам — во время копипаста последней строки нужно быть максимально внимательным.
Примеры
Теперь осталось только доказать вам, что эти выводы — не просто мое заблуждение, а настоящая тенденция. Чтобы подтвердить свою позицию, представлю вам немного примеров. Конечно, не все — только самые простые или наиболее характерные.
Source Engine SDK
|
1
2
3
4
5
6
7
8
|
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
|
1
2
3
4
5
6
7
8
9
|
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;
|
Последний блок идентичен предпоследнему.
Multi Theft Auto
|
1
2
3
4
5
6
7
8
9
10
11
12
13
14
|
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
|
1
2
3
4
5
6
|
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
|
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
|
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
|
1
2
3
4
5
6
|
inline typename Value<Pipe>::Type const & operator*() {
tmp.i1 = *in.in1;
tmp.i2 = *in.in2;
tmp.i3 = *in.in2;
return tmp;
}
|
ReactOS
|
1
2
3
4
|
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. Вот как должна была выглядеть последняя строчка:
|
1
|
const int mjstride = sizeof(mag[0][0]) / sizeof(mag[0][0][0]);
|
Mozilla Firefox
|
1
2
3
4
5
6
7
|
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
|
1
2
3
|
if (fabs(dir[0]) > test->radius ||
fabs(dir[1]) > test->radius ||
fabs(dir[1]) > test->radius)
|
В последней строке должно быть dir[2].
Clang
|
1
2
3
4
5
6
7
8
|
return (ContainerBegLine <= ContaineeBegLine &&
ContainerEndLine >= ContaineeEndLine &&
(ContainerBegLine != ContaineeBegLine ||
SM.getExpansionColumnNumber(ContainerRBeg) <=
SM.getExpansionColumnNumber(ContaineeRBeg)) &&
(ContainerEndLine != ContaineeEndLine ||
SM.getExpansionColumnNumber(ContainerREnd) >=
SM.getExpansionColumnNumber(ContainerREnd)));
|
В самом конце блока выражение SM.getExpansionColumnNumber(ContainerREnd) сравнивается с самим собой.
Выводы
Из этой статьи вы узнали, в чем опасность метода копипаста — ошибка чаще всего совершается в самом конце. Я думаю, причина этого кроется в человеческой психологии, а не профессиональных навыках. То, что вы увидели выше, написали не новички, а высокопрофессиональные разработчики проектов вроде Clang или Qt.
Надеюсь, мои наблюдения пригодятся вам и помогут снизить число багов, пополняющих мою коллекцию.
Источник: блог How Not To Code