Прекратите использовать Else в ваших программах +3



Когда я только начинал программировать, хотел бы я, чтобы тогда нашёлся кто-то, кто мог бы рассказать об основных подводных камнях, с которыми я столкнусь при создании моего первого сайта.


Тогда одной из проблем было чрезмерное использование 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 месяцев?»


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


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

Вы можете помочь и перевести немного средств на развитие сайта

Теги:

else


Комментарии (163):

  1. AndrewA
    /#20954282 / +2

    Рефакторинг в помощь! Больше двух вложенных веток — зло! Надо выносить в отдельные функции.

    • PyVolshebnyi
      /#20958472

      Подскажите, пожалуйста, что делать с подобным кодом? Такое нечитабельно, но вложенные или сложные условия в разы хуже.

      def has_access(user) -> bool:
    if not user.is_authenticated():
        return False

    # пока что все понятно, функция проверяет не надо ли пользователю доступ
    # закрыть, и если все проверки провалятся, то она, видимо, вернет True

    if user.is_superuser():
        return True

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

    if user.has_children():
        return False

    # окей, видимо, проверка на суперпользователя была исключением,
    # сейчас все пойдет как надо

    if not user.has_active_subscription():
        return True

    if user.is_baba_tanya():
        return False

    # ААА! Я запутался.

    return True

    # вроде False будет если не залогинен или
    # не суперпользователь или с подпиской? Надо перечитать

      • peresada
        /#20958498 / +1

        имхо такое решается каким-нибудь rbac'ом, а не рефакторингом

      • mayorovp
        /#20958586

        Да вроде в этом коде всё читабельно (если комментарии убрать).


        Сложность этого кода растёт из сложности бизнес-логики, и сильнее упростить его не меняя бизнес-логику не выйдет. Тут остается только трясти заказчика и аналитиков, почему это правило has_children приоритетнее чем !has_active_subscription, и так ли важна проверка is_baba_tanya.

        • PyVolshebnyi
          /#20961764

          Комментарии не относятся к коду, я так написал мысли читающего код. Проблема в том что когда у нас более сложные условия в такой вот булевой функции, приятное если в проверках всегда возвращается False, а если все проверки провалены — True, или наоборот. Если проверки возвращают то одно то другое и вдруг функция не дает ожидаемый результат — отладка становится сложнее.

          • mayorovp
            /#20962266

            Если у нас более сложные условия — то код можно упростить вынеся в отдельные функции эти самые условия.

      • AndrewA
        /#20958650

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

        • PyVolshebnyi
          /#20961756 / -1

          Что-то я забыл добавить — комментарии это мысли читающего код.

      • alekciy
        /#20961580

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

        • PyVolshebnyi
          /#20961780

          А это идея. Но проблема тогда появляется в том что либо мы используем глобальные исключения проекта, таким образом наш модуль слишком много знает об окружающем мире. Либо мы кидаем исключения модуля, которые потом надо будет обработать во внешнем мире и завернуть в глобальные или сделать что-то другое.

          • alekciy
            /#20963656

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

        • webkumo
          /#20962796

          Так себе идея — отвратительно скажется на производительности же. Ну а причины можно енумом (в java) или аналогами (в других языках) отдавать.

          • alekciy
            /#20963598

            Причем тут java когда топик про PHP? Упали с исключением и отдали текст ошибки на клиент это нормальная ситуация с допустимой производительностью. Скажу больше, если упали на первой же ошибке то производительность будет выше, потому что приложения уже не нужно ходить ни в базу, в рантайме крутить бизнеслогику.
            Если вы в своих приложения мониторите время ответа ваших бэкендов (вы ведь это делайте да?), то легко увидите, что ответы с ошибками отрабатывают быстрее чем нормальный медианный ответ для этого же ресурса.

            • webkumo
              /#20963948

              Топик (статья) озаглавлена "про ёлсы", фактически — про фейл-фаст. Примеры — да на пхп. Комментарии — так же идут без жёсткой привязки к одному языку. Ну и да — я сомневаюсь, что реализация исключений в пыхе настолько уж другая, что внезапно мой комментарий превратится в тыкву.

            • Gryphon88
              /#20965744

              Упали с исключением и отдали текст ошибки на клиент это нормальная ситуация с допустимой производительностью.
              Это правда нормальное поведение при обработке пользовательского ввода? Обработка исключений всё-таки довольно дорогая процедура.

              • alekciy
                /#20966464

                В контексте веба и PHP в условии приложения когда GUI это JS (Angular/React/Vue/jQuery/Native) с полной валидацией на клиенте это нормальное поведение.

                • webkumo
                  /#20966722

                  А давайте спросим нагруженных ребят, которые даже собственные диалекты php пилят, чтобы набрать нужную производительность на имеющемся базисе — нормально ли это. Почему-то мне кажется что они с вами не согласятся.

                  • peresada
                    /#20966916

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

                  • vintage
                    /#20966934

                    Это тех, которые наговнокодили кучу кода, а потом вместо того, чтобы разбирать это дерьмо и, например, переписать на боле подходящем для перфоманса языке, пилят костыли к языку?

                  • alekciy
                    /#20967736

                    Не вопрос. Зовите их в тред. Пусть ответят, делают ли они так, и почему.

                • Gryphon88
                  /#20968272

                  Опять не понял. Т.е. если код крутится на стороне клиента, производительность не важна?

  2. peresada
    /#20954284 / +7

    Тема довольно изъезжена.Складывается ощущение, что автор выбрал не ту причину, так как else тут совсем не причем (уберите блоки else из примеров и ничего не изменится)

    Решение излишней вложенности скорее относится к концепции раннего возврата, а не к защитным условиям

    Например, использовать это вариант

    foreach ($list as $element) {
    if (!$condition) {
        continue;
    }
    // логика
}

    Вместо 
    foreach ($list as $element) {
    if ($condition) {
        // логика
    }
}


    Ну и те примеры, которые предложены автором

  3. LireinCore
    /#20954364

    Интересно было бы посмотреть на более сложные примеры, когда условия взаимозависимы и вычисление условий ресурсоёмко (например требует запроса в БД)

    • SbWereWolf
      /#20955432 / -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;
    }

      • vladqa
        /#20955692 / +16

        Боже, какой ад. В этом коде плохо вообще все. Не надо такое показывать.

      • KIVagant
        /#20955808 / +3

        > из которого убрана большая часть логики

        Битрикс… Как вспомню так вздрогну.

        • peresada
          /#20955892

          Битрикс уже стал именем нарицательным и его следует так же внести в антипаттерны

          • SbWereWolf
            /#20956252

            Особенно когда исполняешь работы по гос контракту, а в гос закупке чёрным по белому написано с использованием Битрикс Управление Сайтом.
            И ты ещё такой в курсе что закупку уже один раз провели, но прошлый раз разработчики сделали без Битрикса, и у них работу просто не приняли как не соответствующую ТЗ, поэтому закупку провели повторно.
            И ты такой директору говоришь «Битрикс это антипаттерн, не будем его использовать» :)

            • peresada
              /#20956264 / +6

              Работать программистом в госструктуре или выполнять гос заказ в нашей стране тоже является антипаттерном, к сожалению

            • symbix
              /#20956778 / +6

              Рабский труд запрещен Женевской конвенцией. Если вас заставляют этим заниматься, моргните два раза.

            • symbix
              /#20956788 / +1

              Кстати, очень интересно, что по вопросу госзакупок, навязывающих продукцию конкретного вендора, думает ФАС.

            • taliban
              /#20957144

              Битрикс не позволяет группировать код по функциям? Или ретурны раньше делать а не в самом конце?

      • tendium
        /#20955816

        Интересно, сколько тестов у вас написано на этот код?

        • vanxant
          /#20956344 / +5

          код не мой, но я угадаю количество тестов с одного раза:)

          • SbWereWolf
            /#20956700 / -2

            Для условно линейного кода тесты написать попроще чем для кода с вложенными ветвлениями.
            И конечно покрытие тестами в помощь.
            Конкретно этому коду от силы две недели, при чём в прошлую пятницу заказчику показали релиз, и всё понравилось, а в четверг мы подписываем акт приёмки и ни какой дальнейшей разработки не будет.
            Смысл тесты писать? для фирмы которая через *цать лет наш код будет дорабатывать? благородное конечно дело, но мой директор не одобрит.

            • psycho-coder
              /#20956798

              Так ведь тесты для себя.

              • SbWereWolf
                /#20957006 / +1

                Для себя я пилю два проектика и там у меня код коверэйдж 100%.
                При чём во втором строчек побольше чем в этом и логика поинтересней.

                • avost
                  /#20957722 / -1

                  При чём во втором строчек побольше чем в этом

                  Побольше строчек в одном методе? Это круто, да.


                  и логика поинтересней.

                  Спасибо, я и этот ваш говнокодище теперь развидеть не могу, а если там ещё "поинтереснее"…

            • tendium
              /#20957514

              в четверг мы подписываем акт приёмки и ни какой дальнейшей разработки не будет.

              Смысл тесты писать? для фирмы которая через *цать лет наш код будет дорабатывать?

              Я б не смог работать в таких условиях… Но так или иначе, зачем приводить тут как образец кусок одноразового кода?

              • SbWereWolf
                /#20957862 / -2

                Хороший пример как можно писать без ветвлений.
                Понятно что его можно декомпозировать на 4 части, но я тут не учу SOLID-у, вот даже такой стиль написания кода он сам способствует декомпозиции, сделали — проверили, сделали — проверили, каждую пару выполнения шага и обработки результата можно в отдельный метод кидать. Но.
                Началось всё с того, что товарищ попросил пример кода, я ему пример кода привёл, понабежали чудаки которые стали судить код вообще с других позиций, смотреть на него со своей колокольни, развели оффтоп, а минусов я отхватил :)
                Если кто то не понимает что такое эффективность, что такое требования по качеству и требования по времени разработки, то успехов этим людям с производством ПО.

                • Chaos_Optima
                  /#20963334

                  Я так понимаю у этого кода требования по качеству были ниже некуда?

                  • SbWereWolf
                    /#20965146

                    Код предельно гранулирован, шаги можно легко менять местами, можно выкидывать, можно добавлять новые, для этого надо только подправить проверку до вставки кода и поправить проверку после вставки кода.
                    Учитывая что проверки на флагах, сделать это легко.

                    Скажите пожалуйста, что вас не устраивает в этом коде?
                    Количество строк в методе?
                    ок, код не декомпозирован и на то есть причины, описанные в моих коментах, что следующее?
                    Использование статических методов?
                    это методы Битрикса, мне свою обёртку на них написать надо?

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

                    • Chaos_Optima
                      /#20969042

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

                      Код предельно гранулирован, шаги можно легко менять местами, можно выкидывать, можно добавлять новые, для этого надо только подправить проверку до вставки кода и поправить проверку после вставки кода.

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

    • alekciy
      /#20961586

      А как это влияет? Несработавшее условие приводит к остановке. Какие связи?!

  4. mayorovp
    /#20954372

    Упомянутый тут антипаттерн называется "if ok".

  5. SpiderEkb
    /#20954434 / -2

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

    Хорошо, если язык позволяет конструкции типа on-exit, а если нет?

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

    А уж как реализовать множественные проверки… Тут надо включить мозг и подумать.

    • webkumo
      /#20954448 / +1

      Действия на выходе (если они вдруг нужны — на практике это редкость) выполняются функцией, в которую вкладывается fail-fast функция. Не надо городить антипаттерны из-за какой-то мифической фигни.

      • SpiderEkb
        /#20954472

        Ок. Вам виднее. Видимо, в Вашей практике действия на выходе не нужны. И проще увеличить стек вызовов еще одной функцией, чем корректно все разрулить в текущей.

        • webkumo
          /#20954556 / +1

          Мне вот интересно, вы на кристалле (МК) разработку ведёте или на платформах (веб/системное) у которых стек вполне себе приличный (мне вот ни разу без рекурсии его выбрать не удалось)? Себестоимость входа в ещё одну функцию так же заведомо на пару порядков ниже предложенных мельком в статье исключений.

          • SpiderEkb
            /#20954682 / -5

            Не поверите — глубокий backend на платформе IBM i — мейнфреймы IBM PowerSystem 824 (тестовый) и 828 (боевой).
            Каждый продукт проходит 4 уровня тестирования — компонентное, интеграционное, нагрузочное и бизнес.
            И на нагрузочном тестировании зачатую не проходят более безобидные на первый взгляд вещи чем лишний уровень стека — лишнее переоткрытие файла, лишнее чтение из dtaara, лишние операции с датой-временем, лишний запуск/остановка фонового процесса (submitted job)
            То, что вы своим кодом не можете выбрать стек ничего не значит. Скорее всего, пока работает одна ваша задача все просто замечательно. До тех пор, пока ваша задача не становится одной из тысяч, работающих одновременно. А вот когда в пике загрузка сервера переваливает за 90% (а то и 95%), начинаются изыскания на тему где и как можно скроить и кто жрет лишнего.

            В общем, мне лень спорить по пустому и рассказывать азы. Давно живу, повидал много на самых разных платформах.

            • webkumo
              /#20954728 / +8

              более безобидные на первый взгляд вещи чем лишний уровень стека — лишнее переоткрытие файла, лишнее чтение из dtaara, лишние операции с датой-временем, лишний запуск/остановка фонового процесса (submitted job)

              Вот вы сейчас прям страшно насмешили. Не знаю как в вашем языке, но в java работа со временем — очень дорогая операция. Не знаю, что такое dtaara, но переоткрытие файла — это работа с ФС ОС (которая даже буферизированная, без прямого обращения к диску — будет безумно дорогой). Что уж говорить про создание нитки или процесса (не знаю уж, эквиваленцией чего является ваш submitted job). И вы вот все эти тяжеловесные операции сравнили с уходом по стеку на один уровнь глубже! Да, разворачивать стек — тяжёлая, дорогая операция (именно этим плохи исключения — они буферизируют стек и могут ещё и разворачивать его для вывода ошибки). А вот войти в ещё одну функцию — что-то я сомневаюсь, что у вас настолько плотные математические алгоритмы. В общем — начните оптимизацию с чего-то более полезного (читайте частого) и ресурсоёмкого, а потом мне говорите бред про азы.

              • serbod
                /#20955084 / -2

                Дамп стектрейса при сбое давно изучали?

                • webkumo
                  /#20955454 / +3

                  А можно комментировать так, чтобы не приходилось догадываться? Если вы считаете, что стектрейс тяжёлая операция, то я с вами согласен:


                  Да, разворачивать стек — тяжёлая, дорогая операция (именно этим плохи исключения — они буферизируют стек и могут ещё и разворачивать его для вывода ошибки).

                  Если нет, то поищити бенчмарки для вашего языка.


                  PS "дамп стектрейса" даже звучит ужасно. Есть стектрейс (трассировка стека, развёрный стек). Есть дамп памяти. Вывод развёрнутого стека в лог — не есть дамп.

                  • serbod
                    /#20955798 / -1

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

                    Например. Настолькое приложение. Форма ввода с кнопкой ОК, выполняются какие-то проверки, вычисления, записи, чтения, и в итоге выполняется COMMIT в базу данных. Если происходит исключение где-то между ОК и COMMIT, то оно всплывает аж на уровень кнопки ОК, то есть обработчика событий GUI. И не факт, что перехватывается и обрабатывается должным образом. В результате, в программе проблема, но о ней могут не догадываться долгое время, а на поиск и исправление уйдет уйма времени.

                    А если внутренняя архитектура разбита на несколько коротих «конвееров» команд, то все намного проще и надежнее. Например, нажатие ОК не выполняет какой-то код, а только кладет команду и данные формы в очередь. Отдельный обработчик берет данные из очереди, проверяет и передает в следующую очередь. Затем следующий обработчик берет данные из очереди, производит вычисления, помещает результат в следующую очередь. Казалось бы, так все намного сложней и медленней. Но на деле, такая «микросервисная» архитектура намного проще в отладке, устойчивей и производительней на многозадачном железе. Исключение всегда всплывает на уровень консьюмера конкретного конвеера, значительно уменьшая размер контекста, сужая область и глубину поиска причины сбоя. Особенно ценно это в хардкорном продакшене, где нет доступа для отладки и приходится работать по продиктованному голосом адресу исключения. Даже не всегда есть возможность посмотреть логи и стектрейс.

                    • mayorovp
                      /#20955862

                      Вы написали полную чушь.


                      Во-первых, для того, чтобы поймать исключение вовремя, совершенно нет необходимости делать конвейеры и очереди сообщений, достаточно написать try-catch!


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

                      • serbod
                        /#20955960

                        Угу. А чтобы избежать ошибок достаточно их не совершать. Все просто! =) Но ошибку все-таки совершили и catch в нужном месте не поставили. Да и не всегда try… catch помогает поймать проблему, обычно ловится уже последствие, факт поломки. Например, когда образовался dead pointer, вроде все работает, но ломается где-то при закрытии программы. Или в конце «всплытия» из длинной цепочки вызовов. А ограничивая длину цепочки вызова, гораздо проще локализовать проблему.

                        • mayorovp
                          /#20955978 / +1

                          Да с чего проще-то?

                          • Quilin
                            /#20956134

                            Проблема всегда в том самом god-классе/методе. Куда уж проще!

                          • serbod
                            /#20956200

                            Меньше вложенных функций приходится изучать и держать их контекст в голове.

                    • kacetal
                      /#20958260 / +1

                      Вот после таких деятелей, боящихся уровней стека, приходится дебажить методы на 15к строк кода с одним ретерном в конце.

                    • webkumo
                      /#20959734

                      Понимаете в чём дело… это от арихтектуры зависит. Возьмём стектрейс в java: он может быть суммарно и на 1000 и на большее количество строк, но


                      • точка происшествия на самом верху. Возможен последовательный проброс исключений (это когда более "хардовое" ловят и выкидывают более общее, в этом случае вываливается цепочка исключений)
                      • он хорошо структурирован и поиск по нему не вызывает проблем
                      • стектрейс и дебаг вот вообще никак не связаны. Стектрейс описывает проблему и выдаёт данные, которые догодался запросить программист (в виде сообщения исключения).

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


                      Если происходит исключение где-то между ОК и COMMIT, то оно всплывает аж на уровень кнопки ОК, то есть обработчика событий GUI. И не факт, что перехватывается и обрабатывается должным образом. В результате, в программе проблема, но о ней могут не догадываться долгое время, а на поиск и исправление уйдет уйма времени.

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


                      Но на деле, такая «микросервисная» архитектура намного проще в отладке, устойчивей и производительней на многозадачном железе.

                      Наивные мечтанья — синхронизация данных между несколькими потоками штука нифига не бесплатная. Распределённая архитектура (микросервисы) хороша при очень лимитированном общении сервисов друг с другом. В такой ситуации и ethernet не становится преградой.


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

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

                      • serbod
                        /#20963624 / -1

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

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

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

                        Насчет отладки распределенной архитектуры. Простой пример: отдельно сервис (или сервер) БД, сервис HTTP и сервис приложения. В этом случае не нужно забивать голову деталями БД и HTTP при отладке приложения, это отдельные «черные ящики». В случае сбоя в БД или HTTP достаточно перезапустить только отдельный сервис, и область поиска причины сбоя значительно сужается. А представьте, что будет, если HTTP и БД обрабатываются в одном приложении, в одной цепочке вызовов? Малейший сбой уронит всю систему. Какой бы хороший ни был язык программирования, проблема будет в дизайне системы.

                        • webkumo
                          /#20964034

                          А что вы называете архитектурой?
                          Пользуйтесь языками, где архитектура предполагает внятные стектрейсы и логи.

                          Вроде и так всё предельно ясно написал. Есть язык, у него есть "платформа" и "окружение". "Окружение" — фактически среда исполнения. для системных языков это ОС, для jvm-запускаемых это, собственно, jvm, для php — это интерпретатор php + сервис в котором он крутится (если он не самостоятельный). "Платформа" — доступный SDK, библиотеки и компилятор/транслятор. Насколько мне известно, получить вменяемый стектрейс (с указанием строки в файле, где произошло исключение) для того же c++ задача не самая тривиальная. А для java это — базовый функционал.


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

                          Стектрейсы это часть логов. Да это важный инструмент, но к дебагу он относится никак. При грамотно реализованном логировании исключений (и отсутствию проблем с логированием внутреннего состояния… что обязательно будет, если работаешь с приватными данными) дебаг может и не понадобиться. Дебаг нужен, когда не удаётся эмпирически понять "а что же тут происходит-то вообще".


                          Насчет отладки распределенной архитектуры. Простой пример: отдельно сервис (или сервер) БД, сервис HTTP и сервис приложения. В этом случае не нужно забивать голову деталями БД и HTTP при отладке приложения, это отдельные «черные ящики». В случае сбоя в БД или HTTP достаточно перезапустить только отдельный сервис, и область поиска причины сбоя значительно сужается. А представьте, что будет, если HTTP и БД обрабатываются в одном приложении, в одной цепочке вызовов? Малейший сбой уронит всю систему. Какой бы хороший ни был язык программирования, проблема будет в дизайне системы.

                          Ну вот а теперь следите за руками:


                          • 3 ваших сервиса хттп, основной и БД.
                          • ситуация — запись не попадает в БД.
                          • раз понадобился дебаг — очевидно логов нет вообще (ну для такой простой ситуации иначе не получается)
                          • включаемся дебагом в сервис БД — а там шквал разных запросов из разных мест сервиса с бизнес-логикой
                          • включаемся дебагом в сервис с бизнес-логикой, ловим в нужной нам точке — всё работает корректно!
                          • что делать дальше? А приходится снова ловить БЛ и перед точкой запроса включать дебаг на сервисе базы и тут же отпускать сервис БЛ… после чего среди пары десятков/сотен/… запросов в БД ловить нужный нам. (вы же сами ратовали за модель "очереди сообщений").

                          И вот это всё — вместо того чтобы просто пройтись в глубину за точку… Я никак не могу назвать это более удобным дебагом.


                          В случае же сбоя сервиса… ну у jvm всё просто — в большинстве случаев с перезапуском справится контейнер, в отдельных случая — софтверным watch dog-ом. Это если брать вебное приложение. С UI-ным сложнее, но тоже можно что-то попытаться сделать. В пыхе, насколько мне известно, ситуация не хуже — в большинстве случаев падение одного запроса не угробит весь сервис.

                          • serbod
                            /#20964362

                            Напомню, что изначально речь шла о стеке и цепочке вызовов. И я на примере классического backend показал, что длинные цепочки вызовов усложняют отладку.

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

                            То есть, первый уровень отладки — это проверка входящих-выходящих данных каждого сервиса, чтобы определить, в каком сервисе проблема и на каких исходных данных. И уже потом лезть в конкретный сервис, ставя брейкпоинты и ассерты на конкретный тип данных.

                            В этом случае не придется изучать сотни запросов в дебагере. И не придется каждый раз перезапускать после сеанса отладки всю монолитную систему, достаточно перезапускать только конкретный сервис. Сфабриковать и подсунуть тестовые данные (mock) тоже проще в отдельный компонент системы, чем монолитную систему.

                            • webkumo
                              /#20964724

                              Напомню, что изначально речь шла о стеке и цепочке вызовов. И я на примере классического backend показал, что длинные цепочки вызовов усложняют отладку.

                              Попытались показать. Не получилось. Для java стек-трейсы легко могут быть в 100 и больше вызовов. При этом с ними вполне удобно работать.


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

                              Вы сами предолжили делать разными сервисами. Ну вот сервис работы с БД я и выделил в отдельный. Вот в него я и предлагал тыкаться дебагом "сначала". А если говорить про саму СУБД — то в зависимости от конкретной — разницы смотреть в логах или лезть в дебаг может и не быть, но всё же СУБД — это последняя точка, куда имеет смысл лезть за инфой, сперва лучше убедиться, что остальные корректно отрабатывают.


                              То есть, первый уровень отладки — это проверка входящих-выходящих данных каждого сервиса, чтобы определить, в каком сервисе проблема и на каких исходных данных. И уже потом лезть в конкретный сервис, ставя брейкпоинты и ассерты на конкретный тип данных.

                              И будете вы часами разгребать логи. Это нужно, чтобы определиться с изначальной проблемой (посмотреть на логи прода/теста). А дебаг уже должен быть спланирован по конкретным точкам, через которые бизнес-flaw тащит данные и запускает логику.


                              В этом случае не придется изучать сотни запросов в дебагере. И не придется каждый раз перезапускать после сеанса отладки всю монолитную систему, достаточно перезапускать только конкретный сервис. Сфабриковать и подсунуть тестовые данные (mock) тоже проще в отдельный компонент системы, чем монолитную систему.

                              В том и дело, что в монолите (хотя правильнее сказать "без конвееров сообщений) в принципе не придётся изучать сотни запросов — тыкаешься в начальную/среднюю точку сломанной бизнес-логики и идёшь дальше по вызовам. Быстро, удобно, комфортно. С конвеерами так не получится — там брекпоинты нужны условные, а они не со всякой платформой эффективно работают. Далее — зачем перезапускать после сеанса — я тоже не понял. А мок-данные и вовсе к дебагу никакого отношения не имеют — это банальный юнит-, компонентный и интеграционный тест (в принципе все уровни пригодятся, да).

            • kahi4
              /#20954804 / +4

              Любопытства ради, как глубина стека зависит от нагрузки?

              • serbod
                /#20955248 / -3

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

                • kahi4
                  /#20955510 / +3

                  Вот только асинхронные вызовы помещаются в event loop (или его аналоги), а не лежат в стеке. Как результат — отказ эвент лупа, а не проблемы, связанные с глубиной вызовов. Проще говоря — процессор не успевает разгребать события и это понятно, но увеличение глубины стека — меньшее из зол, не имеющее отношение к эвент лупу. Ну понятно, что вызов функции — это сложить 4 (8, 16?) регистров в стек, поменять CP, это все помноженное на косвенную адресацию, что в итоге отъедает процессорное время, я сам лично, заинлайнив один метод в three.js, привел к его (очень незначительному) росту производительности, но речь идет про под-функцию внутри перемножения матриц, которая вызывается миллионы раз на каждый кадр, в остальных случаях в реальных приложениях цена вызова функции крайне мала и скорее будет проседать соединение с базой (я уже молчу про сам неоптимизированный запрос), чем проблемы с вызовом функции.

                  • serbod
                    /#20955884 / -1

                    Это если классический синхронный event loop и message queue, как в винде. А в Андроиде, например, таймеры и activities стреляют не дожидаясь завершения предыдущего вызова, как будто в новом потоке. Да и JS вроде тоже. В винде проблемы возникали при банальном переключении визуальной иконки по таймеру. Вроде все нормально работает, а через час падает. А когда смотришь через профилировщик, то виден постоянный рост стека и поглощение какого-то ограниченного ресурса, например, дескрипторов в Windows. Потому что где-то стоял WaitForMultipleObjects, который не блокировал event loop, а тупо ждал выхода из какой-то функции.

                    • mayorovp
                      /#20955970 / +2

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

                      Про таймеры не знаю, но весь UI там в одном потоке.


                      Да и JS вроде тоже.

                      В JS вообще нет потоков. Есть (Web)Workers, но они больше похожи на процессы, ибо обладают своей собственной памятью.


                      В винде проблемы возникали при банальном переключении визуальной иконки по таймеру. Вроде все нормально работает, а через час падает. А когда смотришь через профилировщик, то виден постоянный рост стека и поглощение какого-то ограниченного ресурса, например, дескрипторов в Windows.

                      Проблема утечки ресурсов не имеет никакого отношения к асинхронным таймерам. Её при желании можно и в простейшем цикле устроить, так же как и в таймере избежать.


                      Потому что где-то стоял WaitForMultipleObjects, который не блокировал event loop, а тупо ждал выхода из какой-то функции.

                      Но WaitForMultipleObjects, будучи вызван в UI потоке, не может не блокировать event loop!

                      • serbod
                        /#20956242

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

            • Deosis
              /#20954810 / +1

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

      • SbWereWolf
        /#20955876

        Для меня фишка в отладке, когда делаешь дебаг просто ставишь брейк поинт на единственный ретурн в коде и жмёшь F5 и тебе не надо проживать на каждую строчку кода F10 в ожидании ого что сработает какой то ретурн и ты даже не узнаешь почему и что функция / метод вернули в вызывающий код.
        Другое удобство в том что бы не прожимать весь код, а нажать F5 и после этого пробежаться глазами по флагам и значениям переменных и быстро понять что к чему, успешно ли отработал код, или были ошибки и тут же посмотреть что функция возвращает.
        Мне удобно когда ретурн только один, и меня не смущает использовать флаги, что бы не исполнять код который при «ошибках» исполнять не надо

        • taliban
          /#20957186 / +1

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

    • CrazyElf
      /#20954540 / +1

      Ну нет, как-раз таки логика «если что-то не так, то валим быстрей отсюда нафик» (и чем раньше по коду, тем лучше) — она очень наглядная и понятная.

    • Sabubu
      /#20954692 / +1

      Множественные точки выхода из процедуры не есть хорошо.

      Почему? Где технические аргументы? Вы сейчас верующего напоминаете. По моему, так лапша из ифов в разы хуже.

      • Deosis
        /#20954840 / +7

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

        • questor
          /#20955950

          Звучит логично. А где-нибудь об этом в книжках писалось, можете подсказать? Я вот про изначальный смысл что-то не помню, чтобы где-то читал.

    • vintage
      /#20954858 / +1 

      function 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
      /#20955078 / +3

      Ага, видел как сишники делают в конце функции метку, а потом делают к ней goto…

      • Gryphon88
        /#20955668 / +2

        Достаточно удобный on exit для старого языка. switch-case с fallthrough, где идентификаторы case'ов используются как метки для goto. Goto вызывается в проверках для раннего выхода.

    • Hardcoin
      /#20956984

      единственную точку входа и единственную точку выхода. Тогда ее логика понятна и прозрачна.

      Эх, если бы. Но нет, единственность точек входа/выхода не является ни необходимым, ни достаточным условием понятности функции.


      Если вам приходится в одном критичном месте использовать много уровней вложенности ради экономии одного уровня стека, это исключительный случай.


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

      • tendium
        /#20961986

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


        Ну… Есть еще Rust, который у Си может иногда выигрывать.

  6. trawl
    /#20954446

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

    <?php

if (!$pattern->worked()) {
    $code->useSometimes('else');
} else {
    $code->doNotUse('else');
}

  7. pacific_monkey
    /#20954498 / +2

    Авторы, хватит говорить мне что мне делать.

  8. MrBman
    /#20954644 / +1

    Я конечно не программист и php я тоже не знаю! Однако, почему бы в данном конкретном случае, просто не сделать как-то так?:

    if ($user != null
	&& time() >= strtotime('12 pm')
	&& $user->hasAccess(UserType.PREMIUM)
	&& $store->hasItemsInStock()) {
	// OK
} else {
	// The access is denied!
}

    • peresada
      /#20954742 / +3

      Чтобы понять, почему так лучше не делать, попробуйте ответить на следующий вопрос: какую ошибку должно вывести приложение пользователю в случае запрета доступа?

      • Revertis
        /#20955070 / +5

        «Ууупс! Что-то пошло не так.» :-D

        • peresada
          /#20955262 / +14

          • Пользователь звонит в техподдержку: Техподдержка, что-то пошло не так
          • Техподдержка сообщает в центр мониторинга: Мониторинг, что-то пошло не так!
          • Мониторинг в эксплуатацию: Эксплуатация, что-то пошло не так!
          • Эксплуатация сообщает программисту: Программист, что-то пошло не так!
          • Программист: Сука, знал же, что нужно разделять условия и логировать разные варианты этого «что-то пошло не так»

    • CrazyElf
      /#20954802 / +2

      К сожалению, множественные логические условия точно так же быстро перестают быть понятными, как и множественный if. А уж если там не только AND, но и OR условия и туча скобок, то это ещё хуже для разбора и понимания, чем много if-ов, хоть и выглядит компактнее.

  9. Virgo_Style
    /#20954678 / +2

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

    • CrazyElf
      /#20954818

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

  10. haldagan
    /#20954906 / +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
      /#20954956 / +1

      большинство из этих ошибок ввода обычно необходимо обработать/выдать пользователю одновременно

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

      • haldagan
        /#20955004

        Не совсем понял аргумент.
        То есть по-вашему если приложение пропустило невалидные данные на сервер, то пользователю выдается окошко «ой, что-то стряслось, попробуйте позже»?
        А если сервер таки отдает приложению информативный фидбек (сервер-то в любом случае данные проверять обязан), то почему фидбеку не быть И информативным И удобным (в смысле вываливать все ошибки валидации, а не по одной)?

        • vintage
          /#20955088 / +1

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

          • alekciy
            /#20961646

            Люто плюсую! Сразу виден опытный человек которому приходилось копаться в 100500 сгенерированных ошибках появившихся каскадно из-за пары первых.

          • haldagan
            /#20963484 / -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
          /#20961636

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

          • vintage
            /#20963056 / -2

            А клиентская валидация — все возможные ошибки.

            Тоже не обязательно. Пример: https://mol.js.org/app/demo/-/#demo=mol_form_demo_bids

            • alekciy
              /#20963618

              Не уловил почему «тоже не обязательно» в условиях когда данный пример как раз демонстрирует поведение о котором я говорю.

              • vintage
                /#20963932

                Там показывается по одной ошибке за раз, попробуйте поредактировать.

                • alekciy
                  /#20963940

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

          • haldagan
            /#20963344

            ошибках появившихся каскадно


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

            • alekciy
              /#20963554

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

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

              • haldagan
                /#20963796

                Мы все еще про сервер.

                еще не дошел до ввода данных Х, мы уже «параллельно проверяем все остальные поля»


                Я так понял это Вы про клиент?

                Если все-таки про сервер — поясните, почему и зачем клиент отправляет заведомо неполный набор данных на сервер?
                Если речь идет о multi-step формах с участием сервера, то на сервере валидируется и обрабатывается текущий шаг, а не вся форма целиком.

                • alekciy
                  /#20963968

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

                • psycho-coder
                  /#20964182

                  Если все-таки про сервер — поясните, почему и зачем клиент отправляет заведомо неполный набор данных на сервер?

                  Да мало-ли, что произодйет с клиентом, что он решил отправить заведомо не полный набор данных. Или у вас валидация полноты набора данных только на клиенте?

  11. haldagan
    /#20955002

    del

  12. Zenitchik
    /#20955076

    У этого кода проблема не в «else»

  13. AlNi89
    /#20955092 / -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
      /#20955174

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

    • Zenitchik
      /#20955188

      Вам нравится в IDE видеть много пустых строчек со скобочками?

      • AlNi89
        /#20955628 / -2

        Мне нравится удобство чтения кода.

        У автора всего 10 строк и уже

        трудно понять к какому конкретно условному выражению относится каждое else
        Зато по стандарту. Мыши плакали, кололись, но продолжали есть кактус. А мой пример читается легко и наглядно, сколько бы минусов комментарию не поставили.

        • mayorovp
          /#20955672 / +1

          Ну так в вашем варианте что, проще понять к какому условию else относится — или как?

          • AlNi89
            /#20955986 / -1

            Однозначно.
            Мало того, что видимость прямая (else прямо рядом с соотв. скобками if), так ещё и в комментариях рядом указано, к какому условию это else.

            • Deosis
              /#20958796 / +1

              А кто будет менять комментарий, когда поменяется условие?

          • wadeg
            /#20956528 / +1

            Кстати, да, все-таки намного проще.

        • webkumo
          /#20955714

          Немного тренировки — и формат "по стандарту" читается легче и быстрее, чем ваш. Ну просто потому, что больше значащего текста помещается на экран, при этом не наплывая друг на друга. Да и в целом — вложенность больше 2х — много где считается code smell. Об этом автор и говорит (правда, почему-то, обвиняя бедные else).

          • Lodin
            /#20955804 / +1

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


            К примеру, в C# принят подход с переносом скобок на следующую строку во всех случаях — это правило языка. На JS, наоборот, нужно скобки оставлять на той же строке, даже для классов и методов — и это тоже правило языка. И используя эти языки, я буду использовать именно их правила, чтобы любой другой программист, прочитав код, обращал внимание на логику, а не на нестандартное оформление.

            • webkumo
              /#20955968

              А в java есть несколько стандартов (от нескольких крупных игроков) и они различаются.


              PS да я согласен, что стандарт это хорошо, просто я пользуюсь языком, где владельцам языка не удалось всех свести под гребёнку одного стандарта. Так что, личная оценка "что удобней" у меня есть. Другое дело, что кому-то действительно нехватает этих отступов, чтобы легче структурировать информацию… и в целом в java форматированием уже существующего кода стараются не увлекаться...

              • Nimtar
                /#20958198

                Интересно, в Java, в отличие от C и упомянутого выше JS, я разногласий по теме не встречал: только египетские.

          • AlNi89
            /#20956024 / -2

            Немного тренировки — и

            и Вы на крючке у когнитивного искажения, Вам кажется что так легче, из-за удобного стандарта, а на само деле Вам легче, потому что было
            Немного тренировки

        • Zenitchik
          /#20955756

          трудно понять к какому конкретно условному выражению относится каждое else

          Это автор, простите, брешет. Там невооружённым глазом видно, к какому условию относится else. Тупо по отступам.
          А если бы он переносил else на следующую строку — IDE помогла бы ему сворачивать код поблочно и видеть строки наподобие с
          +if(...){...}
          +else{...}
          где + — иконка разворачивания.

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

          • questor
            /#20955994

            Мнение логичное, хотя и на это есть свои контраргументы.


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


            Хотя, если кто-то умеет силой мысли раскрывать плюсики… </ irony> :)

            • webkumo
              /#20956374

              Фишка в том, что нормальным IDE пофиг на форматирование — они умеют эти плюсики и в сплошном тексте делать. Но я почему-то предпочитаю, чтобы код читался без лишних телодвижений.

              • Zenitchik
                /#20956586

                Плюсики в сплошном тексте — это неудобно. Хочется всё-таки, сворачивание по строкам происходило, от свёрнутого блока оставалась одна строка, по которой можно было понять, что это за блок, а «плюсики» были в начале строки.

        • Hardcoin
          /#20957328

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


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

    • Lodin
      /#20955434 / +2

      Есть же PSR, где чёрным по белому написано, как должны оформляться if-структуры. Зачем изобретать велосипед?

      • AlNi89
        /#20955560 / -2

        Чтобы не писать вот такие комментарии:

        Хотя подход выше и выполняет задачу, уже трудно понять к какому конкретно условному выражению относится каждое else

        • Lodin
          /#20955630

          Как будто простой перенос скобки на новую строку как-то решает проблему. Лично для меня что там понятно, что там. Для новичка, скорее всего, будет непонятно в обоих случаях, просто глаз не привык.


          А вот нестандартный кодстайл создаёт гораздо больше проблем, чем решает.

    • sayber
      /#20955488

      Может не стоит писать как криворукий младенец а использовать стандарт который принят официально?
      Я лично уволю тех, кто будет писать код как в вашем примере.

      • AlNi89
        /#20955594 / -4

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

        Теперь понятно, почему нынешний софт такой тормозной: все думают о стандартах, а не удобстве (читаемости в данном случае) для человека…

        • Lodin
          /#20955658 / +1

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

          И это пишет человек, сказавший в своём комментарии:


          Может просто начать по-человечески оформлять код, без идиотской привычки ставить открывающую скобочку в одной строке с if?

          В своём глазу бревна не видим? :D


          Теперь понятно, почему нынешний софт такой тормозной: все думают о стандартах, а не удобстве (читаемости в данном случае) для человека…

          Вы прям взаимоисключающие параграфы используете. Стандарты ведь придумали как раз для удобства человека.


          А про "тормозной код" даже как-то и сказать нечего: как скорость исполнения кода зависит от стиля написания кода — решительно непонятно.

        • sayber
          /#20955678 / -3

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

          Не знаю как тормозной, но продукт под моим началом спокойно держит 2000qps.

          • goldrobot
            /#20956318 / -1

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

            • sayber
              /#20956364

              Все нормально с глазами.
              Есть PSR. Который обсуждается и принимает не один год.
              Но есть люди, которые любят делать как им захочется. А затем почему то удивляются, почему их не берут на работу, хотя бы на 200т.р.

              • goldrobot
                /#20958612 / -2

                Вот вот, и я про тоже. Эти люди незнают даже fuelphp стандарта из-за чего их даже на позицию с зарплатой 350 т.р. не берут. А далее они от обиды идут и за копейки пашут в компании с PSR.

        • peresada
          /#20955682 / +1

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

          • Perlovich
            /#20955810 / +1

            > Стандарты разрабатываются и обсуждаются неделями, месяцами и иногда годами

            Мне кажется, что к правилам размещения скобочек это не относится. Например, в C# пишут скобочки как раз на отдельной строке, в отличие от php или Java.

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

            • Lodin
              /#20955848

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

              Я бы сказал — в пределах языка. Чтобы, когда читаешь исходники библиотеки, не пришлось дополнительно думать о нестандартном оформлении.

            • peresada
              /#20955912

              Стандарты на то и стандарты, что у них нет понятия «важности». Это всего навсего договоренности между группой людей, и по-моему все договоренности одинаково важны, так как прийти к общему соглашению бывает достаточно трудно.

              Текущее обсуждение как раз это и показывает

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

    • Alexufo
      /#20958048

      дьявол, терпеть не могу { c новой строчки :-). Накой эти набития путые раздувания? Нет никакой там читаемойсти.В PSR-0 более менее все обговоренно

  14. tendium
    /#20955156

    Приём называется early return. Else тут вторичен. Выгода: меньше влодености, легче анализировать такой код, и как следствие снижение количества потенциальных багов.

  15. dadon
    /#20956258 / -2

    Функция с кучей выходов — ад для отладки, функция с кучей ветвлений — ад для чтения. Истина, как обычно, где-то посередине.
    Но улыбает, конечно, когда люди начинают заезжать и бездумно применять какой-то паттерн везде.
    Смиритесь люди, иногда без пару else if не обойтись, не создавая какой-нибудь запутанной фигни :)

  16. MaxVetrov
    /#20958202 / -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
      /#20958598

      0. Дыра в безопасности, больше требований к фильтрации данных
      1. Нет подсветки синтаксиса
      2. Нет поддержки рефакторингов в IDE
      3.…

    • mayorovp
      /#20958600

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


      Но даже с ними мне такой код всё равно не нравится.

      • Gryphon88
        /#20959534

        Кстати, а когда eval нужен, что без него аж грустно? Всегда можно написать немного бойлеплейт кода, а принимать внешние функции для выполнения и файла или по сети как-то совсем некузяво.

        • peresada
          /#20959550

          когда архитектура на нем завязана, в остальном не нужен. Вот бери тот же Modx, там пхп-код в БД хранится. Это ужасно конечно, но такое действительно есть

        • webkumo
          /#20959670

          Реально — он нужен никогда. Но на него действительно могут пытаться завязать дивно-динамическую архитектуру… В принципе и в других языках встречается подобное (в js — прямой аналог и тоже начисто не рекомендуется к использованию, в java — можно на лету сваять или загрузить откуда-нибудь байт-код и загрузить его в jvm, что в принципе уже не является чистым антипаттерном — во-первых с кодом один фиг работают как с кодом, а не текстом, во-вторых случайно такое не сделаешь, там нужно специально постараться… в общем если правильно спроектировать — относительно безопасная вещь).

        • vanxant
          /#20959930

          Я знаю только одно разумное применение — когда колхозят недо-«Excel» с вводом формул юзером вручную и при этом ленятся писать свой парсер + хотят дать доступ к стандартным функциям.

          • mayorovp
            /#20959976

            Не обязательно лениться писать свой парсер. Можно как раз-таки свой парсер написать, далее обработать AST и сгенерировать код, а потом уже сделать eval.


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