Comments 80
На самом деле проверка вообще не нужна
Конечно не нужна! Ведь new не может выдать nullptr.
А поставить обработчик исключений желательно.
Ведь new не может выдать nullptr.Как минимум раньше (не проверял, как сейчас) в настройках студийного компилятора была опция, включающая возврат NULL из new вместо кидания исключения. Так что я бы не был так уверен в невозможности нулевого указателя. Стандарт стандартом, а реализации реализациями.
В данном случае согласен. В более общем, указатель может инициализироваться при одних условиях и оставаться нулевым в других. Программисты знают про это и пишут лишние проверки перед delete (и перед free в C). Я имел в иду, что такие проверки не нужны, так как delete (и free) корректно обработают нулевой указатель.
Да, еще есть new(std::nothrow), но он тут ни при чём.
А ещё и исключение скорее всего не кинет (во многих системах /proc/sys/vm/overcommit_memory = 0), а при обращении OOM пришлёт SIGKILL.
Лечение: поставить в "2" - тогда будешь получать исключение, но gcc перестанет работать.
Код может компилироваться под железяку без сигналов и с отключенной поддержкой исключений.
На это полагаться нельзя и даже вредно про это думать. Неизвестно как и где будет выполняться программа. Более того, код может быть заимствован для других проектов, работающих в других условиях. Всегда следует исходить из того, что new может сгенерировать исключение.
P.S. Раньше подобное приходилось про malloc писать, теперь new.... эх...
del
немного про стили в условиях. Для себя давно уже определил что ! или просто выражение применять только к булевым, те это условие я бы писал как if (SomeGlobalPointer == nullptr).
В таком варианте при прочтении кода лучше видны не стыковки.
Кстати хорошая практика использовать явное приведения аргумента условия к булеву типу. Те, кто начинал программировать на Object Pascal (Delphi, Lazarus итп), стараются хотя бы поначалу делать именно так, поскольку в Паскале передавать в if в качестве аргумента не булевы типы считается ошибкой.
о нее, это не мое:) я читатель кода, и когда читаю что если 0 равен... , и тут меня клинит, ноль может быть равен только нулю.
Мне кажется уже давно эта проблема присваиваний в условии ловится анализаторами.
А я наоборот люблю Yoda style. Именно за то, что обычно то, какая переменная будет проверяться, чаще всего понятно из контекста, и на деле я хочу глазами сразу увидеть значение, которое ожидается в условии. Так просто удобнее читать)
читать такое надо "Если нулю равна переменная SomeGlobalPointer"..
Думаю, стоит считать конструкции a = b ошибкой компиляции в любом месте, если каким либо образом не сказано иначе (хотя, что должно возвращать операция присваивания - для меня загадка)
public static void Main() {
int a, b, c, d;
a = b = c = d = 1;
Console.WriteLine($"{a} {b} {c} {d}");
}
//1 1 1 1Возвращает само значение. Код на шарпах, но помнится на плюсах можно так же делать.
Можно ещё в условии присваивание и сравнение сразу делать, типа `if((b = a) > 10)`. Но я такое на рефакторинге не пропустил бы.
Я это знаю, писал про то, ПОЧЕМУ решено было сделано так.
С моей точки зрения (без всяких исключений, несмотря на то, что я писал выше), лучше в принципе перевести любое присвоение в разряд statement и тем самым запретить его использование в выражениях.
int a, b, c, d;
a = b = c = d = 1;Что до данного кода, я бы его не пропустил, не говоря уже о конструкциях в if, упомянутых ранее.
а теперь то же самое, но со свойством:
int _x;
int X { get => _x; set => _x = 1; }
public static void Main()
{
int a = X = 5;
Console.WriteLine(a.ToString());
}
кто вспомнит, не подглядывая в спецификацию и не запуская код, какое из "самих значений" вернёт операция присваивания?
я бы писал как
if (SomeGlobalPointer == nullptr)
Между прочим, этим вы нарушаете С++ Guidelines
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-if
Если смотреть не из мира розовых единорогов, где весь код короткий и при этом правильный, то это плохой совет. На мой взгляд, длинный код лучше читается и более защищает от ошибок. По крайней мере в контексте сравнения с nullptr.
Вот, например, как узнать что хотели проверить? Указатель или значение по указателю?
int *p = foo();
if (p) .... // невозможно сказать, верно написано или нет
if (*p) .... // невозможно сказать, верно написано или нет
if (p != nullptr) .... // скорее всего всё хорошо, хотели проверить именно указатель
if (*p != nullptr) .... // ошибка компиляцииИли вот такой реальный пример из проекта Mozilla Firefox:
const char *token = str.get();
if (token == '\0') {
return NS_ERROR_DOM_SYNTAX_ERR; // nothing between commas
}
Если-бы человек был фанатом сокращения текста, он бы написал:
if (!token)И тогда анализатор PVS-Studio не смог-бы найти ошибку. А за счёт дополнительной информации, что сравнивают не с абстрактным нулём, а с нулевым литералом '\0', анализатор способен предположить, что здесь опечатка:
V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *token == '\0'. svgnumberlist.cpp 96
В общем, забыли разыменовать указатель. Правильно:
if (*token == '\0')
if (p) .... // невозможно сказать, верно написано или нет
if (*p) .... // невозможно сказать, верно написано или нет
Серьезно? Быть может вам стоит попрактиковаться в программировании?
Если-бы человек был фанатом сокращения текста, он бы написал:
Никто бы так не написал. Человек четко понимает, что работает с текстом, и отчетливо осознает что именно хочет проверить
К сожалению, if(p) vs if (*p) встречаются многократно при долгосрочной жизни кода. Требования меняются, набивки объектов меняются, передачи копией, ссылкой, указателем, тушкой, чучелком -- меняются друг на друга. И да, вот такие implicit проверки становятся минами замедленного действия, всплывающими когда их не ждёшь.
По хорошему не должно быть чтобы и p и *p имели такую семантику. Это означает что у вас в коде есть что то вроде optional<optional<T>> p
По крайней мере в контексте сравнения с
nullptr.
Как раз сравнени с nullptr и не повышает читаемость. Ну или у вас сказывается профессиональная деформация, когда "дуют на молоко, обжегшись водой" :)
if (p) .... // невозможно сказать, верно написано или нет
if (*p) .... // невозможно сказать, верно написано или нет
Не очень корректный пример, потому что не видно, что потом делают с указателем.
По второй строке частично соглашусь, потому что правильным (относительно абстрактного примера) было бы написать:
if(p && *p) ...а теперь перепишите эту же проверку с использованием избыточности
if((p != nullptr) && (*p != '\0')) ...для меня, в этом примере избыточность ухудшает читаемость, потому что в глаза, в первую очередь, бросается с чем сравнивается указатель, а не операции с ним самим, а значит на review можно пропустить ошибку с отсутствием "*" во втором случае, в короткой форме эта проверка явственно видна. Собственно для чего и написана рекомендация.
Или вот такой реальный пример из проекта Mozilla Firefox:
А тут стоит дочитать рекомендацию до конца (я же привёл только часть)
тут не integer, но и char может принимать далеко не два значения, так что рекомендация прямо говорит, что в вашем примере с Mozilla Firefox она не применима.
И, конечно, всегда нужно руководствоваться здравым смыслом.
Любой компилятор даст варнинг на код мозиллы, так что этот пример как аргумент не зачитываю. Во всех ваших примерах понятно что хотели написать, это в коде написано.
killAllHumans() // не понятно возможно хотели loveAllHumans()
loveAllHumansWithMyHartIRellyMeanIt() // вот так лучше
Если смотреть не из мира розовых единорогов, где весь код короткий и при этом правильный...
Вот почему ваш единорожек иногда такой грустный - он хочет вернуться к себе домой.
clang-tidy, наоборот, против: https://clang.llvm.org/extra/clang-tidy/checks/readability/implicit-bool-conversion.html
Мне нравится Reason. "eliminates some opportunities for mistakes". В то же самое время порождает opprotunities для других ошибок :)
В общем, из всего правила, я согласен только с тем, что не стоит сравнивать указатели с нулём. Сранивать с nullptr более чем имеет смысл, если рассуждать со стороны моего личного опыта. Просто на практике ни разу не встретил ошибок, связанных с явным сравнением, но с десяток раз уже натыкался на баги, связанные именно с желанием сократить код.
В таком варианте при прочтении кода лучше видны не стыковки.
Это иллюзия, проявившаяся из-за того, что вы прямо сейчас погружены в контекст проблемы, понимаете проблему и думаете, что вне этого контекста так же легко заметите проблему. Стоит вам только забыть контекст и спустя время вернуться к коду, увидев такие строчки...
if (SomeGlobalPointer == nullptr)
{
delete[] SomeGlobalPointer;
}...никаких несостыковок вы тут не заметите. И явность вам никак вообще не поможет. Потому что проблема не в отсутствии явности, а в том, что мозг крайне плохо обрабатывает в условиях инварианты, завязанные на инверсию
Лично мне же такой код ещё и усложняет чтение. Ведь для кода в голове обычно строится некоторая ментальная модель. И она строится не на конструкциях языка. В моём, примере, мозг поступает иначе — определяет зависимости между телом ветки и содержимым условия. Это необходимая для понимания сути кода низкоуровневая часть ментального процесса. И независимо от того, способны ли вы её пронаблюдать в своём мозгу сознательно, она на некотором уровне работы мозга отрабатывает
И если в исходном условии зависимость лишь одна — SomeGlobalPointer, возможных исходов в ментальной модели тоже два — либо у нас SomeGlobalPointer, либо у нас не SomeGlobalPointer. Добавив же == nullptr, мы привнесли в ментальную модель ещё как минимум одну сущность. И не важно, что это неизменная часть условия
В нашей ментальной модели теперь сущностей, от которых зависит исполнение тела, стало в два раза больше — две. Также и возможное число исходов (где-то в фоне считаемое мозгом) увеличилось до четырёх — SomeGlobalPointer, не SomeGlobalPointer, nullptr, не nullptr
Это лишь увеличит когнитивную нагрузку при том же семантическом наполнении кода. Вы будете уставать чуть быстрее. Уставая же, будете совершать новые ошибки. Решали одну конкретную частную проблему — добавили новых неочевидным для себя образом
В чистом виде дело привычки. Рассмотрим, например, такие варианты:
if (optValue) {doSomething();}
if (optValue != std::nullopt) {doSomething();}
if (optValue.has_value()) {doSomething();}
Является ли какой-то из них уменьшающим/увеличивающим когнитивную нагрузку по отношению к другим? Мне нравится вариант с has_value как соответствующий тому, что я хочу проверить, есть ли в optional значение или нет (поэтому думать при чтении надо меньше). Есть люди, которым нравится вариант со сравнением, потому что похож на сравнение указателя с nullptr, поэтому думать при чтении надо меньше. Есть люди, которые любят буквы экономить, поэтому для них первый вариант - самое оно (потому что думать при чтении надо меньше). Не думаю, что кто-то из нас прав, а остальные нет.
Не только в программировании, но и при изучении, обдумывании и чтении, мы обычно строим не просто некоторую ментальную модель, а часто стараемся приводить её к иерархическому виду. Например, можно вспомнить, что структурированные правила из учебников учатся и воспринимаются сильно легче, чем разрозненные факты или тем более исключения из них (вспомните, легко ли учились спряжения)
Казалось бы, к чему я вообще о какой-то иерархии в этом вопросе и где здесь иерархия? Всё просто — рано или поздно мозг программиста начнёт любой if (something) обобщает до до некоторой общей абстракции. Не думаю, что у неё есть название, но я бы её выразил как if ok/if not ok. Когда мозг их обобщил, становится очень просто читать такой код. И дело не в его длине, а в том, что все подобные конструкции мозг начинает маппить на то самое внутреннее обобщение
Но при этом, когда вы местами используете if (opt.has_value()), местами if (ptr != nullptr), где-то if (result != http::ok), где-то ещё что-то... Мозг просто хуже это делает — вместо одного обобщения он работает лишь с частными случаями всюду. А исключения и частные случаи мозг не очень любит
Но почему мозг точно так же не может обобщить разные примеры выше?
Потому что ещё был упомянут альтернативный вариант проверки, if (opt != std::nullopt). И это просто ужасный, кошмарнейший пример, связанный уже не с длиной кода. Эта строчка подразумевает, что внутри ветки мы работаем с happy path. И автор имеет ввиду положительный путь исполнения, и код пишет под положительную ветку исполнения. Но в условии при этом он спрашивает буквально "не является ли путь негативным". Изначальное семантически чисто положительное условие превратилось аж в две инверсии. Имеем ввиду положительное — спрашиваем дважды негативное. И, хотя формально две инверсии не меняют семантику на негативную, мозг, видя инверсии и негативизацию в условии... считает их чётность... и считает крайне плохо, и сбоит. Видит отрицания — думает про unhappy path. В первую очередь, конечно. Во вторую таки считает чётность, откатывает в своей ментальной модели то, что навыдумывать успел... Много избыточной ментальной работы, много кеш-миссов в голове
И это ещё не всё. Теперь комбинируем все перечисленные проблемы вместе и получаем ещё одну: if (opt != std::nullopt) и if (opt.has_value()) выражают одну мысль, но совершенно разным языком, который не обобщить до одной понятной концепции в своей ментальной модели. И вместо того, чтобы читать произвольную ветку как if (ok), ваш мозг будет вынужден каждый раз дополнительно разбираться, что значит второй аргумент в сравнении, к какому "виду" сравнений оно относится, считать чётность инверсий и перепроверять себя, забивая ментальный контекст этой мусорной работой и утомляя вас
Чем больше вы утомляетесь, тем больше ошибок совершите, разных и в разных местах
Какой вывод? Выражайте мысли прямо, чтобы ошибаться меньше. И if (something) — самый простой способ выразить мысль прямо. Автор будет меньше раздумывать, каким именно правильным способом это написать (правильный останется единственным, если вписать в стайлгайд и распространить на всю кодовую базу), а значит и с меньшей вероятностью совершит ошибку из-за семантического насыщения. А пользователь тоже будет читать привычный паттерн на автомате, снижая риски запутаться при виде всех этих таких похожих визуально, но таких семантически разных != nullpt, != http::ok, != nullopt. Ой, а в средний закралась ошибка. А вы её заметили?
Ну да, ну да, кто-то пишет if(!result) вместо `if (result != http::ok)`, чтоб потом узнать, что http::ok внезапно 200, а не 0. Не надо так.
Но вот чем принципиально для вас отличается `if (opt.has_value())` от `if (vec.isEmpty())`? Лично для меня при чтении особенно не отличается - проверяем какое-то состояние объекта (я тут не рассматриваю вопрос happy/unhappy path, он немного в другой плоскости лежит).
Можно ещё сравнить std::optional и std::any.Оба типа имеют метод `has_value`, который (цитирую) "checks if object holds a value". При этом один тип имеет каст в бул, а второй нет. Вас не смущает использовать разные способы проверки одного и того же?
Не надо просто все хранить в интах. Если это резалт то у него обязан быть operator bool
Ну да, ну да, кто-то пишет
if(!result)вместоif (result != http::ok), чтоб потом узнать, чтоhttp::okвнезапно 200, а не 0. Не надо так
А вы http-коды храните в интах? Тогда у вас более серьёзные проблемы, чем обсуждаемая выше, до которой ещё дорасти нужно
Но вот чем принципиально для вас отличается
if (opt.has_value())отif (vec.isEmpty())?
Тем, что одно из условий положительно, а другое негативно. Главное, что оба выражены прямо так, как подразумевал программист. А вот стоит только взять if (!vec.isEmpty()) — здесь уже проблемы, описанные мной выше. Имеем ввиду наличие элементов, спрашиваем про отсутствие
Любой из двух следующих вариантов я считаю предпочтительнее:
auto has_element = !vec.empty();
if (has_element) /* ... */;if (vec.count()) /* ... */;Ещё лучше было бы, предоставляй стандартные контейнеры сразу оба — и empty, и has_value
Можно ещё сравнить
std::optionalиstd::any
На самом деле нельзя. У них семантика разная, std::optional семантически предполагает отсутствие значения, это фича для конечных пользователей. А для std::any (как и std::variant) это вынужденная фича, необходимая для поддержания мув-семантики, а не для пользователей. Т.е. не предполагается, что пользователь будет пользоваться has_value/valueless_by_exception как нормальным интерфейсом, ему это в норме и не нужно. А соответственно и неявный каст не нужен — это не часть нормального повседневного интерфейса, так что ответ на вопрос...
Вас не смущает использовать разные способы проверки одного и того же?
Однозначно нет. Пользователь для std::any в норме не пользуется этим интерфейсом. Более того, в архитектуре даже худших среди кодовых баз, с которыми я работал, ещё не было столь серьёзных проблем, чтобы мне нужно было использовать "такой" std::any
А там, где он использовался не как архитектурный костыль — там у меня была гарантия не только на конкретный тип в нём, и на то, что он точно всегда имеет значение
Мне нравилась в Perl конструкция unless. Казалось бы, сахарный сахар, но сразу видно, где "если да", а где "если нет". Очень семантично, я считаю.
Немного не по теме статьи, но каково ваше ощущение (по прошествии пары лет) от востребованности анализаторов для C# и Java , а также анализатора MISRA и CVE?
Или всё-таки C++ был и остаётся основным направлением работы PVS Studio?
Можно в л.с., если это sensitive информация.
Просто интересуюсь темой статического анализа, но мы пишем на Delphi и у нас свой самописный анализатор под наши задачи.
Интерес к C# и Java растёт, но медленно. Думаю, мы сами в этом виноваты, так как команде не хватает сил более активно заниматься их продвижением. Про Java, например, вообще давно статей не писали :(. На тему C++ у нас куда больше разнообразного материала и активностей. В том числе больших и сильных, таких как мини-книга про вредные C++ советы.
MISRA - интерес есть, но у малого количества пользователей. Хорошо, что сделали, но существенно на жизнь это не повлияло. По запросам пользователей скоро будем дорабатывать это направление и делать ещё некоторые диагностики.
CVE - как я понимаю, имеется в виду SCA (software composition analysis). Пока не понятно.
C++ остаётся основным направлением? - Да. Оно самое старое, поэтому там больше всего клиентов и поддержки. И с продвижением у нас в этом ловчее получается.
Спасибо, Андрей!
Большую и хорошую работу делаете! Насчет CVE — вчера вечером был немного не в себе и сам не до конца разобрался, что я имел в виду. У меня в памяти застряла вот эта статья:
https://habr.com/ru/companies/pvs-studio/articles/418143/
И я считал (возможно, ошибочно), что PVS-Studio как-то особенно сигнализирует именно об уязвимостях из списка CVE.
Насчет SCA — тоже интересная тема, не знал о ней ранее.
Насчет сравнения языков — C++ всё-таки лидер по стрелянию себе по ногам, возможно поэтому C# и Java менее востребованы, в них изначально меньше косячат + многое выявляется компилятором. Возникнет ли надежная замена языку С++ — это вопрос. Все пророчили Rust, но там местами запутанность конструкций не меньшего уровня, чем в С++, хоть и safe.
Теоретически, мог бы взлететь анализатор для Python или PHP, но это опять же новые направления, требующие еще и еще ресурсов.
Казалось бы, поставь брекпойнт да проверь, выполняется ли деле delete[] и чему при этом равен указатель. Прежде чем уличать в ложном срабатывании ажно само PVS-Studio! Признайтесь, вы этот пример сами выдумали? :)
Опять же, какое-то сбивающее с толку предупреждение:
V575 The null pointer is passed into 'operator delete[]'. Inspect the argument.
При том что, как сами пишете, оператор delete[] корректно обрабатывает нулевые указатели. Что оно сказать-то хочет? Что указатель возможно нулевой или что указатель обязательно нулевой?
+1 про сбивающее предупреждение. Было бы там The pointer passed into ... is always null, его бы думаю прекрасно понимали.
Признайтесь, вы этот пример сами выдумали? :)
Нет. Такой простой случай сложно придумать :) Правильнее будет сказать, я его "услышал". Хотя можно без кавычек. Это в прямом смысле.
Слышу вдруг коллега в соседней секции: "Ааааа!" и прочие эмоции. Заинтригован, подхожу. Что такое? Он рассказывает и показывает. Я понимаю, что это стоит маленькой заметки :)
Про V575.
Сообщение говорит, что в оператор delete всегда передаётся нулевой указатель. Если бы он не всегда был нулевым, то и предупреждения не будет. Это ведь нормально. А вот передача явного nullptr говорит и какой-то ошибке или избыточном коде. Возможно, опечатались и не ту переменную используют. Не очень понятно, что именно сбивает с толку. на мой взгляд вполне понятное предупреждение. Хотя, конечно, у меня может быть взгляд замылен :).
Если бы не PVS-Studio, в коде так и осталась бы эта ошибка, приводящая к утечке памяти.
Все зависит от вашего определения утечки памяти. Если ваш синтетический тест повторяет структуру реального кода, то я бы не согласился с тем что это утечка, т.к. это уже конец мэйна - программа завершена. Опять же можно спорить об определениях и эта "утечка" может мешать анализу настоящих утечек с помощью дебажного алакатора, но по факту конкретно в этом случае это не утечка. Если уж совсем дотошно, то с точки зрения самого с++ это вообще не утечка при любом раскладе, т.к. утечкой язык называет аллоцированную память на которую нет ни единого валидного указателя, а тут он есть.
Так они своим тестовым кодом воспроизводят не (возможную) утечку памяти, а якобы ложное срабатывание статического анализатора кода. Будет ли там реально утечка памяти, статический анализатор вам не определит.
Что вы хотите от синтетического примера? :) Его смысл показать, что предупреждение показалось ложным, а оно не ложное.
Естественно, в реальном приложении, всё это где-то глубоко в потрохах. И утечка памяти самая настоящая. Естественно, реальная программа не заканчивает сразу после этого работу, как в моём примере :).
del
Пишу на C# и у нас на проекте принято соглашение не использовать восклицательный знак, т.к. его не видно. Пишем такую вот, на первый взгляд, странную конструкцию:
if(project.IsEnabled == false)
{
//Что-то делаем
}Сначала непривычно, но быстро привыкаешь. Ищешь отрицание в конце, а не в начале.
Кроме малозаметности, ещё вижу проблему с отрицаниями через восклицательный знак. Когда последовательно читаешь код, нужно держать в голове это отрицание по доберёшься до того, что отрицаешь.
Не страна.город.мэрия.зачем.так.много.вложенности.удалена //удалена что? а! не удалена
vs
страна.город.мэрия.зачем.так.много.вложенности.удалена отрицаем удаление // теперь не читается как предложение, зато понятно о каком свойстве идёт речь.
Признайтесь, у вас KPI по количеству кода?)
К счастью, нет (:
Я вот что ещё понял: мы обычно сравниваем одни значения с другими и только булы особенные: с ними либо ничего не делают, либо отрицают (C#). Т.е. если увидел булевую переменную в выражении, будь любезен сделай глазами влево, чтоб узнать есть там восклицательный знак или нет (и ты его точно не пропустил).
Если использовать сравнение через `== false` вместо отрицания, то ответ на вопрос, было отрицание или нет, всегда будет находится справа от переменной. Лично мне так оказалось удобнее.
Как вам повезло, что Вы не застали в естественном ареале обитания такую дичь, как операторы true и false в дополнение к приведению к bool для пользовательских типов.
Hidden text
The true operator returns the bool value true to indicate that its operand is definitely true. The false operator returns the bool value true to indicate that its operand is definitely false. The true and false operators aren't guaranteed to complement each other. That is, both the true and false operator might return the bool value false for the same operand. If a type defines one of the two operators, it must also define another operator.
if (project.IsEnabled.ToString().Length > 4)
:)
Пишем такую вот, на первый взгляд, странную конструкцию:
Она странная вовсе не на первый взгляд. Обычный признак джуниоров, ещё не привыкших к чтению кода
Попробуйте провести эксперимент с коллегами с pattern matching-ом вида
if (project is {IsEnabled: false})
И появляется дополнительная проверка на то, что проект не null. Возникает вопрос: это специально или побочный эффект?
В данном случае я бы не стал пользоваться паттерн матчингом, т.к. проще не становится.
Ну, если у вас там и так нету NRE и JIT в этом уверен, то вы за это не заплатите.
В данном случае семантика не меняется
Согласен, JIT убирает проверку, если точно уверен, например, если экземпляр создали через new. Но чаще значение мы получаем из функции или из параметра и для таки переменных проверка всё равно будет, хоть переменная и не nullable.
Для параметра видно 2 джампа на L0028.
Но это всё экономия на спичках
А если посмотреть использование этого метода, то видно, как инлайнер отрабатывает
Примерно из таких соображений в Котлине к коллекциям прикрутили метод расширение isNotEmpty в дополнение к стандартному isEmpty, чтобы вероятность ошибиться была меньше.
Не страна.город.мэрия.зачем.так.много.вложенности.удалена //удалена что? а! не удалена
Если у вас такое большое обращение к вложенном свойству, то оно так или иначе читается тяжело.
Одинаково плохо читается и это:
if (страна.город.мэрия.зачем.так.много.вложенности.удалена == false)и это:
if (not страна.город.мэрия.зачем.так.много.вложенности.удалена)
Если в первом случае отрицание далеко от непосредственного контекста, то во втором оно не на том месте и отделено от него синтаксическим шумом. А теперь просто сравните с
if (страна.город.мэрия.зачем.так.много.вложенности.неУдалена())Может, настало время добавить немного выразительности в интерфейсы своих типов?
Как альтернатива, когда типы трогать не хочется, воспользуйтесь древней технологией — переменной:
auto&& foo = страна.город.мэрия.зачем.так.много.вложенности;
if (not foo.удалена())Да интересная ошибка, с одной стороны все очень просто, а с другой стороны вот из таких простых ошибок все проблемы и складываются.
Тут люди делились грамотными методами написания кода, которые позволяют таких ошибок избежать, мне всегда был близок следующий подход:
bool PointerIsDefined = (bool) SomeGlobalPointer;
if(PointerIsDefined){
delete [] SomeGlobalPointer;
}Т.е. в имени переменной заключается текстом что конкретно проверяется в условном выражении. В случае из статьи это не так принципиально, но когда условий несколько и они сложно сочетаются очень удобно прямо текстом написать что каждое из условий проверяет.
И конечно здорово, что среда разработки уже умеет находить подобные проблемы.
Наверное, лучше const bool, или даже так:
const bool PointerIsDefined = (nullptr != SomeGlobalPointer);А то остается вероятность, что значение переменной PointerIsDefined где-то по коду могут изменить. Но еще остается возможность, что bool не изменится, а сам указатель изменится. ИМХО, нужно как следует подумать, разнося одну сущность в две разных переменных. Особенно, если они куда-то еще передаются.
Согласен с вами const в данном случае уместен и скорее всего поможет оптимизатору принять правильное решение. Может быть даже constexpr.
Что касается сравнения с nullptr это обсуждали выше в комментариях.
Бесспорно, раздвоение сущностей — путь к ошибкам. Однако, разделение условий на понятные составляющие может исключить ошибки и поспособствовать при проверке.
Эти ваши безопасные джавы мигрировали с одной бд на другую и вылетали и набили дублетов в новую базу, потому что разрабы не рассчитали память ОЗУ потребляемую и был java out of memory
Иллюстрации к статьям просто бомба!
Неожиданное продолжение про простую ошибку разыменования нулевого указателя: FreeCAD и C++ код с неопределённым поведением для медитации (схожий случай).
Ошибка настолько проста, что программисты её не замечают