Pull to refresh
489.62
PVS-Studio
Static Code Analysis for C, C++, C# and Java

60 антипаттернов для С++ программиста, часть 11 (совет 51 — 55)

Level of difficultyEasy
Reading time10 min
Views5.8K

1053_60_cpp_antipatterns_ru/image2.png


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


Я буду публиковать советы по 5 штук, чтобы не утомить вас, так как мини-книга содержит много интересных отсылок на другие статьи, видео и т. д. Однако, если вам не терпится, здесь вы можете сразу перейти к её полному варианту: "60 антипаттернов для С++ программиста". В любом случае желаю приятного чтения.


Вредный совет N51. Максимально откладывай использование нового C++ стандарта


Поддерживай максимально старый стандарт C++ для совместимости.


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


Но такое бывает редко. Поэтому нет смысла откладывать переход на новую версию языка. Язык C++ с каждой версией позволяет писать всё более лаконичный, безопасный и эффективный код.


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


Это "может быть", скорее всего, никогда не произойдёт. Если же это случится, то, скорее всего, основные проблемы будут вовсе не в том, что какие-то конструкции языка C++ не будут компилироваться. Проблемы будут, например, с библиотеками, с необходимостью адаптировать графический интерфейс, с оптимизацией скорости работы и так далее. Другими словами, будет куда больше разных сложностей, на фоне которых заботиться именно о версии стандарта языка имеет мало смысла.


Вредный совет N52. Переиспользование переменных


Максимально переиспользуйте переменные. Выделяйте память один раз, а затем используйте её повторно, например: если вам нужно три целых числа в функции, создайте одно, назовите его чем-то универсальным, например x, а затем используйте x для всех трех.


Очень вредный совет, усложняющий понимание программы и тем самым провоцирующий ошибки.


Во-первых, имя переменной в любом случае будет неудачным:


  • если дать переменной осмысленное название, то оно будет логичным и понятным только в одном случае. При использовании этой переменной для других целей имя переменной будет только сбивать с толку;
  • если, как предлагается в совете, назвать переменную универсально (a, x, foo), то она не даст никакой информации при чтении программы, и код будет сложен для понимания.

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


Примечание. Это можно обойти, создав различные ссылки на одну и ту же переменную. Однако в итоге так ещё проще запутаться. Случайно можно начать одновременно использовать одну и ту же переменную для записи разных по смыслу значений, перетирая другие, ещё нужные, данные.


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


В-третьих, совет противоречит общепризнанному принципу делать область видимости переменной минимальной:


  • следует объявлять переменную как можно ближе к точке её использования. Идеально, если объявление переменной совмещается с её инициализацией;
  • желательно, чтобы время существования переменной заканчивалось сразу после того, как она перестаёт быть нужна.

Всё это позволяет минимизировать количество ошибок, когда из-за опечаток используется не та переменная. Например, для этого даже позволили объявлять переменные в условии, чтобы их область видимости ограничивалась телом условного оператора:


if (auto p = dynamic_cast<Derived*>(bp))
  p->df();

А как же экономия памяти?


Сомнительный способ экономить память. Если это переменные типа int, создаваемые на стеке, то какой-то существенной экономии не получится. Более того, если время жизни переменной кончилось, компилятор может разместить новую переменную в тех же ячейках памяти на стеке. Или компилятор решит использовать для хранения переменной не стек, а один из регистров процессора. В общем, он может самостоятельно проделать предлагаемую оптимизацию по экономии памяти.


Что с массивами, создаваемыми в куче? Это, конечно, более тяжёлые операции, и оптимизации компилятора, скорее всего, здесь не помогут. Допускаю, что в целях оптимизации может иметь смысл не выделять новый массив, а переиспользовать существующий.


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


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


Вредный совет N53. Отвечай в комментарии на вопрос "что?"


Пишите комментарии, чтобы точно объяснить, что делает строка кода. А почему это делается — и так понятно настоящему программисту.


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


i++; // add 1 to i

Ёлки-палки, любому человеку, знакомому с C++ даже на базовом уровне, и так понятно, что здесь происходит увеличение переменной на единицу. Написанный комментарий не несёт никакой полезной информации и даже вредит, так как загромождает код.


Комментарии должны отвечать не на вопрос "что делаем?", а на вопрос "зачем/почему делаем?".


Здесь очень полезна концепция "Золотого круга", предложенного Саймоном Синеком. Она не про комментарии, но очень хорошо к ним подходит. Все комментарии можно разделить на три группы по вопросу, на который они отвечают:


  • "Что?" — худший вариант, который добавляет мало информации к написанному коду. Единственное осмысленное применение, если перед нами какая-то сложная конструкция и мы хотим пояснить, что она, собственно говоря, делает. По идее, в самодокументируемом коде таких комментариев вообще не должно быть;
  • "Как?" — средний вариант. Такие комментарии могут быть полезны, так как поясняют, как мы что-то делаем. Обычно в них описываются какие-то детали реализации. Например, как мы что-то вычисляем, используя численный метод.
  • "Зачем/почему?" — наиболее полезные высокоуровневые комментарии. Они объясняют суть происходящего, не вдаваясь в детали реализации.

Доклад про золотой круг:



Давайте теперь рассмотрим какой-то реальный пример правильного полезного комментария.


Код, который мы рассмотрим далее, был выписан мною в процессе работы над статьёй "Обработка дат притягивает ошибки или 77 дефектов в Qt 6".


Анализатор PVS-Studio обратил внимание на этот фрагмент кода, выдав предупреждение: V575 [CWE-628] The 'memcpy' function doesn't copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null. qplaintestlogger.cpp 253. Собственно, вот он:


const char *msgFiller = msg[0] ? " " : "";
QTestCharBuffer testIdentifier;
QTestPrivate::generateTestIdentifier(&testIdentifier);
QTest::qt_asprintf(&messagePrefix, "%s: %s%s%s%s\n",
                   type, testIdentifier.data(), msgFiller, msg,
                   failureLocation.data());

// In colored mode, printf above stripped our nonprintable control characters.
// Put them back.
memcpy(messagePrefix.data(), type, strlen(type));

outputMessage(messagePrefix.data());

Обратите внимание на вызов функции memcpy. Код вызывает два вопроса:


  1. Зачем что-то записывается в буфер, содержимое которого было только что сформировано с помощью printf-подобной функции?
  2. Точно не ошибка, что не копируется терминальный ноль? Это как раз и не нравится анализатору.

К счастью, комментарий сразу всё проясняет. Нужно восстановить некие непечатаемые символы.


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


Для сравнения рассмотрим другой фрагмент кода из этого же файла:


char buf[1024];

if (result.setByMacro) {
  qsnprintf(buf, sizeof(buf), "%s%s%s%s%s%s\n", buf1, bufTag, fill,
            buf2, buf2_, buf3);
} else {
  qsnprintf(buf, sizeof(buf), "%s%s%s%s\n", buf1, bufTag, fill, buf2);
}

memcpy(buf, bmtag, strlen(bmtag));
outputMessage(buf);

Здесь забыли сделать аналогичный комментарий. И картина радикально меняется. Этот код способен ввести в замешательство нового члена команды, который будет его сопровождать или модифицировать. Совершенно непонятно, зачем нужен этот memcpy. Более того, непонятно, почему в начало строки печатается содержимое некоего буфера buf1, а затем в начало строки помещается содержимое буфера bmtag. Так много вопросов, и так мало ответов.


Вредный совет N54. Больше многопоточности


Многопоточность всегда эффективнее однопоточности.


Важно, что понимается под эффективностью. Если речь идёт о скорости работы программы в целом, то можно согласиться, что распараллеленный код обработает данные быстрее, чем однопоточный.


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


В этот момент кажется, что утверждение "многопоточность лучше" верна. Пользователю неинтересно, сколько суммарно работают все потоки приложения. Интересно время, за которое будет решена задача.


Не всё так однозначно. Дело в том, что есть ещё такое понятие, как уровни распараллеливания. Чем крупнее "единицы распараллеливания", тем, скорее всего, эффективнее может работать приложение.


Можно привести следующий грубый пример. Пусть у нас есть многоядерный процессор. Стоит задача конвертировать 1 000 000 изображений из jpeg формата, например, в png. В этом случае несколько параллельно работающих однопоточных программ справятся с задачей быстрее, чем программа, которая умеет распараллеливать внутри себя конвертацию картинок. Причина: однопоточные программы проще, и они не тратят время на синхронизацию.


Это ещё не всё. Реализовать высокоуровневый параллелизм легче. Согласитесь, нет ничего сложного, когда несколько экземпляров запущенной программы берут из общего пула картинки и обрабатывают их. Несложно написать менеджер для всего этого.


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


1053_60_cpp_antipatterns_ru/image24.png


Параллельное программирование: ожидание vs реальность.


Приведу ещё один пример разумного подхода. PVS-Studio распараллелен на уровне анализа C++ файлов. Т.е. для анализа каждого C++ файла запускается отдельный процесс, который обрабатывает данные в одном потоке. Это очень эффективно и просто. Сколько есть процессорных ядер, столько экземпляров анализатора и запускается. В общем-то так и параллельная компиляция кода работает.


Но подождите, а если нужно проверить только один файл? Распараллеленный анализатор справился бы с этим быстрее, чем однопоточный.


Да, но это не имеет практического смысла. Один файл и так проверяется быстро. Для пользователя не будет разницы с практической точки зрения. Можно даже грубо прикинуть числа.


PVS-Studio использует внешний препроцессор. И никак не может повлиять на скорость его работы. Препроцессор работает последовательно. Предположим, препроцессирование выполняется за 2 секунды.


Анализатор делает разбор кода и поиск ошибок, предположим, тоже за 2 секунды.


Добавим 1 секунду на разные дополнительные операции. Требуется запросить данные о файле у среды Visual Studio. Запустить препроцессор, анализатор. Открыть окно в среде разработки и отобразить в нём предупреждения.


Итого, пусть анализ файла выполняется за 5 секунд.


Допустим, мы проведём колоссальную работу и распараллелим ядро C++ анализатора. Тогда, анализ файла будет выполняться не за 2, а за 1 секунду. Вряд ли мы сможем получить больше выигрыш, сколько бы потоков ни запускали. Задача разбора на лексемы, построение синтаксического дерева, его обход и так далее очень плохо параллелятся. Ведь нельзя разбирать, скажем, вторую часть файла, не зная, какие типы объявлены в первой части.


В итоге получится 4 секунды, вместо 5. С точки зрения пользователя разница небольшая.


И это единственный сценарий, когда выигрыш заметен. В сценариях, когда обрабатывается много файлов, всё будет максимально быстро.


Зато за ускорение при анализе одного файла придётся заплатить колоссальным усложнением алгоритмов разбора и анализа кода. Стоит ли оно того? Нет.


Как видите, параллельности бывают разные, и не всегда они одинаково полезны.


Вредный совет N55. Чем меньше .cpp файлов, тем лучше


Нет ограничений на количество кода, которое может помещаться в один файл, и так код компилируется быстрее.


Кстати, кто-то может подумать, что я сторонник множества маленьких файлов. Вовсе нет. Меня раздражает подход, когда каждый, даже маленький, класс или какая-то другая сущность реализуется в отдельном файле. В результате найти что-то становится не проще, а сложнее. Часто приходится использовать глобальный поиск по всем файлам. Я за то, чтобы группировать родственные и похожие сущности в одном файле.


Впрочем, вернёмся именно к гигантским файлам. Они плохи по следующим причинам:


  1. В большом файле бывает непросто что-то найти, особенно, когда то, что вы ввели в строку поиска (например, имя функции или переменной), встречается множество раз. Листать гигантский файл вручную вверх-вниз, тоже не вариант. Конечно, среды разработки предоставляют нам различные удобные средства навигации, например, к реализации функции. Но это не всегда работает или помогает. Впрочем, как я уже писал выше, если всё разбить на множество файлов, может быть ещё хуже. Нужен какой-то компромисс, когда в файле находится что-то близкое по своей природе и название файла это ясно отображает. К сожалению, для больших старых проектов это звучит как научная фантастика, так как бывает, что имя файла уже давно не связано с тем, что там находится :).
  2. Неудобно смотреть в системе контроля версий историю изменений. Получается, что всё время правятся одни и те же большие файлы. В результате сложнее понять, что и когда правилось.
  3. Больше вероятность конфликта при командной разработке. Раз файлы гигантские, в них много кода, а значит, много людей работают с этими файлами. Грубо говоря, все постоянно правят одновременно одни и те же файлы. Соответственно, больше вероятность конфликтов при объединении веток кода.

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


В любом случае размер файлов — это точно не то, о чём следует думать, говоря о скорости компиляции. Эффективные подходы и инструменты для этого совсем другие — "Ускорение сборки C и C++ проектов".


И ещё на закуску:



Об этой мини-книге


Автор: Карпов Андрей Николаевич. E-Mail: karpov [@] viva64.com.


Более 15 лет занимается темой статического анализа кода и качества программного обеспечения. Автор большого количества статей, посвящённых написанию качественного кода на языке C++. С 2011 по 2021 год удостаивался награды Microsoft MVP в номинации Developer Technologies. Один из основателей проекта PVS-Studio. Долгое время являлся CTO компании и занимался разработкой С++ ядра анализатора. Основная деятельность на данный момент — управление командами, обучение сотрудников и DevRel активность.


Ссылки на полный текст:



Подписывайтесь на ежемесячную рассылку, чтобы не пропустить другие публикации автора и его коллег.

Tags:
Hubs:
Total votes 6: ↑5 and ↓1+5
Comments2

Articles

Information

Website
pvs-studio.com
Registered
Founded
2008
Employees
31–50 employees