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

    +164

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    // set admin mode
    switch (true) {
      case $nc_core->inside_admin:
        $nc_core->admin_mode = true;
      break;
      case !$passed_thru_404 && isset($posting): // add (edit) action
        $nc_core->admin_mode = $admin_mode;
      break;
      case !$passed_thru_404: //front-office
        $nc_core->admin_mode = true;
      break;
      case $passed_thru_404:
        $nc_core->admin_mode = false;
      break;
      default:
        $nc_core->admin_mode = false;
    }

    /netcat/require/index.php
    Что они там курят, что у них настолько извилины выпрямляются?!

    Запостил: telnet, 26 Мая 2010

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

    • я хуею, дорогая редакция: $nc_core->admin_mode = $admin_mode;
      интересно откуда там этот $admin_mode береца?
      Ответить
      • В этом файле - ниоткуда. Я уж было думал, что дело пахнет уязвимостью (ведь register_globals=on, а дело происходит в глобальной области видимости), но проверка показала, что не катит. До этой конструкции идут 9 инклудов - наверное, где-то в одном из подключаемых файлов $admin_mode выставляется в false.
        Ответить
      • из глобальных переменных.
        В PHP программировании (в отличии от структурного) принято все переменные делать глобальными
        Ответить
        • в ГК принято. Лично у меня только одна глобальная, остальные... хм, суперглобальные )))
          Ответить
        • Толсто.
          Ответить
    • Очень интересная конструкция. Я что топодобное гдето видел даже помоему и ГК.
      С одной стороны дурь и блаж с другой очень тонкое использование особенностей языка на других (с которыми я сталкивался) в подобных конструкциях для выбора ветвеления используется только константа и не вкоем случае не строка. А тут однин свич запихали несколько условий. Единственное что мне прямати расмешило это
      case $nc_core->inside_admin:
      $nc_core->admin_mode = true; три раза код пречитывал так смысл и не увидел.

      Остальное хоть как то можно объяснить
      Хотя тут же зная особности языка можно использовать что вроде if (z) x=b=c=true; else a=b=false;
      Как то так ну может я и ошибаюсь
      Ответить
      • Если $nc_core->inside_admin уже true, остальные проверки не выполняются.
        А присваивание выполняется видимо для явного приведения типа.
        Ответить
    • switch (true) //0_o
      таких жестоких свитчей на говнокоде я еще не видел
      Ответить
      • Это распространённая практика, когда в падлу писать if() {} elseif() {} elseif() {} ...
        Ответить
        • зато выполняется медленней. проще один раз помучаться и написать с ифами, зато потом будет быстрее работать.
          Ответить
        • блин ИМХО какая-то говнопрактика, else if () быстрее пишется чем
          break; case ()
          единственное толковое применение switch(true) - это когда некоторые break - отсутствуют - иногда это может оказатся полезным.
          Ответить
          • кстати, о свитчах. всегда полагал, что идиотизм писать везде брейк. лучше бы по умолчанию он был, а если надо свалиться в следующую ветку, то делать это явно, навроде continue
            Ответить
            • че? ты хоть представляешь, как этот код оттранслируется?
              по кодогенерации тысячи манов - кури на здоровье.
              Ответить
              • а зачем ориентироваться на транслятор? зато меньше ключевых слов и магическая мантра "перед каждым кейсом не забудь брек"!
                Ответить
                • а зачем ориентироваться на здравый смысл?
                  думаешь, ритчи от нех делать такой синтаксис использовал?
                  у тебя есть уникальная возможность создать свой язык с блекджеком и 'правильным' switch
                  Ответить
                  • угу, но нет смысла, когда все уже привыкли
                    Ответить
                    • именно, тогда это возможно было обусловено простотой написания компилера, а потом вошло в стандарт и так там и осталось
                      Ответить
                • А затем, что switch - прямой наследник goto, и работает не на уровне блоков, а на уровне инструкций. Единственный блок - сам switch, из которого и выходит break;

                  Так что убрать break'и можно только заменив switch на совершенно другую конструкцию (с нормальными блоками вместо case'ов, типа case of в erlang'е), но тогда ни о каких проваливающихся case'ах речи быть не может, и будет такая конструкция просто сахаром над if() else if () else ().
                  Ответить
                  • в принципе не проблема, при компиляции при встрече следующего case без continue вставить goto на конец switcha, а при встрече continue; case не вставлять его
                    Ответить
                    • угу, и получим говнокод на уровне компилятора
                      Ответить
                      • говнокод - это когда криво реализовано
                        а тут просто ставится отрицание
                        на условии ставить ли goto endSwitch
                        и детекшн keyworda break заменяется на детект continue.
                        мне всегда казалось что компилеры писали, чтоб было удобнее кодить юзающему, а не наоборот
                        Ответить
                        • вот не надо городить костыли, поверх того, что реально работает (ведь налицо же продуманная идея)
                          если хочется 'подсластить' if else if.. лучше новую конструкцию ввести (в руби именно такой case)
                          Ответить
                          • да не костыль это и не ГК
                            просто обычное отрицание
                            continue!=break
                            другое дело что все уже привыкли
                            Ответить
                            • да че ты оправдываешься-то =)
                              я пока никого и не обвинял)
                              Ответить
                              • я не оправдываюсь, я говорю что это не костыль
                                Ответить
                                • Это костыль

                                  Switch
                                  case: -> метка, кода на своем месте при компиляции не дает
                                  break -> goto в конец блока
                                  Все просто как кувалда

                                  Switch-без-break
                                  case: -> метка, но компилируется в break, но только если перед ней нет continue
                                  continue -> вообще ничего не делает, но влияет на компиляцию case

                                  И это по вашему не говно-костыль?
                                  Ответить
                    • тоже согласен... но раз так сложилось исторически, лучше я один потерплю, чем потом все будут валиться на ошибках с матюками в адрес меня :)
                      Ответить
                  • ну вот что-то в этом роде я и имел ввиду - более элегантный else if
                    Ответить
        • нормальные пацаны просто меняют структуру кода..
          switch (true) {
            case $test->isCool():
              $test->makeBad();
              break;
            case $test->isTrololo();
              $test->goDie();
              break;
          }

          на
          return $test->tryMakeBad() || $test->tryDie();

          А в каждом методе логика аналогична той, что в кейзах
          Ответить
    • А там работает ли `index.php?admin_mode=1`? :-)

      UPD: это была первая мысль, @telnet сообщил что не работает. Поздравим неткат с этим.
      Ответить
    • В неткете данные могут выводиться в обычным виде, в режиме редактирования, а так же в админке. В последних двух случаях к выводимым объектам могут добавляться различные ссылки ( удалить, изменить, етс).
      Добавлять/изменять данные можно как с сайта, а можно в режиме редактирования.
      Понятно, что если пользователь добавляет данные в режиме редактирования, то после операции он должен в этом режиме и остаться.

      // если пользователь в админке - то он явно в режиме редактирования
      case $nc_core->inside_admin:
      $nc_core->admin_mode = true;
      break;
      // тут $admin_mode приходит из формы добавления (в post'e например )
      case !$passed_thru_404 && isset($posting): // add (edit) action
      $nc_core->admin_mode = $admin_mode;
      break;
      // через обработчик чпу запрос не проходил, значит пользователь зашел по адресу
      мойсайт.ru/netcat/ - это режим редактирования
      case !$passed_thru_404: //front-office
      $nc_core->admin_mode = true;
      // запрос прошел через обработчик чпу - показывается просто страница в обычном режиме
      case $passed_thru_404:
      $nc_core->admin_mode = false;

      То есть любой авторизированный пользователь может переключиться на режим редактирования ( в скрипте тогда будет $admin_mode = 1 ), но это не значит, что он может все поудалять, у него не будет никаких на это прав:
      http://netcat.ru/netcat/?catalogue=3&sub=422
      Ответить
      • Разработчек?
        Ответить
      • UPD: впрочем, это не важно. Спасибо за разъяснение, как работает этот кусок кода; мне часто приходится колупаться внутри, чтобы понять, как что-то работает/не работает. Тем не менее, приведённый кусок от этого более грамотно написанным не становится ни капли.
        Ответить
        • Пример грамотного кода в студию
          Ответить
          • все есть калл, потому что пхп.
            Ответить
            • Калл есть гости необезображеные интелектом
              Ответить
              • Сказал обезображенный интилектом PHPшник.
                Ответить
          • // Тот же switch, но без избыточной логики:
            switch (true)
            {
              case $nc_core->inside_admin:
                $nc_core->admin_mode = true;
                break;
              
              case !$passed_thru_404:
                $nc_core->admin_mode = isset($posting) ? $admin_mode : true;
                break;
              
              default:
                $nc_core->admin_mode = false;
            }
            
            // Спорный вариант с синтаксическим сахаром, но я бы предпочёл его:
            $nc_core->admin_mode = ($nc_core->inside_admin)
              ? true
              : ($passed_thru_404 ? false : (isset($posting) ? $admin_mode : true));
            Ответить
            • Мне вариант с синтаксическим сахаром нравится больше всего: он и короткий, лаконичный, работающий ( скорее всего, и самый быстрый), но вариант в оригинале, на мой взгляд, более читаемый.
              Ответить
              • Естественно, вариант в оригинале самый читаемый. Там логика в лучших традициях Капитана Очевидность - прямая, как извилины автора струна. Это выдаёт в авторе самого что ни на есть новичка, который ещё не отточил до автоматизма ни умение держать возможные ходы выполнения в голове, ни тем более умения просчитывать их, дабы не плодить лишних, и хорошо просчитывать их, дабы не перебарщивать с избавлением от лишнего, и поэтому скрупулёзно прописывает каждую ветку. То же самое я могу сказать про весь код NetCat, за редким исключением. Сейчас почитываю код 4-й версии - местами видны поразительно светлые мысли на фоне остальной безблагодатности, да и в мануале местами проскальзывают очень здравые на фоне остальных суждения. Никак команда девелоперов пополнилась кем-то действительно толковым. Надеюсь на лучшее.
                Ответить
                • местами видны поразительно светлые мысли
                  Если не секрет, какие?:-) все, что находится в system/ ?
                  Ответить
                  • Ну как сказать =) То, что основные сущности системы додумались реализовать в виде объектов, и то, что для класса nc_Core кто-то даже догадался применить шаблон "Одиночка", задепрекейтив глобальную переменную $nc_core, на фоне остального выглядит большим шагом вперёд. Вот только то, что в классе nc_Core какой-то умник взялся использовать ReflectionClass, который даже не документирован, повергло меня в шок. Именно из-за последнего, кстати, NetCat оказалась запускаться на PHP 5.3. Никак не возьму в толк, на фига, во-первых, было использовать недокументированный интерфейс, поведение которого может меняться в будущих версиях непредсказуемо, и, во-вторых, для чего вообще автора потянуло к средствам рефлексии (этот момент надо будет попробовать самостоятельно раскурить, как свободное время появится). Впрочем, на фоне того, что у CMS без изрядного рефакторинга будущего всё равно нет, этот факт смотрится не особо ярко.
                    Ответить
                    • Краткость не ваша сестра...
                      Весь текст ужимается до "NetCat - УГ".
                      Ответить
                      • Да, увы, мне часто это вменяют, но почему-то оказывается проще написать много, чем написать много, а потом отжимать воду редактировать =3
                        Ответить
    • $nc_core->admin_mode = $nc_core->inside_admin || (($tmp = !$passed_thru_404 && isset($posting)) && $admin_mode || !$passed_thru_404) && !($tmp && !$admin_mode)

      вроде так))
      Ответить
      • Тогда уж
        $nc_core->admin_mode = $nc_core->inside_admin || (!$passed_thru_404 && (!isset($posting) || $admin_mode));

        Проверять лень =3
        Ответить

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