Андрей Карпов

21 Янв 2011

Теги:

#Cpp

Последствия использования технологии Copy-Paste при программировании на Си++ и как с этим быть

21 Янв 2011

Автор: Андрей Карпов

Я занимаюсь созданием анализатора PVS-Studio, выявляющего ошибки в исходном коде приложений на языке C/C++/C++0x. В связи с этим мне приходится просматривать большой объем исходного кода различных приложений, где с помощью PVS-Studio были обнаружены подозрительные участки кода. У меня накопилось достаточно примеров, в которых хорошо видно, когда ошибка появилась на свет из-за копирования участка кода и его модификации. Конечно, это не новая идея, что использовать Copy-Paste при программировании плохо. Однако попробуем не отделываться рекомендацией "не копируйте код" и подойдем к этой теме более внимательно.

Обычно, когда говорят о Copy-Paste в программировании, то имеют в виду следующую ситуацию. Целиком копируется функция или большой фрагмент кода, а затем скопированный код подвергается модификации. Такие действия создают в программе большое количество похожего кода, что затрудняет его сопровождение. Приходится менять одни и те же фрагменты алгоритма в различных функциях и очень легко забыть что-то исправить.

В данном случае действительно уместно говорить о том, что код лучше не копировать. Если хочется создать функцию с похожим поведением, то полезно произвести рефакторинг и выделить общий код в отдельные методы/классы [1]. Или воспользоваться шаблонами и лямбда-функциями. Мы не будем подробнее рассматривать, как можно избежать дублирования кода, так как это не относится к основному вопросу. Главное по возможности избежать дублирования кода в различных функциях. Про это много писали и с полезными рекомендациями знакомо большинство программистов.

Сосредоточимся теперь на том моменте, который обычно умалчивается в книгах и статьях по написанию качественного кода. На самом деле без Copy-Paste программировать не получается.

Мы все копируем небольшие кусочки кода, когда нам надо написать что-то подобное:

GetMenu()->CheckMenuItem(IDC_ LINES_X, MF_BYCOMMAND | nState);
GetMenu()->CheckMenuItem(IDC_ LINES_Y, MF_BYCOMMAND | nState);

Признайтесь себе честно, что нам бывает лень набрать строчку, которая отличается только тем, что вместо символа 'X' надо будет написать символ 'Y'. И это правильно и логично. Скопировать и отредактировать будет быстрее, чем набрать вторую стоку заново, даже с учетом использования специальных инструментов, таких как Visual Assist и IntelliSence.

При этом здесь нет смысла говорить о дублировании кода. Как не думай, проще здесь написать невозможно. Подобных примеров можно привести огромное количество, взяв любую программу. Не нравится, что здесь пример касается GUI, так и в других задачах мы встретим аналогичное:

int texlump1 = Wads.CheckNumForName ("TEXTURE1", ns_global, wadnum);
int texlump2 = Wads.CheckNumForName ("TEXTURE2", ns_global, wadnum);

Беда в том, что при таком "микрокопировании" вероятность появления ошибки также достаточно высока. А поскольку таких маленьких копирований кода намного больше, чем копирования больших блоков, то это действительно важная проблема. Как быть с этой проблемой непонятно, поэтому про нее стараются умолчать. Нельзя запретить программистам копировать код.

Много из подобных ошибок обнаруживается при первом же запуске программы и быстро безболезненно исправляются. Но многие остаются и живут в коде годами, дожидаясь своего часа. Обнаружить в коде такие ошибки бывает непросто, так как рассматривать похожие строки кода сложно и внимание человека быстро притупляется. При этом наличие ошибок, возникших из-за Copy-Paste, практически не зависит от профессионализма программиста. Опечататься и просмотреть что-то может любой человек. Дефекты такого рода попадаются даже в очень известных и качественных программных продуктах.

Чтобы лучше пояснить, о каких же все-таки ошибках идет речь, рассмотрим несколько примеров кода, взятых из open-source проектов. На правах рекламы: приведенные здесь ошибки были обнаружены мной с помощью анализатора общего назначения, входящего в состав PVS-Studio [2].

Код взят из программы записи и редактирования звука - Audacity.

sampleCount VoiceKey::OnBackward (...) {
  ...
  int atrend = sgn(
    buffer[samplesleft - 2]-buffer[samplesleft - 1]);
      ztrend = sgn(
    buffer[samplesleft - WindowSizeInt-2]-
      buffer[samplesleft - WindowSizeInt-2]);
  ...
}

Программист мужественно и корректно написал инициализацию переменной 'atrend'. Начал писать инициализацию переменной 'ztrend'. Написал "sgn(buffer[samplesleft - WindowSizeInt-2]". Потом вздохнул и скопировал кусочек строки. А отредактировать забыл. Как результат функция 'sgn' получит в качестве аргумента значение 0.

Дальше сценарий будет аналогичен. Программист пишет длинное условие в 3D SDK Crystal Space:

inline_ bool Contains(const LSS& lss)
{
  // We check the LSS contains the two 
  // spheres at the start and end of the sweep
  return
    Contains(Sphere(lss.mP0, lss.mRadius)) && 
    Contains(Sphere(lss.mP0, lss.mRadius));
}

Здесь так и хочется скопировать "Contains(Sphere(lss.mP0, lss.mRadius))" и заменить имя 'mP0' на 'mP1'. Но это так легко случайно забыть сделать.

Наверное, вы иногда замечали, что окна программ вдруг неожиданно начинают вести себя странным образом. Например, многие программисты вспомнят окно поиска в первой редакции Visual Studio 2010. Думаю, такие странности бывает из-за удачного стечения обстоятельств и кода наподобие этого:

void COX3DTabViewContainer::OnNcPaint() 
{
  ...
  if(rectClient.top<rectClient.bottom &&
     rectClient.top<rectClient.bottom)
  {
    dc.ExcludeClipRect(rectClient);
  }
  ...
}

Этот код взят из известного набора классов Ultimate ToolBox. Нормально будет нарисован контрол или нет, будет зависеть от его расположения.

А в eLynx Image Processing SDK скопировали целую строку, и этим растиражировали опечатку.

void uteTestRunner::StressBayer(uint32 iFlags)
{
  ...
  static EPixelFormat ms_pfList[] = 
    { PF_Lub, PF_Lus, PF_Li, PF_Lf, PF_Ld };
  const int fsize = sizeof(ms_pfList) / sizeof(ms_pfList);

  static EBayerMatrix ms_bmList[] = 
    { BM_GRBG, BM_GBRG, BM_RGGB, BM_BGGR, BM_None };
  const int bsize = sizeof(ms_bmList) / sizeof(ms_bmList);
  ...
}

Из-за забытого разыменования указателя переменная 'fsize' равна 1. А потом этот код адаптировали для инициализации 'bsize'. Не верю, что можно два раза подряд так ошибиться, если не копировать код.

В проекте EIB Suite копировалась и редактировалась строка "if (_relativeTime <= 143)". Вот только в последнем условие ее изменить так и забыли:

string TimePeriod::toString() const
{
  ...
  if (_relativeTime <= 143)
    os << ((int)_relativeTime + 1) * 5 << _(" minutes");
  else if (_relativeTime <= 167)
    os << 12 * 60 + ((int)_relativeTime - 143) * 30 << _(" minutes");
  else if (_relativeTime <= 196)
    os << (int)_relativeTime - 166 << _(" days");
  else if (_relativeTime <= 143)
    os << (int)_relativeTime - 192 << _(" weeks");
  ...
}

А значит код "os << (int)_relativeTime - 192 << _(" weeks");" никогда не получит управление.

Даже программисты в компании Intel - тоже всего лишь программисты, а не полубоги. Неудачное копирование в проекте TickerTape:

void DXUTUpdateD3D10DeviceStats(...)
{
  ...
  else if( DeviceType == D3D10_DRIVER_TYPE_SOFTWARE )
    wcscpy_s( pstrDeviceStats, 256, L"WARP" );
  else if( DeviceType == D3D10_DRIVER_TYPE_HARDWARE )
    wcscpy_s( pstrDeviceStats, 256, L"HARDWARE" );
  else if( DeviceType == D3D10_DRIVER_TYPE_SOFTWARE )
    wcscpy_s( pstrDeviceStats, 256, L"SOFTWARE" );
  ...
}

Два раза повторяется условие "DeviceType == D3D10_DRIVER_TYPE_SOFTWARE".

Вообще в зарослях условных операторов очень легко просмотреть ошибку. В реализации Multi-threaded Dynamic Queue в независимости от того что вернет функция IsFixed(), мы сделаем одно и тоже:

BOOL CGridCellBase::PrintCell(...)
{
  ...
  if(IsFixed())
    crFG = (GetBackClr() != CLR_DEFAULT) ?
      GetTextClr() : pDefaultCell->GetTextClr();
  else
    crFG = (GetBackClr() != CLR_DEFAULT) ?
      GetTextClr() : pDefaultCell->GetTextClr();
  ...
}

Кстати, копировать код легко и приятно! Не жалко и лишнюю сточку написать. :)

void RB_CalcColorFromOneMinusEntity( unsigned char *dstColors ) {
  ...
  unsigned char invModulate[3];
  ...
  invModulate[0] = 255 - backEnd.currentEntity->e.shaderRGBA[0];
  invModulate[1] = 255 - backEnd.currentEntity->e.shaderRGBA[1];
  invModulate[2] = 255 - backEnd.currentEntity->e.shaderRGBA[2];
  invModulate[3] = 255 - backEnd.currentEntity->e.shaderRGBA[3];
  ...
}

И не важно, что массив 'invModulate' состоит всего из трех элементов. Код взят из проекта легендарной игры Wolfenstein 3D.

И напоследок пример посложнее. Код взят из весьма полезного инструмента Notepad++.

void KeyWordsStyleDialog::updateDlg() 
{
  ...
  Style & w1Style =
    _pUserLang->_styleArray.getStyler(STYLE_WORD1_INDEX);
  styleUpdate(w1Style, _pFgColour[0], _pBgColour[0],
    IDC_KEYWORD1_FONT_COMBO, IDC_KEYWORD1_FONTSIZE_COMBO,
    IDC_KEYWORD1_BOLD_CHECK, IDC_KEYWORD1_ITALIC_CHECK,
    IDC_KEYWORD1_UNDERLINE_CHECK);

  Style & w2Style =
    _pUserLang->_styleArray.getStyler(STYLE_WORD2_INDEX);
  styleUpdate(w2Style, _pFgColour[1], _pBgColour[1],
    IDC_KEYWORD2_FONT_COMBO, IDC_KEYWORD2_FONTSIZE_COMBO,
    IDC_KEYWORD2_BOLD_CHECK, IDC_KEYWORD2_ITALIC_CHECK,
    IDC_KEYWORD2_UNDERLINE_CHECK);

  Style & w3Style =
    _pUserLang->_styleArray.getStyler(STYLE_WORD3_INDEX);
  styleUpdate(w3Style, _pFgColour[2], _pBgColour[2],
    IDC_KEYWORD3_FONT_COMBO, IDC_KEYWORD3_FONTSIZE_COMBO,
    IDC_KEYWORD3_BOLD_CHECK, IDC_KEYWORD3_BOLD_CHECK,
    IDC_KEYWORD3_UNDERLINE_CHECK);

  Style & w4Style =
    _pUserLang->_styleArray.getStyler(STYLE_WORD4_INDEX);
  styleUpdate(w4Style, _pFgColour[3], _pBgColour[3],
    IDC_KEYWORD4_FONT_COMBO, IDC_KEYWORD4_FONTSIZE_COMBO,
    IDC_KEYWORD4_BOLD_CHECK, IDC_KEYWORD4_ITALIC_CHECK,
    IDC_KEYWORD4_UNDERLINE_CHECK);
  ...
}

Надо сломать глаза, чтобы рассмотреть здесь ошибку. Поэтому сокращу код для ясности:

styleUpdate(...
  IDC_KEYWORD1_BOLD_CHECK, IDC_KEYWORD1_ITALIC_CHECK,
  ...);
styleUpdate(...
  IDC_KEYWORD2_BOLD_CHECK, IDC_KEYWORD2_ITALIC_CHECK,
  ...);
styleUpdate(...
  IDC_KEYWORD3_BOLD_CHECK, IDC_KEYWORD3_BOLD_CHECK,
  ...);
styleUpdate(...
  IDC_KEYWORD4_BOLD_CHECK, IDC_KEYWORD4_ITALIC_CHECK,
  ...);

Рука разработчик дрогнула и он скопировал не то имя ресурса.

Я могу еще привести в этой статье дефектный код, но это уже становится не интересно. Всеми этими примерами я лишь хотел показать, что такие ошибки присутствуют в разнообразнейших проектах и допускаются как новичками, так и профессионалами. Перейдем, наконец, к обсуждению вопроса, что же с этим всем делать.

Если честно, полноценного ответа я не знаю. По крайней мере, в книгах про подобные ситуации я не читал. А вот на практике я часто встречал последствия мелкого Copy-Paste в программах. В том числе и в своих собственных. Придется импровизировать, давая ответ на вопрос.

Будем исходить из следующего положения:

Программисты копируют участки кода и будут копировать, так как это удобно. Следовательно, такие ошибки всегда будут встречаться в программах.

Из этого вывод:

Предотвратить такие ошибки полностью невозможно, но можно постараться сократить вероятность их создания.

Я вижу два пути как можно сократить количество ошибок данного рода. Во-первых, рационально использовать такие инструменты, как статические анализаторы кода. Они позволяют обнаружить многие ошибки данного класса. Причем сделают это на самых ранних этапах. Дешевле и проще обнаружить и исправить ошибку сразу после написания кода, чем работать с этой же ошибкой, обнаруженной в ходе тестирования.

Второй способ, который поможет в некоторых случаях сократить количество ошибок, это дисциплинировать себя и форматировать копируемый код специальным образом. Поясню на примере.

  int ztrend = sgn(
    buffer[samplesleft - WindowSizeInt-2]-buffer[samplesleft 
- WindowSizeInt-2]);

Вот так, ошибку заметить намного сложнее, чем если бы код выглядел так:

  int ztrend = sgn(
    buffer[samplesleft - WindowSizeInt-2] -
    buffer[samplesleft - WindowSizeInt-2]);

Следует форматировать код так, чтобы места, которые должны отличаться выстраивались визуально в столбик. Так сделать ошибку будет гораздо сложнее. Понятно, что во многих случаях это не спасает и выше я приводил такие примеры. Однако хоть что-то лучше, чем совсем ничего.

К сожалению, других способов хоть как-то сократить количество ошибок связанных с Copy-Paste я не знаю. Еще можно использовать инструменты поиска дублирующегося и похожего кода, но это вполне можно отнести к совету использования статических анализаторов.

Обращаюсь к вам читатели. Мне будет интересно, если вы поделитесь мыслями по этому поводу и предложите другие способы избежать ошибок Copy-Paste. Возможно, прозвучат интересные идеи и многим это принесет значительную пользу.

Свои идеи Вы можете присылать на адрес karpov[@]viva64.com и я буду рад, если мне удастся расширить эту статью.

Библиографический список

Steve McConnell, "Code Complete, 2nd Edition" Microsoft Press, Paperback, 2nd edition, Published June 2004, 914 pages, ISBN: 0-7356-1967-0. (Part 24.3. Reasons to Refactor)
Презентация "PVS-Studio, комплексное решение для разработки современных ресурсоемких приложений". http://www.viva64.com/ru/pvs-studio-presentation/

#Cpp