О магических числах и немного об API
Есть у нас на работе группа разработчиков из разных команд, составляющих такой себе архитектурный консилиум. Называю их заочно «людьми, которым всё ещё не похуй» 🙂
Часто подключаюсь к их обсуждениям онлайн, чтоб послушать, и каждый раз прям вдохновляюсь тем, как они обсуждают и решают разного рода реальные проблемы и риски: глобальные, но необходимые рефакторинги, оптимизации, сертификации и т.д.
В частности силами этой группы в проекте внедрили наконец статический анализ кода. И вот вчера кто-то в чате пожаловался, что линтер ругается на magic numbers. Так ли надо, мол, от них везде избавляться? Очевидно же, что то или иное означает…
Это, конечно, не табы против пробелов, но по количеству комментов за один вечер обсуждение уже опередило любое другое за последние месяцы 🙂
Началось всё вот с этого:
warning: 16 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers] retVal << "0x" + QString::number(static_cast(pid), 16).rightJustified(4, '0');Опустим лишнее и оставим вот этот кусок кода:
"0x" + QString::number(static_cast<int>(pid), 16).rightJustified(4, '0');
(Отдельные лучи поноса Teletype, на котором вы читаете этот текст, за невозможность сделать автоматический перенос строк в блоке кода. На телефоне эта строка выглядит как говно).
Что такое 16 и что такое 4? Я вот хз лично. Стопроцентной уверенности у меня нет.
Да, я эти (очевидно, библиотечные) функции никогда не юзал, а если и юзал, то забыл сразу, но в этом ведь и смысл: код должен быть понятным, если читающий с ним незнаком.
Можно, конечно, сделать предположения. Этим до конца статьи и займёмся, немного утрируя по ходу дела в надежде, что читатели умеют экстраполировать один пример на массу других подобных случаев, которые наверняка будут.
В начале строки там "0x", что кагбэ намекает на шестадцатиричную систему. Значит, 16 — это, стало быть, base системы счисления для конвертации. А четвёрка? Ну, наверное, длина результирующей строки, отбитой нулями вправо.
У меня опыта в программировании больше 15 лет, и вот, что он мне говорит по этому поводу: «хуй его знает, что за числа — лучше перепроверь».
Раз:
static QString QString::number(long n, int base = 10);
Два:
QString QString::rightJustified(int width,
QChar fill = QLatin1Char(' '),
bool truncate = false) const;Угадал. Вот реально другого слова тут не подобрать.
Давайте смотреть. Опустим отбивку нулями. Допустим, нет у нас "0x" в качестве префикса, а система двоичная, ну мало ли:
QString::number(mask, 2);
Уже как-то и не так очевидно становится. Что можно из этого вызова понять? Какая-то переменная, какая-то двойка. Хрен знает. Аналогично:
QString::number(mask, 10);
Да, десятку-то можно было и опустить, раз она по дефолту и так идёт. А можно и не опускать. А если можно, то кто-то хоть раз обязательно напишет именно так.
Кстати, лирическое отступление. Я часто говорю, что «senior» developer, знающий только один язык программирования, не может себя senior'ом называть. Ну, банально опыта и широты взглядов недостаточно, чтобы красивые решения пилить. Может, сгущаю краски, кочено. Так вот…
Как бы аналогичные вызовы выглядели на Python? Ну что-то вроде такого:
QString.number(pid, base=16) QString.number(mask, base=2)
Понятнее? В разы! Вроде такая мелочь, но число перестало быть «магическим», а приобрело имя. Так называемые keyword arguments очень популярны в Python, и идея настолько хорошая, что создателям API язык предоставляет возможность пометить некоторые из параметров как keyword-only:
def number(n, *, base=10):
...Всё, base теперь необходимо передавать только с именем аргумента. Обычным number(mask, 10) уже не обойдёшься.
Можно ли сделать подобное на C++? Давайте пробовать.
Статический анализатор вон просит константу сделать. Окей:
static const int hex_base = 16; QString::number(pid, hex_base);
Ну, строка вызова стала значительно понятнее. Константу можно канеш прям по месту определить, а можно и вынести в какой-то глобальный хедер, но это накладывает гораздо больше ответственности на выбор имени. Сча вы мне тут ещё за constexpr небось парить начнёте. Хотите constexpr — на здоровье. Компилер и так, и так это соптимайзит наверняка, подставив напрямую.
Всё равно, однако, API выглядит всрато немного. Можно помечтать, как бы мы его улучшили, если бы писали библиотеку.
Метод почему-то называется number, а не fromNumber по аналогии с кучей других мест в Qt. Сравните QString::number() и QString::fromNumber(): разница разительна на мой взгляд! Это звучит примерно как «мясо соя» и «мясо из сои». Я люблю, когда код читается практически как естественный язык. Ruby к такому достаточно близок, по-моему, но у нас C++.
Далее надо чё-то думать с base. Я себе слабо представляю нужду переводить числа в какую-то тринадцатеричную систему. Если бы в натуре у нас были только «дефолтные» двоичная, восьмеричная, десятичная и шестнадцатеричная системы, то можно бы какой-то enum склепать:
QString::fromNumber(pid, NumberBase::Hex);
Давайте не будем придираться к названию; всё равно решение не ахти. Оригинальная функция принимает значения base от 2 до 32, кстати. Может тогда вот так?
QString::fromNumber<16>(pid);
Мы тут передаём base не параметром функции, а параметром шаблона. Это выглядит сильно лучше, потому что чётко видно, что 16 относится непосредственно к number, из которого мы чё-то пытаемся получить. И если бы мы гарантированно указывали base всегда литералом (то есть в compile-time), то такой вариант на мой вкус предпочтительнее, но это закрывает нам возможность определять основу системы счисления в рантайме.
Ещё есть упоротый вариант а ля JavaScript с передачей мапы с параметрами:
QString::fromNumber(pid, {{"base", 16}});Ну и или не мапы, а кортежа (это уже похоже на tagged tuples из Erlang):
QString::fromNumber(pid, {"base", 16});В принципе, не так и плохо, но программистам на С++ это будет непривычно и отпугнёт новичков. Я не проверял, кстати, можно ли такое сделать вообще, но особых препятствий навскидку не вижу.
Что там у нас ещё осталось? Ну, например, user-defined literals:
int operator ""_base(unsigned long long int n)
{
return static_cast<int>(n);
}
QString::fromNumber(pid, 16_base);Воу-воу. А это неплохо выглядит! Добавьте constexpr и проверочки на диапазон значений по вкусу. И даже не требует вмешательств в библиотеку Qt. Хотя если бы вмешиваться мы могли, то можно было бы сделать ещё одно улучшение, раз уж мы всё равно даже функцию решили переименовать.
Haskell меня в своё время научил, что при возможности не нужно описывать ожидаемые значения ассёртами, документацией или, прости господи, в комментариях. Вместо этого все инварианты нужно выводить в типы, чтобы компилятор делал работу за нас.
Поэтому очевидным шагом было бы не принимать base как int, а завести отдельный тип. В примере выше мы всё ещё могли написать как просто 16, так и 16_base. Люди ленятся много писать, и даже находят кучу аргументов по типу «да это и не magic number вовсе», «ну там да, там имеет смысл завести константу, а вот тут же всё очевидно и так», и всё в таком духе — лишь бы не признать, что им западло править код. Введение отдельного типа им не позволит так делать.
class NumberBase
{
public:
explicit NumberBase(int n);
...
};
constexpr NumberBase operator"" _base(unsigned long long int n)
{
return NumberBase{static_cast<int>(n)};
}
static QString QString::fromNumber(long n, NumberBase base = 10_base);Теперь вызывать можно только вот так:
QString::fromNumber(pid, 16_base);
либо так:
QString::fromNumber(pid, NumberBase{16});Первый вариант в разы приятнее, поэтому все будут пользоваться именно им. PROFIT!
Итак, что я хотел всем этим сказать? Подытожу:
- Не пытайтесь оправдывать свою лень. Просто признайте, если в натуре впадлу — так всем будет проще.
- Не рассчитывайте, что кому-то что-то очевидно. В нашем деле практически нет очевидных штук, но можно писать код так, чтобы он к очевидности стремился.
- Когда проектируете API, думайте в первую очередь о том, как им будут пользоваться. Вот реально возьмите да напишите разные варианты взаимодействия с вашим классом/модулем/функцией/whatsoever. Я всегда стараюсь сделать красиво и упростить жизнь пользователям, а не себе.
- По возможности выводите инварианты в типы и заставляйте компилятор делать все проверки ещё до запуска программы. Это всяко лучше инфы «под звёздочкой» в документации к функции.
Конец 🙂