За последние 24 часа нас посетили 28623 программиста и 1815 роботов. Сейчас ищут 1178 программистов ...

Два простых вопроса о правильном коде

Тема в разделе "PHP для новичков", создана пользователем php_user, 7 авг 2014.

  1. php_user

    php_user Новичок

    С нами с:
    19 апр 2014
    Сообщения:
    91
    Симпатии:
    0
    1)- Какой порядок грамотнее из двух приведенных ниже?
    2)- Как во втором примере грамотно перевернуть выражение (счас там стоит <=, но подозреваю что это не совсем так как нужно)?
    Код (PHP):
    1. if ($result->num_rows > 0)
    2. {
    3. ОСНОВНОЕ ДЕЙСТВИЕ
    4. }
    5. else
    6. {
    7. СООБЩЕНИЕ ОБ ОШИБКЕ
    8. } 
    Код (PHP):
    1. if ($result->num_rows <= 0)
    2. {
    3. СООБЩЕНИЕ ОБ ОШИБКЕ
    4. }
    5. else
    6. {
    7. ОСНОВНОЕ ДЕЙСТВИЕ
    8. } 
     
  2. VLK

    VLK Старожил

    С нами с:
    15 дек 2013
    Сообщения:
    3.010
    Симпатии:
    58
    ты проверяешь есть ли в БД пользователь с таким логином и паролем, если есть у тебя $result->num_rows = 1 если нет, у тебя $result->num_rows = 0

    выражение if ($result->num_rows <= 0) { ... в корне не верно, т.к. в обоих случаях будет выдавать true

    по этому only первый вариант.
     
  3. php_user

    php_user Новичок

    С нами с:
    19 апр 2014
    Сообщения:
    91
    Симпатии:
    0
    просто я хочу в конце выводить основное действие, а до него всякие доп.проверки (как в примере 2), поэтому и вопрос: как правильно перевернуть условия (может знаком ! или еще как)

    Добавлено спустя 1 минуту 32 секунды:
    почему в обоих- там же не >= а <=. Прошу обратить внимание

    Добавлено спустя 1 минуту 19 секунд:
    т.е. вопрос больше касается оформления кода, а не логики
     
  4. dapperkop

    dapperkop Активный пользователь

    С нами с:
    26 сен 2013
    Сообщения:
    890
    Симпатии:
    0
    Да по барабану. Делай как по кайфу. Вот только, если количество строк равно нулю, то это не ошибка. Я бы сделал так:

    Код (PHP):
    1. if ($result->num_rows === 0) {
    2.     // нет записей
    3. } else {
    4.     // we go
    5. }
     
  5. VLK

    VLK Старожил

    С нами с:
    15 дек 2013
    Сообщения:
    3.010
    Симпатии:
    58
    когда я писал сообщение там за место второго варианта было не пойми что, но все равно, значение $result->num_rows не может быть меньше 0, по этому зачем проверять меньше оно нуля или нет.
    Первый вариант правильнее.

    Я помниться когда то писал кучу вложений во вложениях типа:
    Код (PHP):
    1. if ( !empty($val) ) {
    2.     
    3.     if ($val > 0) {
    4.         // ...
    5.     }
    6.     else {
    7.         // ...
    8.     }
    9. }
    10. else {
    11.     // ...
    12. }
    а потом нашел выход попроще, создаешь функцию и пишешь так:
    Код (PHP):
    1. function check($val){
    2.  
    3. if ( empty($val) ) {
    4.     return 'пусто';
    5. }
    6.  
    7. if ($val < 0) {
    8.     return 'меньше нуля';
    9. }
    10.  
    11. return true;
    12.  
    13. } 
    например задача функции проверить полученную переменную на соответствие требованиям и если она соответствует вернуть true, если она не соответствует, то дело до true и из функции отправит раньше.

    И покороче и не утонешь в коде.
     
  6. romach

    romach Старожил

    С нами с:
    26 окт 2013
    Сообщения:
    2.904
    Симпатии:
    719
    имхо, делать можно и так и так, главное что бы это правило (как и любые другие) соблюдались во всем коде.
    Код (PHP):
    1. if ($true) {
    2.   //код
    3. } else {
    4.  //код
    5. }
    6.  
    з.ы. если ты озаботился правильным оформленим кода, то советую обратить внимание на PSR - http://www.php-fig.org/
     
  7. mkramer

    mkramer Суперстар
    Команда форума Модератор

    С нами с:
    20 июн 2012
    Сообщения:
    8.599
    Симпатии:
    1.764
    Хм, а я читал про ифы другое правило - первым должен идти меньший по объёму блок, чтоб else не было скрыто за тремя страницами кода. ИМХО, разумно
     
  8. romach

    romach Старожил

    С нами с:
    26 окт 2013
    Сообщения:
    2.904
    Симпатии:
    719
    Это правило придумали мракобесы ) Если блок разрастается на три страницы, то его нужно делить на составные куски.
     
  9. artoodetoo

    artoodetoo Суперстар
    Команда форума Модератор

    С нами с:
    11 июн 2010
    Сообщения:
    11.128
    Симпатии:
    1.248
    Адрес:
    там-сям
    Есть скрипты автоматического рецензирования кода, которые могут выдать заключение вроде "слишком большая вложенность блоков" или "слишком много строк в методе". Это не догма, конечно, но и не на пустом месте родилось. Код по мере распухания становится менее понятным.

    У меня есть такое личное правило: если один из вариантов if-else заканчивается прерыванием выполнения — return, exit() или break — я ставлю этот блок первым, а второй освобождаю от скобок. Код становится чуть чище и чуть короче.

    Вместо
    Код (PHP):
    1. if ($a > 0) {
    2.     alfa();
    3.     beta();
    4. } else {
    5.     gama();
    6.     return;
    7. } 
    делаю
    Код (PHP):
    1. if ($a <= 0) {
    2.     gama();
    3.     return;
    4. }
    5. alfa();
    6. beta(); 
    Добавлено спустя 7 минут 41 секунду:
    Немного оффтопик:
    PHP Code Sniffer (phpcs) , можно запускать из командной строки или интегрировать с NetBeans и другими IDE. Содержит богатый набор стилистических правил. Как вариант, он может проверять на соответствие стандартам PSR.
     
  10. mkramer

    mkramer Суперстар
    Команда форума Модератор

    С нами с:
    20 июн 2012
    Сообщения:
    8.599
    Симпатии:
    1.764
    artoodetoo, да, я тоже так люблю делать. Я вообще люблю и break, и continue, хоть это и скрытые goto по сути. По мне, так:
    Код (Text):
    1.  
    2. while ($a = f()) {
    3.     if ($a["str"] == "")
    4.        continue;
    5.     // 10-15 строк по обработке $a
    6. }
    читается лучше, чем
    Код (Text):
    1.  
    2. while ($a != f()) {
    3.     if ($a["str"] != "") {
    4.        // 10-15 строк по обработке $a
    5.     }
    6. }
     
  11. [vs]

    [vs] Суперстар
    Команда форума Модератор

    С нами с:
    27 сен 2007
    Сообщения:
    10.559
    Симпатии:
    632
    Если вы пользуетесь исключениями, то лучше второй вариант, т.к. else вообще не нужен.
    Код (PHP):
    1. try {
    2.  if ($result->num_rows <= 0)
    3.  {
    4.   throw new Exception;
    5.  }
    6.  ОСНОВНОЕ ДЕЙСТВИЕ
    7. } catch ($e) {
    8.  СООБЩЕНИЕ ОБ ОШИБКЕ
    9. } 
     
  12. kamael

    kamael Новичок

    С нами с:
    29 июн 2014
    Сообщения:
    9
    Симпатии:
    0
    Все чтото городят - а var_dump() сложно было выложить. и условия.

    Если там не массив а объект? А если массив?

    Нет такого правильно и нет - нужно условие и то что результе...
    Тогда можно и чтото ответить.
     
  13. igordata

    igordata Суперстар
    Команда форума Модератор

    С нами с:
    18 мар 2010
    Сообщения:
    32.408
    Симпатии:
    1.768
    Я делаю как artoodetoo. Считаю это проще читаемым. Еще и экскпшн можно заюзать. Если кидать через функцию, то вообще в одну строку.
     
  14. igordata

    igordata Суперстар
    Команда форума Модератор

    С нами с:
    18 мар 2010
    Сообщения:
    32.408
    Симпатии:
    1.768
    соотв. я юзаю такие вот функции:
    Код (PHP):
    1. <?php
    2.  
    3. /**
    4.  * Просто кидает эксепшн.
    5.  * @param String $message
    6.  * @param Integer $number
    7.  * @throws Exception
    8.  */
    9. function ThrowE($message='', $number=0) {
    10.   throw new Exception($message, $number);
    11. }
    12.  
    13.  
    14. /**
    15.  * Эта функция кидает эксепшн если вар равен true, соотв кидает его с меседжем и номером, как передали.
    16.  * @param $var Проверяется на нестрогое равенство true и если да - кидается эксепшн.
    17.  * @param string $message Текст сообщения в эксепшене.
    18.  * @param int $number Номер эксепшена.
    19.  * @return bool Всегда true.
    20.  * @throws Exception
    21.  */
    22. function ThrowOnTrue($var, $message = '', $number = 0) {
    23.   if ($var) {
    24.     throw new Exception($message, $number);
    25.   }
    26.   return true;
    27. }
    28.  
    29. /**
    30.  * Эта функция кидает эксепшн если вар равен фалсу или нулл, соотв кидает его с меседжем и номером, как передали.
    31.  * @param $var Проверяется на нестрогое равенство false и если да - кидается эксепшн.
    32.  * @param string $message Текст сообщения в эксепшене.
    33.  * @param int $number Номер эксепшена.
    34.  * @return bool Всегда true.
    35.  * @throws Exception
    36.  */
    37. function ThrowOnFalse($var, $message = '', $number = 0) {
    38.   if (!$var) {
    39.     throw new Exception($message, $number);
    40.   }
    41.   return true;
    42. }
    43.  
    44. /**
    45.  * Кидает эксепшн в случае, если переменная не является массивом, или массив пуст.
    46.  * @param Array $array
    47.  * @param string $message Текст сообщения в эксепшене.
    48.  * @param int $number Номер эксепшена.
    49.  * @return bool Всегда true.
    50.  * @throws Exception
    51.  */
    52. function ThrowOnBadArray($array, $message = '', $number = 0) {
    53.   if (!is_array($array) OR empty($array)) {
    54.     throw new Exception($message, $number);
    55.   }
    56.   return true;
    57. }
    Которые дают такой вот простой код:
    Код (PHP):
    1.     try {
    2.       $id = (int)$id;
    3.       $enabled = (int)$enabled;
    4.       if ($enabled == 1) {
    5.         //должно быть право на отмену задачи
    6.         ThrowOnFalse(User::right('requests tasks cancel'), 'no right to cancel');
    7.       }
    8.       ThrowOnFalse(BSMDB::query("UPDATE `requests_tasks` SET `enabled` = $enabled, `closed` = NOW(), `closed_by` = " . User::$id . " WHERE `id` = $id"), 'cant update');
    9.       $task = BSMDB::firstrow("SELECT * FROM `requests_tasks` WHERE `id` = $id");
    10.       ThrowOnFalse($task, 'no task');
    11.       $request = BSMDB::firstrow("SELECT * FROM `requests` WHERE `id` = {$task['request']}");
    12.       ThrowOnFalse($request, 'no request');
    13.       Requests::$managers = self::getManagers();
    14.       ThrowOnFalse(Requests::$managers, 'no managers');
    15.       ThrowOnFalse(self::refreshRequest($request['id']), 'refresh failed');
    16.       Requests::DrawRequest($request);
    17.     } catch (Exception $exc) {
    18.       echo 'ERROR: ' . $exc->getMessage();
    19.     }
    и никаких ифов вообще. каждая следующая строка возможна только если всё прошло гладко и всё что нужно для нее - есть.