Когда я только начинал программировать, хотел бы я, чтобы тогда нашёлся кто-то, кто мог бы рассказать об основных подводных камнях, с которыми я столкнусь при создании моего первого сайта.
Тогда одной из проблем было чрезмерное использование else при написании условных выражений. Сегодня я сам постоянно вижу, как в ту же ловушку попадают другие люди, поэтому решил написать этот пост.
Дисклеймер: нижеизложенное — исключительно моё субъективное мнение.
Приём, о котором я собираюсь рассказать, работает не всегда и иногда использовать else всё-таки придётся. В остальных случаях отказ от else поможет сделать код чище.
Охранные выражения
Согласно Википедии, охранное выражение («guard clause») — предварительная проверка неких условий с целью избежать ошибок позже.
Давайте разберём вышенаписанное на пальцах. Итак, мы выполняем проверку неких условий (где-то в начале кода) чтобы не иметь дела с ошибками во время движения потока исполнения программы.
В идеале мы бы хотели, чтобы основная логика программы была расположена после всех проверок данных.
Давайте рассмотрим такой пример: сайт, где один из разделов доступен только премиум клиентам и только после 12 часов.
<?php
if ($user != null) {
if (time() >= strtotime('12 pm')) {
if ($user->hasAccess(UserType.PREMIUM)) {
if ($store->hasItemsInStock()) {
// Раздел доступен.
} else {
return 'We are completely sold out.';
}
} else {
return 'You do not have premium access to our website.';
}
} else {
return 'This section is not opened before 12PM';
}
} else {
return 'You are not signed in.';
}
В реальной жизни, мы скорее всего будем кидать исключения.
Хотя подход выше и выполняет задачу, уже трудно понять к какому конкретно условному выражению относится каждое else, хотя их пока всего несколько штук.
Можно представить, что начнётся для случая с гораздо более сложной логикой. Код станет совсем неподдерживаемым, так что надо искать альтернативный подход.
На помощь приходят охранные выражения:
<?php
if (condition1()) {
return ...;
}
if (condition2()) {
return ...;
}
// Входные данные в порядке.
doSomething();Перепишем код нашего примера используя охранные выражения:
<?php
if ($user == null) {
return 'You are not signed in.';
}
if (time() < strtotime('12 pm')) {
return 'This section is not opened before 12PM';
}
if (!$user->hasAccess(UserType.PREMIUM)) {
return 'You do not have premium access to our website';
}
if (!$store->hasItemsInStock()) {
return 'We are completely sold out.';
}
// Раздел доступен.При использовании охранных выражений, мы обычно инвертируем условия, которые проверяем. Т.е. если раньше мы проверяли, что пользователь залогинен, теперь будем проверять, что он НЕ залогинен.
Новый подход всё так же решает задачу, но получившийся код гораздо чище и понятнее.
Заключение
При создании программы, постоянно спрашивайте себя: «Насколько легко будет её изменить через 6 месяцев?»
Всегда приятно решить проблему быстро. Но написание кода без мыслей о том, как его поддерживать в будущем, просто безответственно.
Использование охранных выражений позволит вам и вашим коллегам модифицировать проверку данных быстрее и легче, когда это потребуется.
Комментарии (163)
peresada
02.12.2019 10:05+7Тема довольно изъезжена.Складывается ощущение, что автор выбрал не ту причину, так как else тут совсем не причем (уберите блоки else из примеров и ничего не изменится)
Решение излишней вложенности скорее относится к концепции раннего возврата, а не к защитным условиям
Например, использовать это вариант
foreach ($list as $element) { if (!$condition) { continue; } // логика }
Вместо
foreach ($list as $element) { if ($condition) { // логика } }
Ну и те примеры, которые предложены автором
LireinCore
02.12.2019 10:29Интересно было бы посмотреть на более сложные примеры, когда условия взаимозависимы и вычисление условий ресурсоёмко (например требует запроса в БД)
SbWereWolf
02.12.2019 13:57-9Какой бы сложности пример не был, на каждую проверку заводим флаг ($flag) и проверяем. Слова «каждую» не надо понимать буквально, можно один и тот же флаг проверять по 10 раз, можно его переиспользовать.
$flag = proverka1(); if(!$flag){ /* если не выполнилась проверка 1 */} $flag2 = false; if($flag) { /* делаем шаг 1 */ $flag2= proverk2();} if($flag1 && !$flag2){ /* если должны были сделать шаг 1 , но проверка 2 не прошла */} if($flag2){ /* делаем шаг 2, потому что проверка 2 успешная и потому что сделали шаг 1 */}
Ниже пример боевого кода, из которого убрана большая часть логики, но оставлен скелет из if-ов, как видите код без ветвлений совершенно и все проверки на флагах.
public function publishOne($identity) { $output = ['success' => false]; $response = CIBlockElement::GetByID($identity); $isReadSuccess = BitrixOrm::isRequestSuccess($response); if (!$isReadSuccess) { $output['message'] = 'Fail request construction'; } $construct = false; if ($isReadSuccess) { $construct = $response->Fetch(); } $isConstructFound = BitrixOrm::isFetchSuccess($construct); if ($isReadSuccess && !$isConstructFound) { $output['message'] = 'Construction not found'; } $isExistsChild = false; if ($isConstructFound) { $response = CIBlockElement::GetList([], $filter, false, false, $select); $isExistsChild = BitrixOrm::isRequestSuccess($response); } $child = false; if ($isExistsChild) { $child = $response->Fetch(); } $gotChild = BitrixOrm::isFetchSuccess($child); $childId = 0; $childPermit = 0; $gotPublicPermit = false; if ($gotChild) { $childId = $child['ID']; $childPermit = $child['PROPERTY_PERMIT_OF_AD_VALUE']; $gotPublicPermit = !empty($childPermit); } $isSuccessDelete = false; if ($gotPublicPermit) { $isSuccessDelete = CIBlockElement::Delete($childPermit); } if ($gotPublicPermit && !$isSuccessDelete) { $output['message'] = 'Fail delete published permit;'; } if ($gotPublicPermit && $isSuccessDelete) { $output['message'] = 'Success delete published permit;'; } if ($gotChild) { $isSuccessDelete = CIBlockElement::Delete($childId); } if ($gotChild && !$isSuccessDelete) { $output['message'] = 'Fail delete published permit;'; } if ($gotChild && $isSuccessDelete) { $output['message'] = 'Success delete published permit;'; } $source = []; $published = 0; if ($isConstructFound) { $source = $construct; $published = (new CIBlockElement())->Add($construct); } $hasCopy = !empty($published); if ($isConstructFound && !$hasCopy) { $output['message'] = ' Fail copying of construction;'; } $values = []; if ($hasCopy) { $output['success'] = true; $output['message'] = ' Success copying of construction;'; CIBlockElement::GetPropertyValuesArray($values, $source['IBLOCK_ID'], $filter, [], ['GET_RAW_DATA' => 'Y']); } $gotValues = !empty($values); if ($hasCopy && !$gotValues) { $output['message'] = ' Fail read construction properties'; } $properties = []; if ($gotValues) { $values = current($values); foreach ($values as $key => $value) { if (!empty($value['VALUE'])) { $properties[$key] = $value['VALUE']; } } $properties['original'] = $identity; } $answer = null; $hasPermit = !empty($properties) && !empty($properties['permit_of_ad']); if ($hasPermit) { $permitId = (int)$properties['permit_of_ad']; $answer = CIBlockElement::GetByID($permitId); } $hasAnswer = !empty($answer) && $answer->result !== false; if ($hasPermit && !$hasAnswer) { $output['message'] = 'Fail read permit'; } $permit = []; if ($hasAnswer) { $permit = $answer->Fetch(); } $publishedPermit = 0; $gotPermit = !empty($permit) && $permit !== false; if ($gotPermit) { $publishedPermit = static::copyElement($permit, $this->pubPermits); } if ($gotPermit && empty($publishedPermit)) { $output['success'] = false; $output['message'] = 'Fail copying of permit'; } if ($gotPermit && !empty($publishedPermit)) { $output['message'] = ' Success copying of permit'; } if ($gotPermit && !empty($publishedPermit) && !empty($properties)) { $properties['permit_of_ad'] = $publishedPermit; } if (!empty($properties)) { CIBlockElement::SetPropertyValuesEx($published, $this->pubConstructs->getBlock(), $properties, ['NewElement' => true]); } return $output; }
KIVagant
02.12.2019 15:06+3> из которого убрана большая часть логики
Битрикс… Как вспомню так вздрогну.peresada
02.12.2019 15:20Битрикс уже стал именем нарицательным и его следует так же внести в антипаттерны
SbWereWolf
02.12.2019 16:20Особенно когда исполняешь работы по гос контракту, а в гос закупке чёрным по белому написано с использованием Битрикс Управление Сайтом.
И ты ещё такой в курсе что закупку уже один раз провели, но прошлый раз разработчики сделали без Битрикса, и у них работу просто не приняли как не соответствующую ТЗ, поэтому закупку провели повторно.
И ты такой директору говоришь «Битрикс это антипаттерн, не будем его использовать» :)peresada
02.12.2019 16:22+6Работать программистом в госструктуре или выполнять гос заказ в нашей стране тоже является антипаттерном, к сожалению
symbix
02.12.2019 18:10+6Рабский труд запрещен Женевской конвенцией. Если вас заставляют этим заниматься, моргните два раза.
symbix
02.12.2019 18:11+1Кстати, очень интересно, что по вопросу госзакупок, навязывающих продукцию конкретного вендора, думает ФАС.
taliban
02.12.2019 19:34Битрикс не позволяет группировать код по функциям? Или ретурны раньше делать а не в самом конце?
tendium
02.12.2019 15:07Интересно, сколько тестов у вас написано на этот код?
vanxant
02.12.2019 16:41+5код не мой, но я угадаю количество тестов с одного раза:)
SbWereWolf
02.12.2019 17:55-2Для условно линейного кода тесты написать попроще чем для кода с вложенными ветвлениями.
И конечно покрытие тестами в помощь.
Конкретно этому коду от силы две недели, при чём в прошлую пятницу заказчику показали релиз, и всё понравилось, а в четверг мы подписываем акт приёмки и ни какой дальнейшей разработки не будет.
Смысл тесты писать? для фирмы которая через *цать лет наш код будет дорабатывать? благородное конечно дело, но мой директор не одобрит.psycho-coder
02.12.2019 18:13Так ведь тесты для себя.
SbWereWolf
02.12.2019 18:58+1Для себя я пилю два проектика и там у меня код коверэйдж 100%.
При чём во втором строчек побольше чем в этом и логика поинтересней.avost
02.12.2019 21:54-1При чём во втором строчек побольше чем в этом
Побольше строчек в одном методе? Это круто, да.
и логика поинтересней.
Спасибо, я и этот ваш говнокодище теперь развидеть не могу, а если там ещё "поинтереснее"…
tendium
02.12.2019 20:57в четверг мы подписываем акт приёмки и ни какой дальнейшей разработки не будет.
Смысл тесты писать? для фирмы которая через *цать лет наш код будет дорабатывать?Я б не смог работать в таких условиях… Но так или иначе, зачем приводить тут как образец кусок одноразового кода?
SbWereWolf
02.12.2019 22:45-2Хороший пример как можно писать без ветвлений.
Понятно что его можно декомпозировать на 4 части, но я тут не учу SOLID-у, вот даже такой стиль написания кода он сам способствует декомпозиции, сделали — проверили, сделали — проверили, каждую пару выполнения шага и обработки результата можно в отдельный метод кидать. Но.
Началось всё с того, что товарищ попросил пример кода, я ему пример кода привёл, понабежали чудаки которые стали судить код вообще с других позиций, смотреть на него со своей колокольни, развели оффтоп, а минусов я отхватил :)
Если кто то не понимает что такое эффективность, что такое требования по качеству и требования по времени разработки, то успехов этим людям с производством ПО.
Chaos_Optima
04.12.2019 12:29Я так понимаю у этого кода требования по качеству были ниже некуда?
SbWereWolf
04.12.2019 19:02Код предельно гранулирован, шаги можно легко менять местами, можно выкидывать, можно добавлять новые, для этого надо только подправить проверку до вставки кода и поправить проверку после вставки кода.
Учитывая что проверки на флагах, сделать это легко.
Скажите пожалуйста, что вас не устраивает в этом коде?
Количество строк в методе?
ок, код не декомпозирован и на то есть причины, описанные в моих коментах, что следующее?
Использование статических методов?
это методы Битрикса, мне свою обёртку на них написать надо?
Что вам ещё не по душе? давайте проведём код ревью, с учётом того что на метод и его полную отладку у нас одни сутки, выйти за этот бюджет времени нельзя.Chaos_Optima
05.12.2019 18:18Всё просто, это спагетти код. Код не читаемый вообще, приходит новый человек и как ему этот код поддерживать и расширять? Откуда он узнает что какие-то переменные используются только в ограниченном месте и дальше не используются, и какие из них можно переиспользовать и не будет ли конфликтов? Как он узнает, что если он вставит какой то блок в определённое место это не сломает ничего? Правильно для этого ему сначала потребуется полностью разобраться в коде функции, для этого потребуется проследить логику и переменные, что уже гемор. Что вам помешало выделить отдельные куски в функции, это вообще никак не повлияет на скорость написания кода, зато многократно повысит читаемость, расширяемость, и поддержку.
Код предельно гранулирован, шаги можно легко менять местами, можно выкидывать, можно добавлять новые, для этого надо только подправить проверку до вставки кода и поправить проверку после вставки кода.
Только для вас, и только если вы помните этот код. В случае работы с другими людьми или если этот код придётся править через некоторое время это будет жуткий геморрой. Даже для джуна это ужасный код.
SpiderEkb
02.12.2019 10:48-2Множественные точки выхода из процедуры не есть хорошо. Мало того, что логика становится менее наглядной, так еще и не дай бог придется что-то делать на выходе…
Хорошо, если язык позволяет конструкции типа on-exit, а если нет?
Строго говоря, любая процедура должна иметь единственную точку входа и единственную точку выхода. Тогда ее логика понятна и прозрачна.
А уж как реализовать множественные проверки… Тут надо включить мозг и подумать.
webkumo
02.12.2019 10:52+1Действия на выходе (если они вдруг нужны — на практике это редкость) выполняются функцией, в которую вкладывается fail-fast функция. Не надо городить антипаттерны из-за какой-то мифической фигни.
SpiderEkb
02.12.2019 11:00Ок. Вам виднее. Видимо, в Вашей практике действия на выходе не нужны. И проще увеличить стек вызовов еще одной функцией, чем корректно все разрулить в текущей.
webkumo
02.12.2019 11:21+1Мне вот интересно, вы на кристалле (МК) разработку ведёте или на платформах (веб/системное) у которых стек вполне себе приличный (мне вот ни разу без рекурсии его выбрать не удалось)? Себестоимость входа в ещё одну функцию так же заведомо на пару порядков ниже предложенных мельком в статье исключений.
SpiderEkb
02.12.2019 11:51-5Не поверите — глубокий backend на платформе IBM i — мейнфреймы IBM PowerSystem 824 (тестовый) и 828 (боевой).
Каждый продукт проходит 4 уровня тестирования — компонентное, интеграционное, нагрузочное и бизнес.
И на нагрузочном тестировании зачатую не проходят более безобидные на первый взгляд вещи чем лишний уровень стека — лишнее переоткрытие файла, лишнее чтение из dtaara, лишние операции с датой-временем, лишний запуск/остановка фонового процесса (submitted job)
То, что вы своим кодом не можете выбрать стек ничего не значит. Скорее всего, пока работает одна ваша задача все просто замечательно. До тех пор, пока ваша задача не становится одной из тысяч, работающих одновременно. А вот когда в пике загрузка сервера переваливает за 90% (а то и 95%), начинаются изыскания на тему где и как можно скроить и кто жрет лишнего.
В общем, мне лень спорить по пустому и рассказывать азы. Давно живу, повидал много на самых разных платформах.webkumo
02.12.2019 12:01+8более безобидные на первый взгляд вещи чем лишний уровень стека — лишнее переоткрытие файла, лишнее чтение из dtaara, лишние операции с датой-временем, лишний запуск/остановка фонового процесса (submitted job)
Вот вы сейчас прям страшно насмешили. Не знаю как в вашем языке, но в java работа со временем — очень дорогая операция. Не знаю, что такое dtaara, но переоткрытие файла — это работа с ФС ОС (которая даже буферизированная, без прямого обращения к диску — будет безумно дорогой). Что уж говорить про создание нитки или процесса (не знаю уж, эквиваленцией чего является ваш submitted job). И вы вот все эти тяжеловесные операции сравнили с уходом по стеку на один уровнь глубже! Да, разворачивать стек — тяжёлая, дорогая операция (именно этим плохи исключения — они буферизируют стек и могут ещё и разворачивать его для вывода ошибки). А вот войти в ещё одну функцию — что-то я сомневаюсь, что у вас настолько плотные математические алгоритмы. В общем — начните оптимизацию с чего-то более полезного (читайте частого) и ресурсоёмкого, а потом мне говорите бред про азы.
serbod
02.12.2019 13:05-2Дамп стектрейса при сбое давно изучали?
webkumo
02.12.2019 14:01+3А можно комментировать так, чтобы не приходилось догадываться? Если вы считаете, что стектрейс тяжёлая операция, то я с вами согласен:
Да, разворачивать стек — тяжёлая, дорогая операция (именно этим плохи исключения — они буферизируют стек и могут ещё и разворачивать его для вывода ошибки).
Если нет, то поищити бенчмарки для вашего языка.
PS "дамп стектрейса" даже звучит ужасно. Есть стектрейс (трассировка стека, развёрный стек). Есть дамп памяти. Вывод развёрнутого стека в лог — не есть дамп.
serbod
02.12.2019 15:04-1Дело не в формате стекстрейса, а в его размере, глубине. И это сильно влияет на качество и скорость отладки, а также на устойчивость системы при незначительных сбоях.
Например. Настолькое приложение. Форма ввода с кнопкой ОК, выполняются какие-то проверки, вычисления, записи, чтения, и в итоге выполняется COMMIT в базу данных. Если происходит исключение где-то между ОК и COMMIT, то оно всплывает аж на уровень кнопки ОК, то есть обработчика событий GUI. И не факт, что перехватывается и обрабатывается должным образом. В результате, в программе проблема, но о ней могут не догадываться долгое время, а на поиск и исправление уйдет уйма времени.
А если внутренняя архитектура разбита на несколько коротих «конвееров» команд, то все намного проще и надежнее. Например, нажатие ОК не выполняет какой-то код, а только кладет команду и данные формы в очередь. Отдельный обработчик берет данные из очереди, проверяет и передает в следующую очередь. Затем следующий обработчик берет данные из очереди, производит вычисления, помещает результат в следующую очередь. Казалось бы, так все намного сложней и медленней. Но на деле, такая «микросервисная» архитектура намного проще в отладке, устойчивей и производительней на многозадачном железе. Исключение всегда всплывает на уровень консьюмера конкретного конвеера, значительно уменьшая размер контекста, сужая область и глубину поиска причины сбоя. Особенно ценно это в хардкорном продакшене, где нет доступа для отладки и приходится работать по продиктованному голосом адресу исключения. Даже не всегда есть возможность посмотреть логи и стектрейс.mayorovp
02.12.2019 15:13Вы написали полную чушь.
Во-первых, для того, чтобы поймать исключение вовремя, совершенно нет необходимости делать конвейеры и очереди сообщений, достаточно написать try-catch!
Во-вторых, начиналось это обсуждение с того, что разбиение одной функции на две — якобы очень затратное изменение. Но всё, что вы написали, никоим образом не зависит от числа функций; ведь число точек, где ловятся исключения, от подобных рефакторингов-то не меняются!
serbod
02.12.2019 15:32Угу. А чтобы избежать ошибок достаточно их не совершать. Все просто! =) Но ошибку все-таки совершили и catch в нужном месте не поставили. Да и не всегда try… catch помогает поймать проблему, обычно ловится уже последствие, факт поломки. Например, когда образовался dead pointer, вроде все работает, но ломается где-то при закрытии программы. Или в конце «всплытия» из длинной цепочки вызовов. А ограничивая длину цепочки вызова, гораздо проще локализовать проблему.
mayorovp
02.12.2019 15:34+1Да с чего проще-то?
serbod
02.12.2019 16:09Меньше вложенных функций приходится изучать и держать их контекст в голове.
mayorovp
02.12.2019 16:24+1А как из одного следует другое?
serbod
04.12.2019 13:38Ниже ответил, на примере классического backend: habr.com/ru/post/478158/#comment_20963624
kacetal
03.12.2019 03:35+1Вот после таких деятелей, боящихся уровней стека, приходится дебажить методы на 15к строк кода с одним ретерном в конце.
webkumo
03.12.2019 13:38Понимаете в чём дело… это от арихтектуры зависит. Возьмём стектрейс в java: он может быть суммарно и на 1000 и на большее количество строк, но
- точка происшествия на самом верху. Возможен последовательный проброс исключений (это когда более "хардовое" ловят и выкидывают более общее, в этом случае вываливается цепочка исключений)
- он хорошо структурирован и поиск по нему не вызывает проблем
- стектрейс и дебаг вот вообще никак не связаны. Стектрейс описывает проблему и выдаёт данные, которые догодался запросить программист (в виде сообщения исключения).
А вот дебаг на нескольких потоках — это знатное такое извращение. Не то, чтобы сложно, но как минимум неудобно. А в одном потоке — цепочку брекпоинтов, если очень надо — вотчеров натыкал и смотришь, что происходит в конкретных точках...
Если происходит исключение где-то между ОК и COMMIT, то оно всплывает аж на уровень кнопки ОК, то есть обработчика событий GUI. И не факт, что перехватывается и обрабатывается должным образом. В результате, в программе проблема, но о ней могут не догадываться долгое время, а на поиск и исправление уйдет уйма времени.
В java все исключения UI-ных приложений, вываливающиеся в поток управления UI роняют приложение и попадают в лог. У вас как-раз таки проблема, что UI отделён в отдельный поток и не узнаёт о проблеме корректно. Ну и вы предлагаете всё это ещё больше усугубить.
Но на деле, такая «микросервисная» архитектура намного проще в отладке, устойчивей и производительней на многозадачном железе.
Наивные мечтанья — синхронизация данных между несколькими потоками штука нифига не бесплатная. Распределённая архитектура (микросервисы) хороша при очень лимитированном общении сервисов друг с другом. В такой ситуации и ethernet не становится преградой.
Исключение всегда всплывает на уровень консьюмера конкретного конвеера, значительно уменьшая размер контекста, сужая область и глубину поиска причины сбоя. Особенно ценно это в хардкорном продакшене, где нет доступа для отладки и приходится работать по продиктованному голосом адресу исключения. Даже не всегда есть возможность посмотреть логи и стектрейс.
Пользуйтесь языками, где архитектура предполагает внятные стектрейсы и логи. Тогда не придётся гадать. И да я знаю, что подробная инорфмация о текущем состоянии энтерпрайзной системы будет закрыта от разработчиков (из соображения приватности продовых данных), но и неприватной информации можно извлечь достаточно, чтобы хотья бы примерно понимать, что же там произошло.
serbod
04.12.2019 13:28-1Согласен насчет java, там все хорошо с отладкой. А что вы называете архитектурой? Свойства языка, железа или дизайн программы? Судя по дальнейшему тексту — свойства языка, платформы разработки.
стектрейс и дебаг вот вообще никак не связаны. Стектрейс описывает проблему и выдаёт данные, которые догодался запросить программист (в виде сообщения исключения)
Стектрейс это часть дебага, важный инструмент. Без него все намного сложнее. Позволяет видеть истоки проблемы, откуда она пришла.
Насчет отладки распределенной архитектуры. Простой пример: отдельно сервис (или сервер) БД, сервис HTTP и сервис приложения. В этом случае не нужно забивать голову деталями БД и HTTP при отладке приложения, это отдельные «черные ящики». В случае сбоя в БД или HTTP достаточно перезапустить только отдельный сервис, и область поиска причины сбоя значительно сужается. А представьте, что будет, если HTTP и БД обрабатываются в одном приложении, в одной цепочке вызовов? Малейший сбой уронит всю систему. Какой бы хороший ни был язык программирования, проблема будет в дизайне системы.webkumo
04.12.2019 14:47А что вы называете архитектурой?
Пользуйтесь языками, где архитектура предполагает внятные стектрейсы и логи.
Вроде и так всё предельно ясно написал. Есть язык, у него есть "платформа" и "окружение". "Окружение" — фактически среда исполнения. для системных языков это ОС, для jvm-запускаемых это, собственно, jvm, для php — это интерпретатор php + сервис в котором он крутится (если он не самостоятельный). "Платформа" — доступный SDK, библиотеки и компилятор/транслятор. Насколько мне известно, получить вменяемый стектрейс (с указанием строки в файле, где произошло исключение) для того же c++ задача не самая тривиальная. А для java это — базовый функционал.
Стектрейс это часть дебага, важный инструмент. Без него все намного сложнее. Позволяет видеть истоки проблемы, откуда она пришла.
Стектрейсы это часть логов. Да это важный инструмент, но к дебагу он относится никак. При грамотно реализованном логировании исключений (и отсутствию проблем с логированием внутреннего состояния… что обязательно будет, если работаешь с приватными данными) дебаг может и не понадобиться. Дебаг нужен, когда не удаётся эмпирически понять "а что же тут происходит-то вообще".
Насчет отладки распределенной архитектуры. Простой пример: отдельно сервис (или сервер) БД, сервис HTTP и сервис приложения. В этом случае не нужно забивать голову деталями БД и HTTP при отладке приложения, это отдельные «черные ящики». В случае сбоя в БД или HTTP достаточно перезапустить только отдельный сервис, и область поиска причины сбоя значительно сужается. А представьте, что будет, если HTTP и БД обрабатываются в одном приложении, в одной цепочке вызовов? Малейший сбой уронит всю систему. Какой бы хороший ни был язык программирования, проблема будет в дизайне системы.
Ну вот а теперь следите за руками:
- 3 ваших сервиса хттп, основной и БД.
- ситуация — запись не попадает в БД.
- раз понадобился дебаг — очевидно логов нет вообще (ну для такой простой ситуации иначе не получается)
- включаемся дебагом в сервис БД — а там шквал разных запросов из разных мест сервиса с бизнес-логикой
- включаемся дебагом в сервис с бизнес-логикой, ловим в нужной нам точке — всё работает корректно!
- что делать дальше? А приходится снова ловить БЛ и перед точкой запроса включать дебаг на сервисе базы и тут же отпускать сервис БЛ… после чего среди пары десятков/сотен/… запросов в БД ловить нужный нам. (вы же сами ратовали за модель "очереди сообщений").
И вот это всё — вместо того чтобы просто пройтись в глубину за точку… Я никак не могу назвать это более удобным дебагом.
В случае же сбоя сервиса… ну у jvm всё просто — в большинстве случаев с перезапуском справится контейнер, в отдельных случая — софтверным watch dog-ом. Это если брать вебное приложение. С UI-ным сложнее, но тоже можно что-то попытаться сделать. В пыхе, насколько мне известно, ситуация не хуже — в большинстве случаев падение одного запроса не угробит весь сервис.
serbod
04.12.2019 15:59Напомню, что изначально речь шла о стеке и цепочке вызовов. И я на примере классического backend показал, что длинные цепочки вызовов усложняют отладку.
Вы приводите хороший пример, где запись не попадает в БД. Но зачем-то предлагаете тыкаться в БД дебагом. Это нужно делать в последнюю очередь. Для начала нужно глянуть, что поступает на вход сервиса БД, текст запросов. Да, это традиционно делается через логи. И включить запись логов намного проще, чем влезать в сервис дебагом. Кроме того, нужно еще глянуть, что приходило на вход сервиса бизнес-логики.
То есть, первый уровень отладки — это проверка входящих-выходящих данных каждого сервиса, чтобы определить, в каком сервисе проблема и на каких исходных данных. И уже потом лезть в конкретный сервис, ставя брейкпоинты и ассерты на конкретный тип данных.
В этом случае не придется изучать сотни запросов в дебагере. И не придется каждый раз перезапускать после сеанса отладки всю монолитную систему, достаточно перезапускать только конкретный сервис. Сфабриковать и подсунуть тестовые данные (mock) тоже проще в отдельный компонент системы, чем монолитную систему.webkumo
04.12.2019 17:19Напомню, что изначально речь шла о стеке и цепочке вызовов. И я на примере классического backend показал, что длинные цепочки вызовов усложняют отладку.
Попытались показать. Не получилось. Для java стек-трейсы легко могут быть в 100 и больше вызовов. При этом с ними вполне удобно работать.
Вы приводите хороший пример, где запись не попадает в БД. Но зачем-то предлагаете тыкаться в БД дебагом. Это нужно делать в последнюю очередь. Для начала нужно глянуть, что поступает на вход сервиса БД, текст запросов. Да, это традиционно делается через логи. И включить запись логов намного проще, чем влезать в сервис дебагом. Кроме того, нужно еще глянуть, что приходило на вход сервиса бизнес-логики.
Вы сами предолжили делать разными сервисами. Ну вот сервис работы с БД я и выделил в отдельный. Вот в него я и предлагал тыкаться дебагом "сначала". А если говорить про саму СУБД — то в зависимости от конкретной — разницы смотреть в логах или лезть в дебаг может и не быть, но всё же СУБД — это последняя точка, куда имеет смысл лезть за инфой, сперва лучше убедиться, что остальные корректно отрабатывают.
То есть, первый уровень отладки — это проверка входящих-выходящих данных каждого сервиса, чтобы определить, в каком сервисе проблема и на каких исходных данных. И уже потом лезть в конкретный сервис, ставя брейкпоинты и ассерты на конкретный тип данных.
И будете вы часами разгребать логи. Это нужно, чтобы определиться с изначальной проблемой (посмотреть на логи прода/теста). А дебаг уже должен быть спланирован по конкретным точкам, через которые бизнес-flaw тащит данные и запускает логику.
В этом случае не придется изучать сотни запросов в дебагере. И не придется каждый раз перезапускать после сеанса отладки всю монолитную систему, достаточно перезапускать только конкретный сервис. Сфабриковать и подсунуть тестовые данные (mock) тоже проще в отдельный компонент системы, чем монолитную систему.
В том и дело, что в монолите (хотя правильнее сказать "без конвееров сообщений) в принципе не придётся изучать сотни запросов — тыкаешься в начальную/среднюю точку сломанной бизнес-логики и идёшь дальше по вызовам. Быстро, удобно, комфортно. С конвеерами так не получится — там брекпоинты нужны условные, а они не со всякой платформой эффективно работают. Далее — зачем перезапускать после сеанса — я тоже не понял. А мок-данные и вовсе к дебагу никакого отношения не имеют — это банальный юнит-, компонентный и интеграционный тест (в принципе все уровни пригодятся, да).
kahi4
02.12.2019 12:17+4Любопытства ради, как глубина стека зависит от нагрузки?
serbod
02.12.2019 13:30-3Например, по асинхронному таймеру происходит вызов функции не дожидаясь выхода. При высокой нагрузке такие вызовы накапливаются. Конечно, тут вопросы к проектированию такого решения, но ведь в нормальных условиях все работает.
kahi4
02.12.2019 14:13+3Вот только асинхронные вызовы помещаются в event loop (или его аналоги), а не лежат в стеке. Как результат — отказ эвент лупа, а не проблемы, связанные с глубиной вызовов. Проще говоря — процессор не успевает разгребать события и это понятно, но увеличение глубины стека — меньшее из зол, не имеющее отношение к эвент лупу. Ну понятно, что вызов функции — это сложить 4 (8, 16?) регистров в стек, поменять CP, это все помноженное на косвенную адресацию, что в итоге отъедает процессорное время, я сам лично, заинлайнив один метод в three.js, привел к его (очень незначительному) росту производительности, но речь идет про под-функцию внутри перемножения матриц, которая вызывается миллионы раз на каждый кадр, в остальных случаях в реальных приложениях цена вызова функции крайне мала и скорее будет проседать соединение с базой (я уже молчу про сам неоптимизированный запрос), чем проблемы с вызовом функции.
serbod
02.12.2019 15:19-1Это если классический синхронный event loop и message queue, как в винде. А в Андроиде, например, таймеры и activities стреляют не дожидаясь завершения предыдущего вызова, как будто в новом потоке. Да и JS вроде тоже. В винде проблемы возникали при банальном переключении визуальной иконки по таймеру. Вроде все нормально работает, а через час падает. А когда смотришь через профилировщик, то виден постоянный рост стека и поглощение какого-то ограниченного ресурса, например, дескрипторов в Windows. Потому что где-то стоял WaitForMultipleObjects, который не блокировал event loop, а тупо ждал выхода из какой-то функции.
mayorovp
02.12.2019 15:33+2А в Андроиде, например, таймеры и activities стреляют не дожидаясь завершения предыдущего вызова, как будто в новом потоке.
Про таймеры не знаю, но весь UI там в одном потоке.
Да и JS вроде тоже.
В JS вообще нет потоков. Есть (Web)Workers, но они больше похожи на процессы, ибо обладают своей собственной памятью.
В винде проблемы возникали при банальном переключении визуальной иконки по таймеру. Вроде все нормально работает, а через час падает. А когда смотришь через профилировщик, то виден постоянный рост стека и поглощение какого-то ограниченного ресурса, например, дескрипторов в Windows.
Проблема утечки ресурсов не имеет никакого отношения к асинхронным таймерам. Её при желании можно и в простейшем цикле устроить, так же как и в таймере избежать.
Потому что где-то стоял WaitForMultipleObjects, который не блокировал event loop, а тупо ждал выхода из какой-то функции.
Но WaitForMultipleObjects, будучи вызван в UI потоке, не может не блокировать event loop!
serbod
02.12.2019 16:15Может вы и правы. Когда-то разбирался в тонкостях работы асинхронных вызовов, а сейчас уже забыл. Потому что перестал их использовать и ломать голову над асинхронностью однопоточного по сути приложения.
Deosis
02.12.2019 12:18+1Мне кажется, что вы говорите о сильно разных вещах. Так как запуск процесса ну никак не может быть безобиднее одного дополнительного вызова функции. Хотя бы потому, что сам подразумевает эти вызовы.
SbWereWolf
02.12.2019 15:17Для меня фишка в отладке, когда делаешь дебаг просто ставишь брейк поинт на единственный ретурн в коде и жмёшь F5 и тебе не надо проживать на каждую строчку кода F10 в ожидании ого что сработает какой то ретурн и ты даже не узнаешь почему и что функция / метод вернули в вызывающий код.
Другое удобство в том что бы не прожимать весь код, а нажать F5 и после этого пробежаться глазами по флагам и значениям переменных и быстро понять что к чему, успешно ли отработал код, или были ошибки и тут же посмотреть что функция возвращает.
Мне удобно когда ретурн только один, и меня не смущает использовать флаги, что бы не исполнять код который при «ошибках» исполнять не надоtaliban
02.12.2019 19:43+1Современные иде позволяют указывать много брекпоинтов, и их можно указывать не только в одном месте а раскидать по коду чтоб попасть непосредственно в проблемное место, а не гадать почему ошибка и в какой из 100500 строк она обьявилась
CrazyElf
02.12.2019 11:16+1Ну нет, как-раз таки логика «если что-то не так, то валим быстрей отсюда нафик» (и чем раньше по коду, тем лучше) — она очень наглядная и понятная.
Sabubu
02.12.2019 11:55+1Множественные точки выхода из процедуры не есть хорошо.
Почему? Где технические аргументы? Вы сейчас верующего напоминаете. По моему, так лапша из ифов в разы хуже.
Deosis
02.12.2019 12:23+7Все повторяют как мантру фразу "У функции должна быть одна точка входа и одна точка выхода", хотя изначальный смысл у нее немного другой:
Одна точка входа — не начинать выполнять функцию с середины.
Одна точка выхода — функция должна вернуть управление туда откуда её вызвали.questor
02.12.2019 15:30Звучит логично. А где-нибудь об этом в книжках писалось, можете подсказать? Я вот про изначальный смысл что-то не помню, чтобы где-то читал.
vintage
02.12.2019 12:25+1function getUser( id : any ) { let error : string checks : { if( typeof id !== number ) { error = 'not a number' ; break checks } if( id == val&0 ) { error = 'not an integer' ; break checks } return DB.getNode( id ) as User } throw new Error( error ) }
Revertis
02.12.2019 13:03+3Ага, видел как сишники делают в конце функции метку, а потом делают к ней goto…
Gryphon88
02.12.2019 14:41+2Достаточно удобный on exit для старого языка. switch-case с fallthrough, где идентификаторы case'ов используются как метки для goto. Goto вызывается в проверках для раннего выхода.
Hardcoin
02.12.2019 18:54единственную точку входа и единственную точку выхода. Тогда ее логика понятна и прозрачна.
Эх, если бы. Но нет, единственность точек входа/выхода не является ни необходимым, ни достаточным условием понятности функции.
Если вам приходится в одном критичном месте использовать много уровней вложенности ради экономии одного уровня стека, это исключительный случай.
Любой, кто использует не си, уже тратит "впустую" намного больше ресурсов, чем можно сэкономить на подобных нанооптимизациях.
tendium
04.12.2019 00:46Любой, кто использует не си, уже тратит «впустую» намного больше ресурсов, чем можно сэкономить на подобных нанооптимизациях.
Ну… Есть еще Rust, который у Си может иногда выигрывать.
trawl
02.12.2019 10:51Приём, о котором я собираюсь рассказать, работает не всегда и иногда использовать else всё-таки придётся. В остальных случаях отказ от else поможет сделать код чище.
<?php if (!$pattern->worked()) { $code->useSometimes('else'); } else { $code->doNotUse('else'); }
MrBman
02.12.2019 11:41+1Я конечно не программист и php я тоже не знаю! Однако, почему бы в данном конкретном случае, просто не сделать как-то так?:
if ($user != null && time() >= strtotime('12 pm') && $user->hasAccess(UserType.PREMIUM) && $store->hasItemsInStock()) { // OK } else { // The access is denied! }peresada
02.12.2019 12:04+3Чтобы понять, почему так лучше не делать, попробуйте ответить на следующий вопрос: какую ошибку должно вывести приложение пользователю в случае запрета доступа?
Revertis
02.12.2019 13:02+5«Ууупс! Что-то пошло не так.» :-D
peresada
02.12.2019 13:31+14- Пользователь звонит в техподдержку: Техподдержка, что-то пошло не так
- Техподдержка сообщает в центр мониторинга: Мониторинг, что-то пошло не так!
- Мониторинг в эксплуатацию: Эксплуатация, что-то пошло не так!
- Эксплуатация сообщает программисту: Программист, что-то пошло не так!
- Программист: Сука, знал же, что нужно разделять условия и логировать разные варианты этого «что-то пошло не так»
CrazyElf
02.12.2019 12:17+2К сожалению, множественные логические условия точно так же быстро перестают быть понятными, как и множественный if. А уж если там не только AND, но и OR условия и туча скобок, то это ещё хуже для разбора и понимания, чем много if-ов, хоть и выглядит компактнее.
psycho-coder
02.12.2019 12:57Можно сильно запутаться, как выше писали. А чтобы распутаться, придется строить таблицу истинности.
Virgo_Style
02.12.2019 11:50+2Мне кажется, если собрать воедино все подобные статьи, то придется писать линейные программы. И то не факт.
CrazyElf
02.12.2019 12:19Не совсем так. При грамотном рефакторинге будет меньше условий на одном уровне, но самих уровней вложенности может стать гораздо больше и это тоже может стать проблемой.
Линейная программа тоже не есть гуд, в большой длинной простыне мозгу тоже разбираться ничего хорошего.
haldagan
02.12.2019 12:34+4Все это зашибись, конечно, но в таких случаях как на первой картинке этот метод не сработает по той простой причине, что большинство из этих ошибок ввода обычно необходимо обработать/выдать пользователю одновременно.
Статьи с медиума как всегда «радуют» качеством содержимого. Надо отдать должное автору этой, что он по крайней мере не налил воды для пущего эффекта (если не считать картинок и врезок кода).
Переводчик, а что ж вы самую мякотку-то не перевели из последней части статьи?
It has personally caused me to entirely rewrite whole features from scratch because of how much technical debt I accumulated over time through numerous band-aid fixes.
Джун прозрел и теперь учит всех как правильно. Ну ничего, скоро узнает слово «валидатор» и снова напишет статью.vintage
02.12.2019 12:40+1большинство из этих ошибок ввода обычно необходимо обработать/выдать пользователю одновременно
Это было актуально, когда пользователь взаимодействовал с сервером напрямую через формы. Сейчас он взаимодействует с клиентским приложением, так что серверных ошибок валидации в норме не видит — они лишь сигнализируют, где накосячил разработчик клиентского приложения.
haldagan
02.12.2019 12:50Не совсем понял аргумент.
То есть по-вашему если приложение пропустило невалидные данные на сервер, то пользователю выдается окошко «ой, что-то стряслось, попробуйте позже»?
А если сервер таки отдает приложению информативный фидбек (сервер-то в любом случае данные проверять обязан), то почему фидбеку не быть И информативным И удобным (в смысле вываливать все ошибки валидации, а не по одной)?vintage
02.12.2019 13:06+1Потому, что не надо делать звездолёт, когда нужен трамвай (а звездолёт — это затраты времени, денег, баги и сложности с пониманием).
Потому, что появление больше одной ошибки в данных условиях крайне большая редкость (ибо все они должны были провериться еще на клиенте).
Потому, что часто все остальные ошибки являются следствием первой (например, передано неправильное число цифр кредитки, поэтому контрольная сумма не сошлась).
Потому, что остальные валидации часто невозможны, пока не прошла предыдущая (например, нельзя проверить, что число больше нуля, если передана строка, которую невозможно привести к числу).
Потому, что чинить ошибки разработчик всё-равно будет по очереди (ибо однозадачный).
alekciy
03.12.2019 22:14Люто плюсую! Сразу виден опытный человек которому приходилось копаться в 100500 сгенерированных ошибках появившихся каскадно из-за пары первых.
haldagan
04.12.2019 13:02-1Потому, что часто все остальные ошибки являются следствием первой (например, передано неправильное число цифр кредитки, поэтому контрольная сумма не сошлась)
А зачем дальше проверять валидность УЖЕ невалидного поля? Или у вас в коде именно так, как у автора статьи? Ну то есть:
if (!is_number($card_number)) return "card_number is wrong";// or throw or whatever if (!is_visa($card_number)) return "we accept only visas"; if (!has_valid_checksum($card_number)) return "card number is not valid";
Такую лапшу же читать невозможно если у вас хотя бы с десяток полей и правила валидации чуть сложнее чем «должно содержать цифры»/«должно соответствовать маске».
По остальным пунктам — ну ок, согласен.
alekciy
03.12.2019 22:12Нет, сервер отдает первую ошибку валидации северных данных. А клиентская валидация — все возможные ошибки. Тогда волосы становятся желковистыми, а код дебажиться просто.
vintage
04.12.2019 11:29-2А клиентская валидация — все возможные ошибки.
Тоже не обязательно. Пример: https://mol.js.org/app/demo/-/#demo=mol_form_demo_bids
alekciy
04.12.2019 13:27Не уловил почему «тоже не обязательно» в условиях когда данный пример как раз демонстрирует поведение о котором я говорю.
haldagan
04.12.2019 12:31ошибках появившихся каскадно
Речь о другом шла. Естественно незачем проверять невалидное поле по дальнейшим критериям.
Я ратовал за то, чтобы параллельно проверять остальные поля в этой же итерации, а не дожидаться следующего ввода.alekciy
04.12.2019 13:17чтобы параллельно проверять остальные поля в этой же итерации, а не дожидаться следующего ввода
Давайте для начала определимся про какую валидацию идет речь: клиент, сервер?
Второе. Пользователь еще не дошел до ввода данных Х, мы уже "параллельно проверяем все остальные поля" и говорим ему "данные обязательны для ввода". Это так делать правильно?haldagan
04.12.2019 13:59Мы все еще про сервер.
еще не дошел до ввода данных Х, мы уже «параллельно проверяем все остальные поля»
Я так понял это Вы про клиент?
Если все-таки про сервер — поясните, почему и зачем клиент отправляет заведомо неполный набор данных на сервер?
Если речь идет о multi-step формах с участием сервера, то на сервере валидируется и обрабатывается текущий шаг, а не вся форма целиком.alekciy
04.12.2019 14:32Если мы уже на сервере, то вся валидация на клиенте уже прошла и мы при валидном посыле формы вообще не должны попасть ни на одну из ошибок. Если это не так, то либо продолбали интеграцию с фронтом, либо форма поддельная. В любом из их случаев чем быстрее завершим такой запрос тем лучше потому что есть лигитимные клиенты с которыми нужно работать.
psycho-coder
04.12.2019 15:19Если все-таки про сервер — поясните, почему и зачем клиент отправляет заведомо неполный набор данных на сервер?
Да мало-ли, что произодйет с клиентом, что он решил отправить заведомо не полный набор данных. Или у вас валидация полноты набора данных только на клиенте?
AlNi89
02.12.2019 13:06-15Хотя подход выше и выполняет задачу, уже трудно понять к какому конкретно условному выражению относится каждое else, хотя их пока всего несколько штук.
Может просто начать по-человечески оформлять код, без идиотской привычки ставить открывающую скобочку в одной строке с if?
if ($user != null) { if (time() >= strtotime('12 pm')) { if ($user->hasAccess(UserType.PREMIUM)) { if ($store->hasItemsInStock()) { // Раздел доступен. } else { return 'We are completely sold out.'; } } /* if ($user->hasAccess(UserType.PREMIUM)) */ else { return 'You do not have premium access to our website.'; } }/* if (time() >= strtotime('12 pm')) */ else { return 'This section is not opened before 12PM'; } } /* if ($user != null) */ else { return 'You are not signed in.'; }CrazyElf
02.12.2019 13:19И всё-равно это плохой стиль, когда начало блока и конец блока потенциально могут отстоять друг от друга на расстоянии несколько экранов и надо как-то поддерживать при этом правильные отступы и понимать, в каком месте вносить изменения. Для осознания такого кода его придётся листать туда-сюда неоднократно.
Zenitchik
02.12.2019 13:21Вам нравится в IDE видеть много пустых строчек со скобочками?
AlNi89
02.12.2019 14:33-2Мне нравится удобство чтения кода.
У автора всего 10 строк и уже
трудно понять к какому конкретно условному выражению относится каждое else
Зато по стандарту. Мыши плакали, кололись, но продолжали есть кактус. А мой пример читается легко и наглядно, сколько бы минусов комментарию не поставили.mayorovp
02.12.2019 14:41+1Ну так в вашем варианте что, проще понять к какому условию else относится — или как?
webkumo
02.12.2019 14:50Немного тренировки — и формат "по стандарту" читается легче и быстрее, чем ваш. Ну просто потому, что больше значащего текста помещается на экран, при этом не наплывая друг на друга. Да и в целом — вложенность больше 2х — много где считается code smell. Об этом автор и говорит (правда, почему-то, обвиняя бедные else).
Lodin
02.12.2019 15:05+1На самом деле, имхо, как писать скобочку — вообще не важно. Стандарт — это в первую очередь для того, чтобы вся кодовая база была консистентна, и новичок на проекте не испытывал когнитивный диссонанс каждый раз, когда видит разные стили, и не тратил свой рабочий ресурс на то, чтобы определиться, как именно ему писать. Ну и для того, чтобы использовать fmt, который будут сами стилизовать код.
К примеру, в C# принят подход с переносом скобок на следующую строку во всех случаях — это правило языка. На JS, наоборот, нужно скобки оставлять на той же строке, даже для классов и методов — и это тоже правило языка. И используя эти языки, я буду использовать именно их правила, чтобы любой другой программист, прочитав код, обращал внимание на логику, а не на нестандартное оформление.
webkumo
02.12.2019 15:32А в java есть несколько стандартов (от нескольких крупных игроков) и они различаются.
PS да я согласен, что стандарт это хорошо, просто я пользуюсь языком, где владельцам языка не удалось всех свести под гребёнку одного стандарта. Так что, личная оценка "что удобней" у меня есть. Другое дело, что кому-то действительно нехватает этих отступов, чтобы легче структурировать информацию… и в целом в java форматированием уже существующего кода стараются не увлекаться...
Nimtar
03.12.2019 02:13Интересно, в Java, в отличие от C и упомянутого выше JS, я разногласий по теме не встречал: только египетские.
AlNi89
02.12.2019 15:40-2Немного тренировки — и
и Вы на крючке у когнитивного искажения, Вам кажется что так легче, из-за удобного стандарта, а на само деле Вам легче, потому что было
Немного тренировки
Zenitchik
02.12.2019 14:57трудно понять к какому конкретно условному выражению относится каждое else
Это автор, простите, брешет. Там невооружённым глазом видно, к какому условию относится else. Тупо по отступам.
А если бы он переносил else на следующую строку — IDE помогла бы ему сворачивать код поблочно и видеть строки наподобие с
+if(...){...}
+else{...}
где + — иконка разворачивания.
Собственно из-за этого сугубо практического соображения я считаю, что открывающая фигурная скобка должна быть на одной строке с оператором, а закрывающая — на отдельной строке.questor
02.12.2019 15:37Мнение логичное, хотя и на это есть свои контраргументы.
Когда говорят "код читаем" — это значит, что его можно прочитать взглядом, это не значит, что нужно ещё что-то прокручивать, целиться то в первую раскрывашку, то во вторую или жать какие-то хоткеи.
Хотя, если кто-то умеет силой мысли раскрывать плюсики… </ irony> :)
webkumo
02.12.2019 16:48Фишка в том, что нормальным IDE пофиг на форматирование — они умеют эти плюсики и в сплошном тексте делать. Но я почему-то предпочитаю, чтобы код читался без лишних телодвижений.
Zenitchik
02.12.2019 17:22Плюсики в сплошном тексте — это неудобно. Хочется всё-таки, сворачивание по строкам происходило, от свёрнутого блока оставалась одна строка, по которой можно было понять, что это за блок, а «плюсики» были в начале строки.
Hardcoin
02.12.2019 20:11Разумеется у автора трудно понять. Он же специально сделал несколько вложенных условий. Это антипаттерн и так писать обычно не нужно, тогда и с читабельностью всё нормально будет.
А расставлять скобки в плохо написанном коде ради мифической читабельности — это как лечить мертвого. Тем более что ваш вариант многим людям читабельность ухудшает наоборот. Возможно даже, что почти всем, но верю, что конкретно вам чуть лучше.
Lodin
02.12.2019 13:57+2Есть же PSR, где чёрным по белому написано, как должны оформляться
if-структуры. Зачем изобретать велосипед?AlNi89
02.12.2019 14:21-2Чтобы не писать вот такие комментарии:
Хотя подход выше и выполняет задачу, уже трудно понять к какому конкретно условному выражению относится каждое else
Lodin
02.12.2019 14:33Как будто простой перенос скобки на новую строку как-то решает проблему. Лично для меня что там понятно, что там. Для новичка, скорее всего, будет непонятно в обоих случаях, просто глаз не привык.
А вот нестандартный кодстайл создаёт гораздо больше проблем, чем решает.
sayber
02.12.2019 14:09Может не стоит писать как криворукий младенец а использовать стандарт который принят официально?
Я лично уволю тех, кто будет писать код как в вашем примере.AlNi89
02.12.2019 14:28-4Эффективный менеджер, сразу видно. Сначала научитесь общению с людьми, без хамства, а потом уже увольняйте :)
Теперь понятно, почему нынешний софт такой тормозной: все думают о стандартах, а не удобстве (читаемости в данном случае) для человека…
Lodin
02.12.2019 14:40+1Эффективный менеджер, сразу видно. Сначала научитесь общению с людьми, без хамства, а потом уже увольняйте :)
И это пишет человек, сказавший в своём комментарии:
Может просто начать по-человечески оформлять код, без идиотской привычки ставить открывающую скобочку в одной строке с if?
В своём глазу бревна не видим? :D
Теперь понятно, почему нынешний софт такой тормозной: все думают о стандартах, а не удобстве (читаемости в данном случае) для человека…
Вы прям взаимоисключающие параграфы используете. Стандарты ведь придумали как раз для удобства человека.
А про "тормозной код" даже как-то и сказать нечего: как скорость исполнения кода зависит от стиля написания кода — решительно непонятно.
sayber
02.12.2019 14:45-3Не менеджер я.
Просто главный разработчик, который имеет опыт в программировании более 20 лет.
В команде не потерплю людей который пишет быдлокод. Мне они не нужны, значит просто увольняем.
А делается это просто, говорю начальству, что данный товарищ не подходит.
Не знаю как тормозной, но продукт под моим началом спокойно держит 2000qps.goldrobot
02.12.2019 16:35-1Согласен, быдло которое не уважает глаза других людей и открывает скобки на одной строке с if'ом необходимо уволнять с волчим билетом.
sayber
02.12.2019 16:46Все нормально с глазами.
Есть PSR. Который обсуждается и принимает не один год.
Но есть люди, которые любят делать как им захочется. А затем почему то удивляются, почему их не берут на работу, хотя бы на 200т.р.
goldrobot
03.12.2019 08:57-2Вот вот, и я про тоже. Эти люди незнают даже fuelphp стандарта из-за чего их даже на позицию с зарплатой 350 т.р. не берут. А далее они от обиды идут и за копейки пашут в компании с PSR.
peresada
02.12.2019 14:45+1Стандарты разрабатываются и обсуждаются неделями, месяцами и иногда годами, как раз для удобства и читаемости, основываясь на опыте и реальном использовании языка множества людей, которые и образуют сообщество. Ваше «мне так кажется» в данном случае не котируется
Perlovich
02.12.2019 15:06+1> Стандарты разрабатываются и обсуждаются неделями, месяцами и иногда годами
Мне кажется, что к правилам размещения скобочек это не относится. Например, в C# пишут скобочки как раз на отдельной строке, в отличие от php или Java.
Я думаю, что это вообще не важный и не принципиальный момент. Просто в пределах проекта (или группы проектов) надо придерживаться одинакового код стайла.Lodin
02.12.2019 15:12Я думаю, что это вообще не важный и не принципиальный момент. Просто в пределах проекта (или группы проектов) надо придерживаться одинакового код стайла.
Я бы сказал — в пределах языка. Чтобы, когда читаешь исходники библиотеки, не пришлось дополнительно думать о нестандартном оформлении.
peresada
02.12.2019 15:24Стандарты на то и стандарты, что у них нет понятия «важности». Это всего навсего договоренности между группой людей, и по-моему все договоренности одинаково важны, так как прийти к общему соглашению бывает достаточно трудно.
Текущее обсуждение как раз это и показывает
Если больше углубляться в тему, то кроме удобства-читаемости есть еще инструменты и утилиты для языка, более низкоуровневые. Например, анализаторы коды, снифферы и т.д. Эти инструменты так же пишутся программистами, и им гораздо проще, когда язык стандартизирован. И возможно для них проблема переноса фигурной скобки является более принципиальной, чем для вас
Alexufo
02.12.2019 23:58дьявол, терпеть не могу { c новой строчки :-). Накой эти набития путые раздувания? Нет никакой там читаемойсти.В PSR-0 более менее все обговоренно
tendium
02.12.2019 13:16Приём называется early return. Else тут вторичен. Выгода: меньше влодености, легче анализировать такой код, и как следствие снижение количества потенциальных багов.
dadon
02.12.2019 16:21-2Функция с кучей выходов — ад для отладки, функция с кучей ветвлений — ад для чтения. Истина, как обычно, где-то посередине.
Но улыбает, конечно, когда люди начинают заезжать и бездумно применять какой-то паттерн везде.
Смиритесь люди, иногда без пару else if не обойтись, не создавая какой-нибудь запутанной фигни :)
MaxVetrov
03.12.2019 02:18-1А если так? )
$conds = [ ['return $user != null;', 'You are not signed in.'] ,['return time() >= strtotime(\'12 pm\');', 'This section is not opened before 12PM'] ,['return $user->hasAccess(UserType.PREMIUM);', 'You do not have premium access to our website'] ,['return $store->hasItemsInStock();', 'We are completely sold out.'] ]; foreach($conds as $cond) { if (!eval($cond[0])) { return $cond[1]; } } // Раздел доступен.
skazo4nik
03.12.2019 08:480. Дыра в безопасности, больше требований к фильтрации данных
1. Нет подсветки синтаксиса
2. Нет поддержки рефакторингов в IDE
3.…
mayorovp
03.12.2019 08:49eval на пустом месте — это всегда плохо, хотя бы стрелочные функции используйте в таких случаях.
Но даже с ними мне такой код всё равно не нравится.
Gryphon88
03.12.2019 12:50Кстати, а когда eval нужен, что без него аж грустно? Всегда можно написать немного бойлеплейт кода, а принимать внешние функции для выполнения и файла или по сети как-то совсем некузяво.
peresada
03.12.2019 12:55когда архитектура на нем завязана, в остальном не нужен. Вот бери тот же Modx, там пхп-код в БД хранится. Это ужасно конечно, но такое действительно есть
webkumo
03.12.2019 13:23Реально — он нужен никогда. Но на него действительно могут пытаться завязать дивно-динамическую архитектуру… В принципе и в других языках встречается подобное (в js — прямой аналог и тоже начисто не рекомендуется к использованию, в java — можно на лету сваять или загрузить откуда-нибудь байт-код и загрузить его в jvm, что в принципе уже не является чистым антипаттерном — во-первых с кодом один фиг работают как с кодом, а не текстом, во-вторых случайно такое не сделаешь, там нужно специально постараться… в общем если правильно спроектировать — относительно безопасная вещь).
vanxant
03.12.2019 14:22Я знаю только одно разумное применение — когда колхозят недо-«Excel» с вводом формул юзером вручную и при этом ленятся писать свой парсер + хотят дать доступ к стандартным функциям.
mayorovp
03.12.2019 14:33Не обязательно лениться писать свой парсер. Можно как раз-таки свой парсер написать, далее обработать AST и сгенерировать код, а потом уже сделать eval.
Такой способ может выйти быстрее в случае многократных вычислений по одной и той же формуле, если интерпретатор языка умеет в JIT.
AndrewA
Рефакторинг в помощь! Больше двух вложенных веток — зло! Надо выносить в отдельные функции.
PyVolshebnyi
Подскажите, пожалуйста, что делать с подобным кодом? Такое нечитабельно, но вложенные или сложные условия в разы хуже.
peresada
имхо такое решается каким-нибудь rbac'ом, а не рефакторингом
mayorovp
Да вроде в этом коде всё читабельно (если комментарии убрать).
Сложность этого кода растёт из сложности бизнес-логики, и сильнее упростить его не меняя бизнес-логику не выйдет. Тут остается только трясти заказчика и аналитиков, почему это правило has_children приоритетнее чем !has_active_subscription, и так ли важна проверка is_baba_tanya.
PyVolshebnyi
Комментарии не относятся к коду, я так написал мысли читающего код. Проблема в том что когда у нас более сложные условия в такой вот булевой функции, приятное если в проверках всегда возвращается False, а если все проверки провалены — True, или наоборот. Если проверки возвращают то одно то другое и вдруг функция не дает ожидаемый результат — отладка становится сложнее.
mayorovp
Если у нас более сложные условия — то код можно упростить вынеся в отдельные функции эти самые условия.
AndrewA
да вроде ничего сложного. Как уже написали, убрать комментарии (они только отвлекают), и вполне себе читаемый код.
PyVolshebnyi
Что-то я забыл добавить — комментарии это мысли читающего код.
alekciy
Как минимум делать не возврат болевого значения, а нормальные исключения. В тексте исключения писать причину, тогда комментарии не нужны.
PyVolshebnyi
А это идея. Но проблема тогда появляется в том что либо мы используем глобальные исключения проекта, таким образом наш модуль слишком много знает об окружающем мире. Либо мы кидаем исключения модуля, которые потом надо будет обработать во внешнем мире и завернуть в глобальные или сделать что-то другое.
alekciy
Нет проблемы если прописана архитектура исключений и всегда есть общее для приложения исключения от которого наследуются остальные. Тогда на самом верхнем уровне при желании можно поймать любое исключение совершенно не зная специфичных исключений модулей.
В архитектуру нужно закладывать не только внешние интерфейсы модуля, но исключения которые оно будет генерить. Не все возможные исключения которые в нем есть, а именно та часть, которая может потребоваться наружному коду. Это такая же часть контракта как и интерфесы.
Поэтому проблемы возникают если такое проектирование и такие контракты не создаются.
webkumo
Так себе идея — отвратительно скажется на производительности же. Ну а причины можно енумом (в java) или аналогами (в других языках) отдавать.
alekciy
Причем тут java когда топик про PHP? Упали с исключением и отдали текст ошибки на клиент это нормальная ситуация с допустимой производительностью. Скажу больше, если упали на первой же ошибке то производительность будет выше, потому что приложения уже не нужно ходить ни в базу, в рантайме крутить бизнеслогику.
Если вы в своих приложения мониторите время ответа ваших бэкендов (вы ведь это делайте да?), то легко увидите, что ответы с ошибками отрабатывают быстрее чем нормальный медианный ответ для этого же ресурса.
webkumo
Топик (статья) озаглавлена "про ёлсы", фактически — про фейл-фаст. Примеры — да на пхп. Комментарии — так же идут без жёсткой привязки к одному языку. Ну и да — я сомневаюсь, что реализация исключений в пыхе настолько уж другая, что внезапно мой комментарий превратится в тыкву.
Gryphon88
alekciy
В контексте веба и PHP в условии приложения когда GUI это JS (Angular/React/Vue/jQuery/Native) с полной валидацией на клиенте это нормальное поведение.
webkumo
А давайте спросим нагруженных ребят, которые даже собственные диалекты php пилят, чтобы набрать нужную производительность на имеющемся базисе — нормально ли это. Почему-то мне кажется что они с вами не согласятся.
peresada
При действительно высокой нагрузке много разных нюансов появляется практически везде, но проектов с реально высокой нагрузкой не так уж и много в расчете на количество программистов. А если речь идет о паре-тройке миллионов пользователей в месяц не самого сложного сервиса, то проблема, которую здесь обсуждают, далеко не самая приоритетная
vintage
Это тех, которые наговнокодили кучу кода, а потом вместо того, чтобы разбирать это дерьмо и, например, переписать на боле подходящем для перфоманса языке, пилят костыли к языку?
alekciy
Не вопрос. Зовите их в тред. Пусть ответят, делают ли они так, и почему.
Gryphon88
Опять не понял. Т.е. если код крутится на стороне клиента, производительность не важна?