Обновленный сборник ужасных советов для C++-разработчиков превратился в целую электронную книгу. Там вы найдете 60 ужасных советов, каждый с пояснением, почему им не стоит следовать. Все будет и в шутку, и всерьез одновременно. Какими бы нелепыми ни казались эти советы, они не выдуманы, а замечены в реальном мире программирования.
Я буду выкладывать сразу по 5 советов, чтобы не утомлять вас — в книге много ссылок на другие интересные статьи, видео и т. д. Однако, если вам не терпится, сразу перейти к полной версии книги: 60 ужасные советы для разработчика C++». В любом случае, приятного чтения!
Ужасный совет N16. sizeof(int) == sizeof(void *)
Размер int всегда равен 4 байтам. Не стесняйтесь использовать этот номер. Число 4 выглядит намного элегантнее, чем несуразное выражение с оператором sizeof.
Размер int может значительно отличаться. Размер int действительно составляет 4 байта на многих популярных платформах. Но многие — это не значит все! Существуют системы с разными моделями данных. Размер int может быть 8 байт, 2 байта и даже 1 байт!
Формально вот что можно сказать о размере int:
1 == sizeof(char) <= sizeof(short) <= sizeof(int) <= sizeof(long) <= sizeof(long long)
Указатель так же легко может отличаться по размеру от типа int и 4. Например, на большинстве 64-битных систем размер указателя составляет 8 байт, а размер int тип 4 байта.
С этим связана довольно распространенная картина 64-битной ошибки. В старых 32-битных программах разработчики иногда сохраняли указатель в переменных типа int/unsigned. При переносе таких программ на 64-битные системы возникают ошибки — когда разработчики записывают значение указателя в 32-битную переменную, старшие биты теряются. См. Упаковка указателей в Уроки разработки 64-битных приложений на C/C++.
Дополнительные ссылки:
Ужасный совет N17. Не проверяйте, что вернула функция malloc
Нет смысла проверять, была ли выделена память. Современные компьютеры имеют большой объем памяти. А если памяти для выполнения операций недостаточно, то программе и не нужно продолжать работу. Пусть программа рухнет. В любом случае, вы больше ничего не можете сделать.
Если память кончилась, компьютерная игра может задавить. Иногда это приемлемо. Авария в этом случае неприятна, но она не похожа на конец света. Ну, разве что вы сейчас не участвуете в игровом чемпионате :).
Но предположим ситуацию: вы потратили полдня на проект в CAD-системе. Внезапно не хватает памяти для следующей операции — приложение вылетает. Это гораздо неприятнее. Одно дело, если приложение не может выполнить операцию, и совсем другое, если оно вылетает без предупреждения. CAD и аналогичные системы должны продолжать работать. Хотя бы дать возможность сохранить результат.
Есть несколько случаев, когда недопустимо писать код, который вылетает из-за нехватки памяти:
- Встроенные системы. Им просто может быть "негде разбиться" :). Многие встроенные программы в любом случае должны продолжать работать. Даже если невозможно нормально работать, программа должна работать по какому-то особому сценарию. Например, программе нужно выключить оборудование, и только потом остановиться. Говорить о встраиваемом ПО вообще и давать рекомендации невозможно. Эти системы и их назначение сильно различаются. Главное, не вариант для таких систем игнорировать нехватку памяти и зависать;
- Системы, в которых пользователь долго работает над проектом. Примеры: CAD-системы, базы данных, системы видеомонтажа. Сбой в случайное время может привести к потере части работы или привести к повреждению файлов проекта;
- Библиотеки. Вы не знаете, в каком проекте будет использоваться библиотека и как. Так что игнорировать ошибки выделения памяти в них просто недопустимо. Код библиотеки должен возвращать ошибку или вызывать исключение. И пользовательское приложение должно решить, как поступить в этой ситуации;
- Другие вещи, которые я забыл или не упомянул.
Эта тема во многом пересекается с моей статьей «Четыре причины проверить, что вернула функция malloc». Я рекомендую прочитать это. Не все так просто и очевидно, как кажется на первый взгляд с ошибками выделения памяти.
Ужасный совет N18. Расширить стандартное пространство имен
Расширьте пространство имен std различными дополнительными функциями и классами. Ведь для вас эти функции и классы являются стандартными и базовыми.
Несмотря на то, что такие программы успешно компилируются и выполняются, изменение пространства имен std может привести к неопределенному поведению. То же самое касается пространства имен posix.
Чтобы прояснить ситуацию, приведу диагностическое правило V1061, которое предназначено для обнаружения этих недопустимых расширений пространства имен.
Содержимое пространства имен std определяется исключительно комитетом ISO, и стандарт запрещает добавлять в него следующее:
- объявления переменных;
- объявления функций;
- объявления класса/структуры/объединения;
- декларации перечисления;
- объявления шаблонов функций/классов/переменных (C++14).
Стандарт позволяет добавлять следующие специализации шаблонов, определенных в пространстве имен std, при условии, что они зависят по крайней мере от одного определенного в программе типа:
- полная или частичная специализация шаблона класса;
- полная специализация шаблона функции (до C++20);
- полная или частичная специализация шаблона переменной, не расположенного в заголовке ‹type_traits› (до C++20);
Однако специализации шаблонов, расположенных внутри классов или шаблонов классов, запрещены.
Наиболее распространенными сценариями, когда пользователь расширяет пространство имен std, является добавление перегруженной версии функции std::swap и добавление полной/частичной специализации std: :hash шаблон класса.
В следующем примере показано добавление перегруженной версии функции std::swap:
template <typename T>
class MyTemplateClass
{
....
};
class MyClass
{
....
};
namespace std
{
template <typename T>
void swap(MyTemplateClass<T> &a, MyTemplateClass<T> &b) noexcept // UB
{
....
}
template <>
void swap(MyClass &a, MyClass &b) noexcept // UB since C++20
{
....
};
}
Первый шаблон функции добавляет новую перегруженную версию std::swap, так что это объявление приведет к неопределенному поведению. Второй шаблон функции является специализацией, и поведение программы определяется стандартом C++20. Однако есть и другой способ: мы могли бы переместить обе функции из пространства имен std и поместить их в то, где определены классы:
template <typename T>
class MyTemplateClass
{
....
};
class MyClass
{
....
};
template <typename T>
void swap(MyTemplateClass<T> &a, MyTemplateClass<T> &b) noexcept
{
....
}
void swap(MyClass &a, MyClass &b) noexcept
{
....
};
Теперь, когда вам нужно написать шаблон функции, который использует функцию swap для двух объектов типа T, вы можете написать следующее:
template <typename T>
void MyFunction(T& obj1, T& obj2)
{
using std::swap; // make std::swap visible for overload resolution
....
swap(obj1, obj2); // best match of 'swap' for objects of type T
....
}
Теперь компилятор выберет требуемую перегрузку на основе поиска, зависящего от аргумента (ADL): определяемые пользователем функции swap для MyClass и для MyTemplateClass шаблон. Компилятор также выберет стандартную функцию std::swap для всех остальных типов.
Следующий пример демонстрирует добавление специализации шаблона класса std::hash:
namespace Foo
{
class Bar
{
....
};
}
namespace std
{
template <>
struct hash<Foo::Bar>
{
size_t operator()(const Foo::Bar &) const noexcept;
};
}
По стандарту этот код действителен, поэтому анализатор не выдает здесь предупреждение. Но, начиная с C++11, есть и другой способ сделать это, а именно написать специализацию шаблона класса вне пространства имен std:
template <>
struct std::hash<Foo::Bar>
{
size_t operator()(const Foo::Bar &) const noexcept;
};
В отличие от пространства имен std, стандарт C++ вообще запрещает любое изменение пространства имен posix.
Вот дополнительная информация:
- Стандарт C++17 (рабочий проект N4659), параграф 20.5.4.2.1
- Стандарт C++20 (рабочий проект N4860), параграф 16.5.4.2.1
Ужасный совет N19. Старая школа
Ваши коллеги должны знать о вашем обширном опыте работы с языком C. Не стесняйтесь демонстрировать им свои сильные навыки ручного управления памятью и использования longjmp.
Другая версия этого совета: умные указатели и прочие RAII — от лукавого. Управляйте всеми ресурсами вручную — это делает код простым и понятным.
Нет причин отказываться от умных указателей и использовать сложные конструкции при работе с памятью. Умные указатели в C++ не требуют дополнительного времени обработки. Это не сборка мусора. Кроме того, умные указатели помогают сократить и упростить код, тем самым снижая риск совершения ошибки.
Давайте разберемся, почему ручное управление памятью ненадежно и утомительно. Мы начнем с простейшего кода на C, в котором память выделяется и освобождается.
Примечание. В примерах я проверяю только выделение и освобождение памяти. На самом деле это огромная тема ручного управления ресурсами. Вы можете использовать, например, fopen вместо malloc.
int Foo()
{
float *buf = (float *)malloc(ARRAY_SIZE * sizeof(float));
if (buf == NULL)
return STATUS_ERROR_ALLOCATE;
int status = Go(buf);
free(buf);
return status;
}
Код простой и понятный. Функция выделяет память для каких-то целей, использует ее, а затем освобождает. Кроме того, вы должны проверить, смог ли malloc выделить память. Ужасный совет N17 объясняет, почему эта проверка имеет решающее значение в коде.
А теперь представьте: вам нужно выполнить операции с двумя разными буферами. Код сразу начинает расти — если очередное выделение памяти не удается, нужно позаботиться о предыдущем буфере. Более того, теперь вам нужно рассмотреть результат функции Go_1.
int Foo()
{
float *buf_1 = (float *)malloc(ARRAY_SIZE_1 * sizeof(float));
if (buf_1 == NULL)
return STATUS_ERROR_ALLOCATE;
int status = Go_1(buf_1);
if (status != STATUS_OK)
{
free(buf_1);
return status;
}
float *buf_2 = (float *)malloc(ARRAY_SIZE_2 * sizeof(float));
if (buf_2 == NULL)
{
free(buf_1);
return STATUS_ERROR_ALLOCATE;
}
status = Go_2(buf_1, buf_2);
free(buf_1);
free(buf_2);
return status;
}
Становится хуже. Код растет нелинейно. С тремя буферами:
int Foo()
{
float *buf_1 = (float *)malloc(ARRAY_SIZE_1 * sizeof(float));
if (buf_1 == NULL)
return STATUS_ERROR_ALLOCATE;
int status = Go_1(buf_1);
if (status != STATUS_OK)
{
free(buf_1);
return status;
}
float *buf_2 = (float *)malloc(ARRAY_SIZE_2 * sizeof(float));
if (buf_2 == NULL)
{
free(buf_1);
return STATUS_ERROR_ALLOCATE;
}
status = Go_2(buf_1, buf_2);
if (status != STATUS_OK)
{
free(buf_1);
free(buf_2);
return status;
}
float *buf_3 = (float *)malloc(ARRAY_SIZE_3 * sizeof(float));
if (buf_3 == NULL)
{
free(buf_1);
free(buf_2);
return STATUS_ERROR_ALLOCATE;
}
status = Go_3(buf_1, buf_2, buf_3);
free(buf_1);
free(buf_2);
free(buf_3);
return status;
}
Интересно, что код все еще не такой сложный. Вы можете легко писать и читать его. Однако что-то в этом кажется неправильным. Больше половины кода почти бесполезно — просто проверяет статусы и выделяет/освобождает память. Вот почему ручное управление памятью — это плохо. Много нужных, но ненужных строк кода.
Хоть код и не сложный, разработчики легко могут ошибиться при его росте. Например, они могут забыть освободить указатель при выходе из функции ранее и получить утечку памяти. Фактически мы сталкиваемся с этой ошибкой в проектах, когда проверяем их с помощью PVS-Studio. Вот фрагмент кода из проекта PMDK:
static enum pocli_ret
pocli_args_obj_root(struct pocli_ctx *ctx, char *in, PMEMoid **oidp)
{
char *input = strdup(in);
if (!input)
return POCLI_ERR_MALLOC;
if (!oidp)
return POCLI_ERR_PARS;
....
}
Функция strdup создает копию строки в буфере. Затем буфер должен быть где-то освобожден с помощью функции free. Здесь, если аргумент oidp является нулевым указателем, произойдет утечка памяти. Правильный код должен выглядеть так:
char *input = strdup(in);
if (!input)
return POCLI_ERR_MALLOC;
if (!oidp)
{
free(input);
return POCLI_ERR_PARS;
}
Или вы можете переместить проверку аргумента в начало функции:
if (!oidp) return POCLI_ERR_PARS; char *input = strdup(in); if (!input) return POCLI_ERR_MALLOC;
Так или иначе, это классическая ошибка в коде с ручным управлением памятью.
Вернемся к синтетическому коду с тремя буферами. Можно проще? Да, с одним шаблоном точки выхода и операторами goto.
int Foo()
{
float *buf_1 = NULL;
float *buf_2 = NULL;
float *buf_3 = NULL;
int status;
buf_1 = (float *)malloc(ARRAY_SIZE_1 * sizeof(float));
if (buf_1 == NULL)
{
status = STATUS_ERROR_ALLOCATE;
goto end;
}
status = Go_1(buf_1);
if (status != STATUS_OK)
goto end;
buf_2 = (float *)malloc(ARRAY_SIZE_2 * sizeof(float));
if (buf_2 == NULL)
{
status = STATUS_ERROR_ALLOCATE;
goto end;
}
status = Go_2(buf_1, buf_2);
if (status != STATUS_OK)
{
status = STATUS_ERROR_ALLOCATE;
goto end;
}
buf_3 = (float *)malloc(ARRAY_SIZE_3 * sizeof(float));
if (buf_3 == NULL)
{
status = STATUS_ERROR_ALLOCATE;
goto end;
}
status = Go_3(buf_1, buf_2, buf_3);
end:
free(buf_1);
free(buf_2);
free(buf_3);
return status;
}
Теперь код стал намного лучше, и это то, что часто делают разработчики C. Я не могу назвать такой код хорошим и красивым, но мы имеем то, что имеем. Ручное управление ресурсами в любом случае пугает…
Кстати, некоторые компиляторы поддерживают специальные расширения для языка C, которые могут сильно упростить жизнь разработчику. Можно использовать конструкции следующего вида:
void free_int(int **i) {
free(*i);
}
int main(void) {
__attribute__((cleanup (free_int))) int *a = malloc(sizeof *a);
*a = 42;
} // No memory leak, free_int is called when a goes out of scope
Подробнее об этой магии читайте здесь: RAII в C: расширение компилятора очистки gcc.
Теперь вернемся к страшному совету. Проблема в том, что некоторые разработчики до сих пор используют ручное управление памятью в коде C++, даже когда это не имеет смысла! Не делайте этого. C++ позволяет сократить и упростить код.
Вы можете использовать такие контейнеры, как std::vector. Даже если вам нужен массив байтов, выделенный с помощью оператора new [], вы можете сделать код намного лучше.
int Foo()
{
std::unique_ptr<float[]> buf_1 (new float[ARRAY_SIZE_1]);
if (int status = Go_1(buf_1); status != STATUS_OK)
return status;
std::unique_ptr<float[]> buf_2(new float[ARRAY_SIZE_2]);
if (int status = Go_2(buf_1, buf_2); status != STATUS_OK)
return status;
std::unique_ptr<float[]> buf_3(new float[ARRAY_SIZE_3]);
reutrn Go_3(buf_1, buf_2, buf_3);
}
Великолепный! Результат вызова оператора new[] проверять не нужно — в случае ошибки создания буфера будет выброшено исключение. Буферы освобождаются автоматически, если возникают исключения или когда функция завершается нормально.
Итак, какой смысл писать на C++ по-старому? Никто. Почему мы до сих пор встречаем такой код? Я думаю, что есть несколько ответов.
Во-первых. Разработчики делают это по привычке. Они не хотят узнавать что-то новое и менять свои шаблоны кодирования. На самом деле они пишут код на C с небольшим количеством дополнительной функциональности из C++. Это печально, и я не знаю, как это исправить.
Во-вторых. Ниже приведен код на C++, который раньше был кодом на C. Он был немного изменен, но не был переписан или подвергнут рефакторингу. То есть malloc просто заменили на new, а free заменили на delete. Такой код легко узнать по двум артефактам.
Во-первых, посмотрите на эти чеки — это атавизмы:
in_audio_ = new int16_t[AUDIO_BUFFER_SIZE_W16];
if (in_audio_ == NULL) {
return -1;
}
Нет смысла проверять указатель на NULL. В случае ошибки выделения памяти будет сгенерировано исключение типа std::bad_alloc. Это заказной атавизм. Конечно, есть new(std::nothrow), но это не наш случай.
Во-вторых, часто возникает следующая ошибка: память выделяется с помощью оператора new[], а освобождается с помощью delete. Хотя правильнее использовать delete []. См. «Почему массивы должны быть удалены через delete [] в C++». Пример:
char *poke_data = new char [length + 2*sizeof(int)]; .... delete poke_data;
В-третьих. Страх перед накладными расходами. Нет причин бояться их. Да, интеллектуальные указатели иногда могут иметь накладные расходы, но они незначительны по сравнению с простыми указателями. Однако имейте в виду:
- Возможные накладные расходы от интеллектуальных указателей незначительны по сравнению с относительно медленными операциями выделения и освобождения памяти. Если вам нужна максимальная скорость, то вам нужно думать о том, как уменьшить количество операций выделения/освобождения памяти, а не о том, использовать ли умный указатель или управлять указателями вручную. Другой вариант — написать свой собственный распределитель;
- Простота, надежность и безопасность кода, использующего умные указатели, на мой взгляд, однозначно перевешивают накладные расходы (которых, кстати, может и не быть вовсе).
Дополнительные ссылки:
- Затраты памяти и производительности интеллектуальных указателей.
- Каковы накладные расходы интеллектуальных указателей по сравнению с обычными указателями в C++?
Четвертое. Разработчики просто не знают, как можно использовать, например, std::unique_ptr. Наверное, они думают так:
Хорошо, у меня есть std::unique_ptr. Он может управлять указателем на объект. Но мне все еще нужно работать с массивами объектов. А еще есть файловые дескрипторы. В некоторых местах я даже вынужден продолжать использовать malloc/realloc. Для всего этого unique_ptr не подходит. Таким образом, проще везде продолжать управлять ресурсами вручную.
Все описанное вполне можно контролировать с помощью std::unique_ptr.
// Working with arrays:
std::unique_ptr<T[]> ptr(new T[count]);
// Working with files:
std::unique_ptr<FILE, int(*)(FILE*)> f(fopen("a.txt", "r"), &fclose);
// Working with malloc:
struct free_delete
{
void operator()(void* x) { free(x); }
};
....
std::unique_ptr<int, free_delete> up((int*)malloc(sizeof(int)));
Вот и все. Надеюсь, я развеял все ваши сомнения.
P.S. Про longjmp я ничего не писал. И не вижу в этом смысла. В C++ для этой цели следует использовать исключения.
Ужасный совет N20. Компактный код
Используйте как можно меньше фигурных скобок и разрывов строк. Попробуйте писать условные конструкции в одну строку. Это уменьшит размер кода и ускорит его компиляцию.
Код будет короче — это бесспорно. Также нельзя отрицать, что код будет содержать больше ошибок.
«Сокращенный код» труднее читать. Это означает, что опечатки с большей вероятностью не будут замечены автором кода и коллегами во время код-ревью. Вы хотите доказательства? Легкий!
Однажды пользователь написал нам по электронной почте, что анализатор PVS-Studio выдает странные ложные срабатывания для условия. Вот картинка, прикрепленная к письму:
Можете ли вы обнаружить ошибку? Возможно нет. Ты знаешь почему? Потому что у нас есть большое сложное выражение, записанное в одну строку. Трудно читать и понимать этот код. Бьюсь об заклад, вы не пытались найти ошибку и продолжили читать статью :).
Но анализатор не поленился попробовать. Он правильно указал на аномалию: некоторые подвыражения всегда истинны или ложны. Давайте рефакторим код:
if (!((ch >= 0x0FF10) && (ch <= 0x0FF19)) ||
((ch >= 0x0FF21) && (ch <= 0x0FF3A)) ||
((ch >= 0x0FF41) && (ch <= 0x0FF5A)))
Теперь гораздо легче заметить, что логический оператор НЕ (!) применяется только к первому подвыражению. Ну, нам просто нужно написать дополнительные скобки. Более подробный рассказ об этой ошибке здесь: «Как PVS-Studio оказалась внимательнее трех с половиной программистов».
В наших статьях мы рекомендуем форматировать сложный код в виде таблицы. Вы можете увидеть этот стиль форматирования чуть выше. Табличное форматирование не гарантирует отсутствие опечаток, но облегчает и ускоряет их замечание.
Я проиллюстрирую эту тему другим примером. Давайте рассмотрим фрагмент кода из проекта ReactOS. PVS-Studio выдал на него следующее предупреждение: V560 Часть условного выражения всегда истинна: 10035L.
void adns__querysend_tcp(adns_query qu, struct timeval now) {
...
if (!(errno == EAGAIN || EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) {
...
}
Это небольшой фрагмент кода — здесь несложно найти ошибку. Однако заметить это в реальном коде может быть гораздо сложнее. Вы можете просто пропустить блок однотипных сравнений и продолжить.
Причина, по которой мы не замечаем таких ошибок, заключается в том, что условия плохо отформатированы. Мы не хотим читать их внимательно, это требует усилий. Остается только надеяться, что раз проверки однотипные, то все будет хорошо — автор кода не ошибся в условии.
Один из способов борьбы с опечатками — табличное форматирование.
Для тех читателей, которые хотят знать, где ошибка, но не хотят сами ее искать — «errno ==» отсутствует в одном месте. В результате условие всегда выполняется, поскольку константа EWOULDBLOCK равна 10035. Вот правильный код:
if (!(errno == EAGAIN || errno == EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) {
Теперь давайте посмотрим, как лучше рефакторить этот фрагмент. Сначала я покажу вам код, оформленный в простейшем табличном стиле. Мне это не нравится.
if (!(errno == EAGAIN || EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) {
Стало лучше, но не настолько. Мне не нравится этот стиль по двум причинам:
- Ошибка по-прежнему не очень заметна;
- Вы должны вставить большое количество пробелов для выравнивания.
Вот почему нам нужно улучшить форматирование кода. Во-первых, не более одного сравнения в строке, тогда ошибку легко заметить. Действительно, ошибка стала более заметной:
a == 1 && b == 2 && c && d == 3 &&
Во-вторых, операторы &&, || и т. д. лучше писать рационально — не справа, а слева.
Обратите внимание, как много работы нужно написать пробелы:
x == a && y == bbbbb && z == cccccccccc &&
Но так работы гораздо меньше:
x == a && y == bbbbb && z == cccccccccc
Код выглядит необычно, но к нему можно привыкнуть.
Соединим все вместе и напишем приведенный выше код в новом стиле:
if (!( errno == EAGAIN
|| EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM)) {
Да, код теперь занимает больше строк кода, но ошибка гораздо заметнее.
Согласен, код выглядит необычно. Тем не менее, я рекомендую этот подход. Пользуюсь уже много лет и очень доволен, поэтому смело рекомендую всем читателям.
То, что код стал длиннее, вообще не проблема. Я бы даже написал что-то вроде этого:
const bool error = errno == EAGAIN
|| errno == EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM;
if (!error) {
Кто-то ворчит, что это длинно и загромождает код? Я согласен. Итак, давайте поместим это в функцию!
static bool IsInterestingError(int errno)
{
return errno == EAGAIN
|| errno == EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM;
}
....
if (!IsInterestingError(errno)) {
Может показаться, что я преувеличиваю и слишком перфекционист, но ошибки в сложных выражениях случаются очень часто. Я бы не запомнил их, если бы не сталкивался с ними постоянно: эти ошибки везде и плохо видны.
Вот еще один пример из проекта WinDjView:
inline bool IsValidChar(int c)
{
return c == 0x9 || 0xA || c == 0xD ||
c >= 0x20 && c <= 0xD7FF ||
c >= 0xE000 && c <= 0xFFFD ||
c >= 0x10000 && c <= 0x10FFFF;
}
В функции всего несколько строк, и все равно в нее закралась ошибка. Функция всегда возвращает true. Проблема в том, что он плохо спроектирован. Многие годы разработчики пропускали его при рецензировании и не заметили там ошибки.
Проведем рефакторинг кода в табличном стиле. Я бы добавил больше скобок:
inline bool IsValidChar(int c)
{
return
c == 0x9
|| 0xA
|| c == 0xD
|| (c >= 0x20 && c <= 0xD7FF)
|| (c >= 0xE000 && c <= 0xFFFD)
|| (c >= 0x10000 && c <= 0x10FFFF);
}
Нет необходимости форматировать код именно так, как я предлагаю. Цель этой заметки — привлечь внимание к опечаткам в «хаотичном коде». Форматируя код в виде таблицы, вы можете избежать множества глупых опечаток. И это здорово. Надеюсь, эта заметка кому-нибудь поможет.
Ложка дегтя. Я честный разработчик, и поэтому должен упомянуть, что иногда табличное форматирование может быть вредным. Вот пример:
inline
void elxLuminocity(const PixelRGBi& iPixel,
LuminanceCell< PixelRGBi >& oCell)
{
oCell._luminance = 2220*iPixel._red +
7067*iPixel._blue +
0713*iPixel._green;
oCell._pixel = iPixel;
}
Это проект eLynx SDK. Разработчик хотел выровнять код, поэтому добавил 0 к 713. К сожалению, разработчик забыл, что 0 в начале числа означает, что оно будет представлено в восьмеричном формате.
Массив строк. Надеюсь, теперь концепция табличного форматирования понятна. Однако, почему бы нам не пересмотреть его еще раз? Давайте проверим один фрагмент кода. Он показывает, что табличное форматирование можно применять ко всем языковым конструкциям, а не только к условиям.
Фрагмент кода взят из проекта Asterisk. Анализатор PVS-Studio выдал следующее предупреждение: V653 Для инициализации массива используется подозрительная строка, состоящая из двух частей. Возможно, что запятая отсутствует. Рассмотрите возможность проверки этого литерала: «KW_INCLUDES» «KW_JUMP».
static char *token_equivs1[] =
{
....
"KW_IF",
"KW_IGNOREPAT",
"KW_INCLUDES"
"KW_JUMP",
"KW_MACRO",
"KW_PATTERN",
....
};
Опечатка — забыли одну запятую. В результате две разные по смыслу строки объединяются в одну. Собственно, вот что здесь написано:
.... "KW_INCLUDESKW_JUMP", ....
Ошибки можно было избежать, используя табличное форматирование. В этом случае разработчик может легко заметить пропущенную запятую.
static char *token_equivs1[] =
{
....
"KW_IF" ,
"KW_IGNOREPAT" ,
"KW_INCLUDES" ,
"KW_JUMP" ,
"KW_MACRO" ,
"KW_PATTERN" ,
....
};
Напомню, что если мы ставим справа разделитель (в данном случае это запятая), то приходится добавлять много пробелов, что неудобно. Особенно если добавить новую длинную строку/выражение — придется переформатировать всю таблицу.
Поэтому код лучше оформить так:
static char *token_equivs1[] =
{
....
, "KW_IF"
, "KW_IGNOREPAT"
, "KW_INCLUDES"
, "KW_JUMP"
, "KW_MACRO"
, "KW_PATTERN"
....
};
Теперь вы можете легко заметить пропущенную запятую и избежать добавления пробелов. Код выглядит аккуратно и понятно. Возможно, такое форматирование может быть непривычным для вас, но я призываю вас попробовать этот формат — вы быстро к нему привыкнете.
Помните: красивый код — это правильный код.
Автор: Андрей Карпов. Электронная почта: karpov[@] viva64.com.
Более 15 лет опыта работы в области статического анализа кода и качества программного обеспечения. Автор большого количества статей о качестве кода C++. Microsoft MVP for Developer Technologies с 2011 по 2021 год. Андрей Карпов — один из основателей проекта PVS-Studio. Долгое время был техническим директором компании и разработчиком ядра анализатора C++. В настоящее время он в основном занимается управлением командой, обучением сотрудников и DevRel.
Полный текст по ссылке: 60 ужасных советов C++-разработчику.
Подпишитесь на ежемесячную рассылку, чтобы быть в курсе последних статей автора и его коллег в PVS-Studio.
