Recently, to avoid overly nested If I use the principle of early return. However, there are situations when this principle can "clog" the code. For example, today you need to return json with the results, so you can do this:

 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); }

Apparently from an example - here 2 lines of an output here are repeated 3 times

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

If the method is small and no changes are planned, then there are no problems. But what if there are 10 such checks, and the output format changes from json to some other one? Well, yes, you have to stupidly change the output in these 10 checks.

In this regard, I began to use recursion, as a result, the final return is now always in such cases at the beginning of the method. It looks that way.

 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 ); }

The question is, how correct is it, how understandable, and is there anything similar in your practice?

  • Very dull in the end - why recursion? You end up with an algorithm for checking the order and sending - divide it into two different functions - let the checked function be called checkOrder, the send function is sendSms, called at the end of the checkOrder function via return $ this-> sendSms ($ result), and even better in the root code where it will be decided what to do with the result of the check. Put the reference in a separate function and do not propel the slime further. - Daniel Protopopov
  • let me decide what and where, as well as how to respond. I answered in essence - you have a jumble in the code, in the same function there are two functionalities, even with “good” intentions. Maintain this code after it will be difficult. - Daniel Protopopov
  • And why do you write echo in your canSendSms function if it should return the result and not output it? - Bykuznec
  • @Bykuznec this function handles ajax request. If you return json via return, then js will not get anything. Therefore echo - Peresada
  • $ canSendSms = canSendSms ($ params); echo $ canSendSms; - So much for json - Bykuznec

2 answers 2

Your function does two different tasks. It’s as if you mix tea with a spoon and then turn the pages of the book to it. Agree - strange.

You need some other function that you will call from this. Then there will be no such questions. Recursion is not needed for that and it looks wrong.

If another reason makes it difficult to do something, then you can use exceptions (worse) or one-time cycle (better):

 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); }

Otherwise, you should think very well about the structure of the program . For example, what would a unit test look like for this function if it immediately writes data with headers? ..

  • In fact, the original task was to avoid additional nesting of operators and so that it wouldn’t be necessary to duplicate the final output, so I used return just like you use break in the example, having a single loop. But I didn’t think about unit tests and side effects. thanks - Peresada

You are somehow strangely limited to one function. You need to understand what this or that function does and break it into components. Here, for example, you have the following atoms:

  1. Input Parameter Check
  2. Validation of data
  3. Sending an answer.

If you additionally implement them with separate functions, everything will become easier:

  1. ValidateParams params - at the entrance, true if everything is good, text / error code - if everything is bad
  2. ValidateData - Likewise
  3. Validate is a function that consistently applies the first two
  4. SendResponce - Actually forming the response

and then you get something like

 SendSms(){ res = Validate(...) if(res==true){ res = "Успешно" } SendResponce(res); }
  • In fact, in my question the function Validate is presented, in which the validity of the data is checked. Just unsuccessfully designated the name initially. The question was different - Peresada
  • one
    Then it is not clear what the question is. Use recursion to ensure that the same function returns a logically different result? It smacks of perversion. And a vulnerability - I can call this function with a parameter and it will not check anything, but give the parameter in some format - Chad
  • The essence of the question is stated in the comments to the answer above. About the vulnerability is understandable, but this feature is only available in the administrative panel - Peresada
  • one
    You reason like that, as long as you have one developer of the code. This is a vulnerability in terms of writing code. Well, not to mention that you can accidentally do something even from the admin panel. - Chad