1. PHP / Говнокод #12210

    +51

    1. 1
    2. 2
    3. 3
    4. 4
    $needMoreDataWM = ($exchange->getSumTo()->getCurrency()->getSystem()->getModule() == 'WebMoney' || $exchange->getSumFrom()->getCurrency()->getSystem()->getModule() == 'WebMoney') || $this->getRequest()->request->get('user_wmid');
$needMoreDataWM = $needMoreDataWM && !$user->getWmid();
$needMoreDataBank = ($exchange->getSumTo()->getCurrency()->getSystem()->getClass() == 'bank') && !$user->getVat() || $this->getRequest()->request->get('user_vat');
$needMoreData = $needMoreDataBank || $needMoreDataWM || !($user->getFullname() && $user->getPassport()) || $this->getRequest()->request->get('user_fullname') || $this->getRequest()->request->get('user_passport');

    А что поделаешь, Doctrine принуждает

    Запостил: nick4fake, 28 Ноября 2012

    Комментарии (17) RSS

    • ava Fai 28.11.2012 01:50 # +1

      Блин. Дожил. Когда увидел на конце имени большую букву M, сразу подумал о монадах.

      Долго гадал, чего это "функции" со знака доллара начинаются.
      Ответить

    • ava Vasiliy 28.11.2012 11:04 # 0

      комментарии тоже доктрина мешает писать ?
      Одна сплошная магия и не одного слова зачем.
      Ответить

      • ava nick4fake 28.11.2012 13:42 # +2

        Там магии нет, и все очевидно (серьезно). Определяется система платежа, производится проверка, должен ли человек еще данные ввести. А вот получилась каша. :)
        Ответить

        • ava Vasiliy 28.11.2012 16:44 # +2

          ну не знаю меня подобные конструкции $needMoreDataWM && !$user->getWmid(); ввергают в бездну уныния. Ходи гадай чего это функция которая должна возвращать int возвращает bool а если она все же возвращает 0 то empty было бы симпатичней.
          в любом случае такие конструкции надо комментировать хотя бы так >Определяется система платежа, производится проверка, должен ли человек еще данные ввести.
          Ответить

    • ava eth0 28.11.2012 20:45 # 0

      > $exchange->getSumFrom()->getCurrency()->getSystem()->getModule()
      Люди любят копипасту и ненавидят промежуточные переменные. Seems o.k.
      Ответить

      • ava nick4fake 28.11.2012 21:09 # 0

        Промежуточная переменная там будет излишня.
        Ответить

        • ava eth0 28.11.2012 22:03 # 0

          Не знаю, как дела обстоят в ПХП с кешированием, но в худшем случае будут четыре лишних вызова. Я бы проверил, но как-то лениво.
          Слишком глубокий путь - явный косяк в архитектуре. По-моему так.
          Ответить

          • ava wvxvw 28.11.2012 22:09 # 0

            getSumFrom != getSumTo
            Последний только 2 раза вызывается, но вот с $this->getRequest()->request такой косяк имеет место быть. И кешировоть оно никак не может, а вдруг там побочный еффект? ПХП не то, что не станет разбираться в том можно ли соптимизировать или нет, он просто даже не оперирует такими понятиями, там лишь бы до победного конца дотянуть, а как - уже не суть важно.
            Ответить

            • ava nick4fake 28.11.2012 22:13 # −1

              Не правда. Очень часто пхп кеширует (например, foreach($var->a() as $row), метод будет вызван только один раз).
              А так - да, можно было запрос кешировать, только разница была бы небольшой.
              Ответить

              • ava wvxvw 28.11.2012 22:25 # 0

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

                • ava nick4fake 29.11.2012 00:32 # 0

                  Вы читали мой комментарий? В вашем случае действительно можно упростить.
                  Ответить

                  • ava wvxvw 29.11.2012 01:38 # 0

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

            • ava eth0 29.11.2012 08:41 # 0

              Проглядел, мой фолт. Хотя, минимум три вызова из пути повторяются строчками ниже.
              Если в ЭТОМ есть ещё и побочные эффекты - проще застрелиться сразу.
              Ответить

          • ava Fai 28.11.2012 22:17 # −1

            > явный косяк в архитектуре

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

            Ну или требования менялись.
            Ответить

            • ava nick4fake 29.11.2012 00:33 # 0

              Требований много. Пришлось использовать такой подход для гибкости.
              Ответить

    Добавить комментарий