I read the documentation I want to bring the code to a normal form.

/** * Display forms adding the sell offer * * @param integer $id orders * @return mixed * @throws HttpException */ public function actionOfferSellForm($id = null) { $modelOfferBuyForm = new OfferBuyForm(); $order = Order::findOne($id); if (!$order) { throw new \yii\web\HttpException(404, 'Page not found'); } $certificate = Certificate::findOne(['id' => $order->source_id]); if (!$certificate) { throw new \yii\web\HttpException(404, 'Page not found'); } if ($order->user_id == Yii::$app->user->identity->id) { return $this->renderAjax('/site/info', ['message' => 'Вы не можете приобрести свой сертификат']); } $modelOfferBuyForm->source_id = $order->id; $modelOfferBuyForm->certificate_code = $certificate->certificate_code; $modelOfferBuyForm->buyer_id = Yii::$app->user->identity->id; $modelOfferBuyForm->seller_id = $order->user_id; if ($modelOfferBuyForm->load(Yii::$app->request->post()) && $modelOfferBuyForm->validate()) { $modelOfferBuyForm->status = 1; $modelOfferBuyForm->expertise = 0; $modelOfferBuyForm->priced_currency = $order->priced_currency; if ($modelOfferBuyForm->save()) { Yii::$app->session->setFlash('success', 'Заявка отправленна.'); return $this->redirect('index'); } else { Yii::$app->session->setFlash('error', 'Ошибка отправки заявки.'); } } elseif (Yii::$app->request->isAjax) { return $this->renderAjax('offerBuyForm', ['model' => $modelOfferBuyForm]); } else { return $this->render('offerBuyForm', ['model' => $modelOfferBuyForm]); } }

This code takes a Transaction and searches for a certificate, after which it performs manipulations. I want to make it more elegant, since if (!$order) { throw new \yii\web\HttpException(404, 'Page not found'); } $certificate = Certificate::findOne(['id' => $order->source_id]); if (!$certificate) { throw new \yii\web\HttpException(404, 'Page not found'); } if (!$order) { throw new \yii\web\HttpException(404, 'Page not found'); } $certificate = Certificate::findOne(['id' => $order->source_id]); if (!$certificate) { throw new \yii\web\HttpException(404, 'Page not found'); } if (!$order) { throw new \yii\web\HttpException(404, 'Page not found'); } $certificate = Certificate::findOne(['id' => $order->source_id]); if (!$certificate) { throw new \yii\web\HttpException(404, 'Page not found'); } I think not the best practice. well and other moments like i f ($modelOfferBuyForm->save()) { without validation

  • one
    What does this code do? and what specifically wanted to improve? - Grundy Nov.
  • one
    add this description directly to the question - Grundy

1 answer 1

  1. If you are confused by save without validate , then this is you in vain. As validation all the same happens, if only it forcibly not to muffle save(false) . If this form is inherited from ActiveRecord, if not, then it is logical to put validation in the save method itself.

  2. Validation in your code goes after load

  3. How about two queries pull through one with relay?

     $order = Order::find() ->where(['id' => $id]) ->with(['certificate']) ->one();

  4. Instead of magic numbers try to use constants, in a month another you don’t remember.

  5. With scenarios when you have "Error sending request", there is no redirect or render.

  6. Some kind of twisted logic. I think you need to revise it as a whole. I personally did not understand until the end when the action is processed through ajax, and when not. Maybe it makes sense to smash them?