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

    +118

    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
    /**
     * Функция создает новую таблицу. 
     * @param $name - имя новой таблицы. Имя должно быть проверено (например функцией mysql_real_escape_*)
     * @return TRUE - если новая таблица была создана или FALSE - если нет
     */ 
    function createTable($name) {
        $retval = false;
        if(!empty($name)){
            $query = "CREATE TABLE IF NOT EXISTS `" . $name . "` (`Adres` varchar(150) DEFAULT NULL, `send` int(1) DEFAULT NULL )";
            $result = mysql_query($query);
            if($result){
                $retval = true;
            }
        }
        return $retval;
    }

    Вроде все хорошо и красиво. Но есть говнинка которая все портит

    Запостил: Vasiliy, 19 Сентября 2010

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

    • Adres != Address ?
      Ответить
      • if($result)
        {
        $retval = true;
        }
        нет вот это . Не пойму зачем я код этого юноши просматриваю вроде и код нормальный а встречаются вот такие вещи и все сразу портит.
        Ответить
        • Я вот только не пойму что в этом такого плохого...
          Или возможно я ещё не достиг высшего уровня мастерства в программировании, поведай пожалуйста?
          Можно было конечно обойтись без этой переменной и сделать return $result;
          Я правило все понял?
          Ответить
          • а если не сработает "if(!empty($name))"..
            Ответить
            • function createTable($name) {
              if(!empty($name)){
              $query = "CREATE TABLE IF NOT EXISTS `" . $name . "` (`Adres` varchar(150) DEFAULT NULL, `send` int(1) DEFAULT NULL )";
              $result = mysql_query($query);
              return $result;
              }
              else return false;
              }
              Ответить
              • не вникая в подробности:
                func .... {
                   if(...) {
                      ...
                      return  mysql_query($query);
                   }
                   return false;
                }

                - наф плодить лишние переменные, которые не используются?..
                - да и ветка "else" по сути не нужна; но это на любителя...
                Ответить
                • показать все, что скрытоДве точки выхода - это плохо.
                  Ответить
                  • >Две точки выхода - это плохо
                    Тогда почему у человека две ноздри ?
                    Ответить
                    • Ну Вы и придумали сравнение...
                      Нос человека выполнен согласно MISC архитектуре, а в данном примере только RISC
                      Ответить
                    • Пример не точен ноздри это еще и точки входа

                      а Вот анус это точка выхода и он один. (всяких педерастов не рассматриваю)
                      Ответить
                      • <?php
                        use анус;
                        ob_end_flush();
                        ?>
                        Ответить
                      • >а Вот анус это точка выхода и он один
                        помимо ануса есть ещё и уретра
                        Ответить
                        • это точка выхода другой функции, результат иной
                          Ответить
                          • Но вход-то один и тот же.
                            Ответить
                          • вообще-то функция одна и так же (вывод ненужной херни)...
                            хотя есть и другая функция... но о ней речь не идёт =)
                            Ответить
                  • бред;
                    с точки зрения интерпретатора Несколько точек выхода не ухудшат производительность и даже наоборот: не придётся выполнять ненужные жампы на конец функции, чтобы выполнить жамп выхода из функции...


                    с точки зрения нуба(паскалист, школоло, и т.п.):
                    - несколько точек выхода заставляет напрягать мозг... и "трудно" разобраться в коде...
                    и правда,- лазить по всем ветвлениям, что куда пошло -- гораздо читабельнее чем "не устраивает -- пшол на ret"...
                    Ответить
                    • Когда трудно разобраться в чьем-то коде - этот кто-то, видимо, ошибся профессией.
                      Ответить
                      • Абсолютно не согласен. Бывает трудно, бывает легко.
                        Причем тут ошибка в профессии?
                        В любом деле могут быть как простые, так и сложные моменты.
                        Ответить
                        • Программа должна быть максимально читаема, если конечно речнь не идет о хардкорной оптимизации. Но в PHP такая оптимизация не нужна впринципе.
                          Вообще в современном мире она МОЖЕТ БЫТЬ нужна только в особых случаях: драйверы, и прочий код режима ядра, ядра каких-то крутых СУБД (типа оракл), системы для обработки гигантских массивов данных (типа поисковых машин гугла и яндекса) и иже с ними.

                          В PHP она не нужна. Так что код PHP в первую очередь должен быть читаемым.

                          Так что если код на PHP прочитать трудно -- значит автор кода скорее всего плохой программист, ага))

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

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

                            должен быть баланс между читабельностью и оптимальностью...
                            ---------------------------------------


                            но речь была даже не о том...
                            - речь была о "Н-ное количество выходов из функции == плоха"...
                            Ответить
                            • >>- речь была о "Н-ное количество выходов из функции == плоха"...
                              на самом деле это спорно.
                              Как бы Вы переписали эту функцию? (извините за синтаксис -- пых я подзабыл)
                              /**
                              * @returns user favorite beer. Returns null if has ulcer (язвенник)
                              **/
                              function getUserFavoriteBeer() {
                                  if ( hasUlcer()) {
                                     return null;
                                  }
                                  //20 строк по доставанию любимого пива
                                 return $beer;
                              }
                              Ответить
                              • function getUserFavoriteBeer() {
                                    //20 строк по доставанию любимого пива
                                   return $beer;
                                }

                                а в пользовательском коде:
                                $beer =  hasUlcer() ? null : getUserFavoriteBeer();
                                Ответить
                              • def get_favorite_beer
                                  ulser || heineken
                                end
                                Ответить
                          • Хотел бы заметить, что это не всегда верно относительно читаемости. Например использование ORM или просто следование convension over configuration идёт в ущерб понятности кода, но тем не менее очень эффективно. В большинстве других случаев вы конечно правы.
                            Ответить
                            • Окей, ORM -- штука особая.
                              Работа с базой почти всегда является узким местом PHP приложений.

                              Так что там можно пожертвовать читаемостью
                              Ответить
                • тут скорее
                  function createTable($name) {
                    if (empty($name)) return false;
                  
                    $query
                  }

                  а вообще глупая функция, что она именно сделала невозможно понять по результатам возвращаемого значения
                  Ответить
                  • а что тут непонятного она создаст таблицу если её нет. т.е. когда вернется true значт гарантированно табла с именем $name существует
                    Ответить
                    • да вы что такое говорите клёвое! А если вернёт false - значит таблица однозначно не существует?
                      Ответить
                      • а если вернет false то однозначно ошибка запрос не прошел.
                        Ответить
                        • мда, это вы объясняете что происходит по коду? Спасибо, у меня всё-таки есть глаза и голова.

                          Я вам ещё раз повторю свою мысль. Обратите внимание на то, что функция называется createTable. И нормальное для неё поведение - возвращать true - если таблица создана, если false, если не создана. Согласно коду логика другая. О чём я и сказал выше.

                          Если совсем непонятно - простой пример:
                          у вас есть метод find который ищет записи по id, например посты. Нормальное для него поведение - возвращать пост или null/false в случае если пост не был найден. А теперь мы добавили в метод логику, согласно которой, если пост не найден, то метод будет возвращать пустой объект Post. А сам метод не переименуем. Вот такого же рода бред в и функции. Название не соответствует действиям.
                          Ответить
                          • ИМХО, если псто не найден - лучше кидать эксепшен. Иначе будет непонятно, то ли псто не найден, то ли ошибка произошла при выполнении запроса.
                            Вы еще предложите коды ошибок возвращать.
                            Ответить
                            • >Вы еще предложите коды ошибок возвращать.
                              В php может и не актуально. А когда дело стоит за производительностью -
                              лучше код ошибки, чем исключение.
                              Ответить
                              • >>А когда дело стоит за производительностью -
                                >>лучше код ошибки, чем исключение.
                                Да, во многих средах исключения действительно хуже по производительности, чем код ошибки.

                                Но если Вы не собираетесь бросать over 1K исключений в секунду -- врядли Вы заметите от них хоть какой-то урон.

                                Ну а для PHP эито правда не актуально
                                Ответить
                                • >Но если Вы не собираетесь бросать over 1K исключений в секунду -- врядли Вы заметите от них хоть какой-то урон.
                                  Не обязательно их вообще даже генерировать пользователю.
                                  Сами библиотеки, которые поддерживают исключения более медленные, чем их
                                  свободные от исключений аналоги.
                                  Ответить
                                  • Я смотрю, вы любитель "кавычечной оптимизации"? Не думаю, что вы найдете проект, в котором перепасовка исключениями будет узким местом.
                                    Ответить
                            • я не писал выше как правильно обрабатывать поиск поста, я писал о другом, ваш коммент не в тему
                              Ответить
                              • Вы писали, что "нормальное для [функции] поведение - возвращать true - если таблица создана, если false, если не создана". Это неправильное поведение с точки зрения легкости сопровождения программы. Вот деплоите вы проект на боевой, а таблица не создается. Почему? Потому что false.
                                Ответить
                    • Да нахрена ж она нужна -- функция-то такая?
                      Ответить
    • 1) в таблице нет PK
      2) if($result){ ...
      не проверяет создана ли таблица, а проверяет только прошел ли запрос к БД.
      КО.
      Ответить
      • в случае ошибки будет
        $result===false

        Меня волнет выделение лишних переменных
        и отсктвие механизма логировния в случаи ошибки я бы делал так
        if ($result===false)
        {
        writelog('Error: '.__FILE__." ".__LINE__.mysql_error().' SQL '.CREATE TABLE IF NOT EXISTS `" . $name . "` (`Adres` varchar(150) DEFAULT NULL, `send` int(1) DEFAULT NULL )");
        }

        Ну или как то так в любом случае ошибка не осталось не замеченной а тут. Получилось хорошо не получилось ну и ладно потом про пыху и говорят плохо.
        Ответить
        • для чего там result
          if(mysql_query($query) == 0) {}

          не ?
          Тащемта, там абсолютно не нужны retval & result
          Ответить
          • результ там нужен чтобы функция значение вернула
            как там все нормально или были проблемы.
            Ответить
            • мы и без того значение вернуть можем, лишнее - ящитаю.
              Ответить
            • правильно товарищ заметил..
              вообще-то можно и вообще не плодить переменные...
              Ответить
            • это гуд, если ты потом хотишь использовать эту переменную, по типу:
              - обработка ошибок по свичу или там какое-то логирование...

              а если тебе нужно просто "да/нет", которое тут же и возвращается из функции, то плодить переменные нету смысла (это влечет за собою лишнюю нагрузку на сервер, пускай и незначительную, но такие задержки накапливаются по всему скрипту)
              Ответить
    • из двух названий столбцов товарищ умудрился сделать ошибки в двух.
      А вообще говно не функция, а говно такой подход.
      Или кто-то любит юзать DDL в своей программе и создавать по таблице на сущность,
      или кто-то вместо дампа SQL юзает свой код.
      И то и другое говно.
      Ответить
    • Я вот не пойму, кому понадобится создавать постоянно столько таблиц с таким набором полей?.. Это 1 раз наверное выполнится в каком-нибудь install.php, где юзеру будет предложено выбрать имя для таблицы с адресами :D
      Ответить
    • Мы забрызгивали Натку спермой с головы до ног! Буквально! Сперма была у нее на лице, на животе, на ногах, на волосах…
      Ответить

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