write a better code

Here is my code




$bookId= Yii::$app->request->post('id');

        

if ($bookId == NULL) return [];

        

$book= Book::findOne($costCodeId);

        

if ($book== NULL) return [];




There must be a better way, is the below equivalent to the above ?? I feel something is not right, with the $book assignment




$bookId= Yii::$app->request->post('id');


if ($bookId == NULL OR ($book = Book::findOne($costCodeId)) == NULL) ) return []


//do something with $book




The two example looks equal. But I prefer this:




$bookId= Yii::$app->request->post('id');


if ($bookId != NULL) {

    $book = Book::findOne($bookId);

}


if ($book == NULL) {

    return [];

}


// do something with $book



Because you have only one return point, it’s easier to debugging and better to understand what happen there.

Your code will crash if $bookId is NULL




if (($book= Book::findOne(Yii::$app->request->post('id')) !== null)){

        return $book;

}



I assume that you meant $bookId and not $costcodeid. If not then you should post all of your code.

I usually use something like this.




<?php


namespace app\controllers;


use Yii;

use app\models\Book;

use yii\web\Controller;

use yii\web\NotFoundHttpException;


class BookController extends Controller

{

    public function actionBook()

    {

        $model = $this->findBook(Yii::$app->request->post('id'));

        var_dump($model);

    }

    

    /**

     * Finds the Book model based on its primary key value.

     * If the model is not found, a 404 HTTP exception will be thrown.

     * @param integer $id

     * @return Book the loaded model

     * @throws NotFoundHttpException if the model cannot be found

     */

    protected function findBook($id)

    {

        if (($model = Book::findOne((int) $id)) !== null) {

            return $model;

        } else {

            throw new NotFoundHttpException('Your message here');

        }

    }

}



You should only do this if you use the function more than once or it’s a waste of typing and makes it harder to manage code wise. Also, you should remove the (int) before the id. This would only make this work in a int. based primary key field (i.e. this wouldn’t work in MongoDb however, removing it will make it work in both types of dbs) and it’s also unnecessary making it one more place the code could give you an issue.

hehe, my mistake when I try to replace my code with something else. Thanks for suggestion

Good points, that was the reason I added the I usually use like this.

Pretty hard to write code in the comments fields without any extra info, but usually I load the the same model many times within the same controller. If you don’t, please make a better usage of the same style, or create your own.

:blink: