0

В последнее время, чтобы избежать чрезмерно вложенных If я пользуюсь принципом раннего возврата. Однако возникают ситуации, когда этот принцип может "засорить" код. Например, сегодня нужно вернуть json с результатами, поэтому можно делать так:

public function canSendSms( ) {
        $shopId = $this->request->post['shop_id'];
        $dateEnd = $this->request->post['dateEnd'];
        $orderId = $this->request->post['order_id'];

        if ( ! $shopId || !$dateEnd || !$orderId ) {
            $result["error"][] = "Неверные данные";
            header("Content-type: application/json");
            return json_encode($result, JSON_UNESCAPED_SLASHES, JSON_UNESCAPED_UNICODE);
        }

        // Собираем оставшиеся данные
        $this->load->model('sale/order');
        $order_info = $this->model_sale_order->getOrder($orderId);
        if ( !$order_info ) {
            $result["error"][] = "Не получилось найти заказ под номером " . $orderId;
            header("Content-type: application/json");
            return json_encode($result, JSON_UNESCAPED_SLASHES, JSON_UNESCAPED_UNICODE);
        }

        // финальный код

        $result["success"][] = "Сообщение можно отправить";
        header("Content-type: application/json");
        return json_encode($result, JSON_UNESCAPED_SLASHES, JSON_UNESCAPED_UNICODE);
    }

Как видно из примера - тут 3 раза повторяются 2 строки вывода

header("Content-type: application/json");
return json_encode($result, JSON_UNESCAPED_SLASHES, JSON_UNESCAPED_UNICODE);

Если метод небольшой, и изменений не планируется, то проблем нет. Но что если таких проверок будет 10, а формат вывода поменяется с json на какой-нибудь другой? Ну да, придется тупо изменить вывод в этих 10 проверках.

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

public function canSendSms( $result = NULL ) {
        if ( $result ) {
            header("Content-type: application/json");
            echo json_encode($result, JSON_UNESCAPED_SLASHES, JSON_UNESCAPED_UNICODE);
            return;
        }

        $shopId = $this->request->post['shop_id'];
        $dateEnd = $this->request->post['dateEnd'];
        $orderId = $this->request->post['order_id'];

        if ( ! $shopId || !$dateEnd || !$orderId ) {
            $result["error"][] = "Неверные данные";
            return $this->canSendSms( $result );
        }

        // Собираем оставшиеся данные
        $this->load->model('sale/order');
        $order_info = $this->model_sale_order->getOrder($orderId);
        if ( !$order_info ) {
            $result["error"][] = "Не получилось найти заказ под номером " . $orderId;
            return $this->canSendSms( $result );
        }

        // какой-то код

        $result["success"][] = "Сообщение можно отправить";
        return $this->canSendSms( $result );
    }

Вопрос, насколько это правильно, насколько понимаемо и встречается ли подобное в вашей практике?

5
  • Очень мутно в итоге - зачем рекурсия? У вас в итоге есть алгоритм проверки заказа и отсылка - разделите это в две разных функции - пусть функция с проверкой называется checkOrder, функция отсылки - sendSms, вызываемая в конце функции checkOrder через return $this->sendSms($result), а еще лучше в корневом коде где будет решаться что делать с результатом проверки. Поместите отсылку в отдельную функцию и не плодите муть дальше. 16 мар 2018 в 7:49
  • предоставьте мне решать что и где, а так же как отвечать. Я ответил по сути - у вас в коде получается мешанина, в одной и той же функции два функционала, пусть даже и с "добрыми" намерениями. Поддерживать такой код после будет сложно. 16 мар 2018 в 7:57
  • А зачем вы в вашей функции canSendSms пишите echo если она должна возвращать результат, а не выводить его?
    – Bykuznec
    16 мар 2018 в 8:01
  • @Bykuznec эта функция обрабатывает ajax запрос. Если вернуть json через return, то js ничего не получит. Поэтому echo
    – Peresada
    16 мар 2018 в 8:08
  • $canSendSms = canSendSms($params); echo $canSendSms; - Вот вам и json
    – Bykuznec
    16 мар 2018 в 8:30

2 ответа 2

2

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

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

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

public function canSendSms($result = [])
{
    do {
        $shopId = $this->request->post['shop_id'];
        $dateEnd = $this->request->post['dateEnd'];
        $orderId = $this->request->post['order_id'];

        if (!$shopId || !$dateEnd || !$orderId) {
            $result["error"][] = "Неверные данные";
            break;
        }

        // Собираем оставшиеся данные
        $this->load->model('sale/order');
        $order_info = $this->model_sale_order->getOrder($orderId);
        if (!$order_info) {
            $result["error"][] = "Не получилось найти заказ под номером $orderId";
            break;
        }

        $result["success"][] = "Сообщение можно отправить";
    } while (false);

    header("Content-type: application/json");
    echo json_encode($result, JSON_UNESCAPED_SLASHES, JSON_UNESCAPED_UNICODE);
}

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

1
  • По сути первоначальная задача была избежать дополнительной вложенности операторов и чтобы при этом не нужно было дублировать финальный вывод, поэтому я использовал return так же, как вы используете в примере break, имея одиночный цикл. Но про юнит-тесты и на тему побочных эффектов не подумал. спасибо
    – Peresada
    16 мар 2018 в 8:41
0

Вы как-то странно ограниченны одной функцией. Вам надо понять что делает та или иная функция и разбить её на составляющие. Тут например у Вас есть следующие атомы:

  1. Проверка входных параметров
  2. Проверка валидности данных
  3. Отправка ответа.

Если Вы дополнительно реализуете их отдельными функциями, всё станет проще:

  1. ValidateParams params - на вход, true если всё хорошо, текст/код ошибки - если всё плохо
  2. ValidateData - аналогично
  3. Validate - функция которая последовательно применяет первые две
  4. SendResponce - Собственно формирование ответа

и тогда у вас получится что то типа

SendSms(){
res = Validate(...)
if(res==true){
    res = "Успешно"
}
SendResponce(res);
}
4
  • По сути в моем вопросе и представлена функция Validate, в которой проводится проверка валидности данных. Просто неудачно обозначил название изначально. Вопрос был в другом
    – Peresada
    16 мар 2018 в 8:15
  • 1
    Тогда не понятно в чём вопрос. Использовать рекурсию для того, чтобы одна и та же функция возвращала логически разный результат? Это попахивает извращением. И уязвимостью - я могу вызвать эту функцию с параметром и она ничего не проверит, а отдаст параметр в каком-то формате
    – Чад
    16 мар 2018 в 8:19
  • суть вопрос изложил в комментариях к ответу выше. Про уязвимость это понятно, но эта функция доступна только в административной панели
    – Peresada
    16 мар 2018 в 8:53
  • 1
    Вы так рассуждаете, пока у Вас Вы один разработчик кода. Это уязвимость с точки зрения написания кода. Ну не говоря уже что можно случайно что-то сделать даже из админской панели.
    – Чад
    16 мар 2018 в 8:57

Ваш ответ

Нажимая на кнопку «Отправить ответ», вы соглашаетесь с нашими условиями использования и подтверждаете, что прочитали и поняли наши политику конфиденциальности и нормы поведения.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.