Optimization Controller and Model

Yii 2.0 General Discussions
1

Hi I read about in the Documentation that the Controller should be much thinner than the Models. Based on the mass of generated common model files for the DB it fits. But based on the lines of code and the code density the Site-Controller is quite fat.

So the question or discussion is: What is sensible to optimize in the Controller?

For example:

The fattest thing is the actionIndex for logged-in users where many information are provided. And where a lot of information comes from different DB tables. For sure the user information itself can be populated in the Models as well as relational data. But now what is with something like this:




    public function getCandidates()

    {

        $user_employers = UserEmployer::find()->innerJoinWith('roles')->where([

            'auth_assignment.item_name' => 'pending',

            'user_employer.employer_id' => Yii::$app->user->identity->getCompInfo()['company_id'],

        ])->all();

        foreach ($user_employers as $user_employer) {

            $_user = UserDetails::findOne($user_employer->user_id);

            $user[] = array('id' => $user_employer->user_id, 'name' => $_user->last_name, 'title' => $_user->title, 'image' => $_user->avatar);

        }

        return isset($user) ? $user : null;

    }

I put this kind of functions in my controller what inflates it. Also I have 57 DB Queries - they are loaded fast in 59ms. But This function adds 3 Queries and so 57 feels like to much?

So the final question is: Is this the wrong place or the wrong way to provide arrays like the $user one here for the actionIndex?

I am very interested in your tips how a well-designed application should look like.

2

  1. You can get UserEmployer along with User using joinWith: http://www.yiiframework.com/doc-2.0/guide-db-active-record.html#joining-with-relations.

  2. If controller is getting too fat you can move such data collection methods into separate class.

3

In my opinion is a mix of optimisation and personal opinion…

For me model must be data provider and all logic must reside in controller as MVC should be.

This way you find the code where you expect.

Going back to you code your foreach create anyway data so you can move it in a model.

But these data are required just in the index action so rather than moving in the main model and making it fat with a function that aggregate data from different model I would create a new model which handle these ( and may be some other) data.

If you want to create a model that expand the basic one or create a new one is up to you.

This way the controller became slim, the main model do not became fat with a function that is called only in one specific place, the code will result more organized.

that’s my 2 cents.

This is because you use lazy loading for the query.

http://www.yiiframework.com/doc-2.0/guide-db-active-record.html#lazy-and-eager-loading

4

Okay and what is with things like this:




        $_role = Yii::$app->user->identity->getRole();

        $_details = Yii::$app->user->identity->getCompInfo();

        if (($_role == 'company')) {

            return $this->render('index', array(

                    'compdet' => $_details,

                    'details' => Yii::$app->user->identity->getDetails(),

                    'data' => $_role,

                    'tends' => $this->getTenders(),

                    'company_name' => Yii::$app->user->identity->getEmployerName(),

                    'users' => $this->getEmployers(),

                    'description' => !empty($_details['self_describe']) ? $_details['self_describe'] : Yii::t('lang', 'Your text')

                )

            );

        }

Everytime I call a method it is a query. I could reduce those by merging the data before and use eager loading. But then I have huge arrays with all information in one variable. And this could be quite difficult in reading the code afterwards or?

And this:




    public function getDetails()

    {

        $_user = self::find()->where(['user.id' => $this->id])->with(

            'userDetails',

            'authAssignment',

            'userEmployer'

        )->one();

        $details = array(

            'id' => $_user->userDetails->user_id,

            'employer_id' => '',

            'prename' => $_user->userDetails->pre_name,

            'lastname' => $_user->userDetails->last_name,

            'title' => $_user->userDetails->title,

            'position' => $_user->userDetails->position,

            'mailpre' => $_user->userDetails->mail_pre,

            'avatar' => $_user->userDetails->avatar,

            'role' => $_user->authAssignment->item_name

        );

        return $details;

    }

produces 16 more connections than grabbing each table by itself. in it’s own method. So populating data in one query (i hope i made eager loading) isn’t with good performance?

5

joinWith, not with.

6

Still the same. This produces 5 more connections than leaving joinWith('role') out and do it with a separate method.




    public function getDetails()

    {

        $user_details = UserDetails::find()->where([

            'user_details.user_id' => $this->getId()

        ])->joinWith('role')->all();

        foreach ($user_details as $_details) {

            $details = array(

                'id' => $_details->user_id,

                'prename' => $_details->pre_name,

                'lastname' => $_details->last_name,

                'title' => $_details->title,

                'position' => $_details->position,

                'mailpre' => $_details->mail_pre,

                'avatar' => $_details->avatar,

                'role' => $_details->role->item_name

            );

        }

        return $details;

    }

Is the relation a problem?




    public function getRole()

    {

        return $this->hasOne(AuthAssignment::className(), ['user_id' => 'user_id']);

    }

And in the documentation eager-loading is described with:




// SQL executed: SELECT * FROM customer LIMIT 100;

//               SELECT * FROM orders WHERE customer_id IN (1,2,...)

$customers = Customer::find()->limit(100)

    ->with('orders')->all();


foreach ($customers as $customer) {

    // no SQL executed

    $orders = $customer->orders;

    // ...handle $orders...

}

Is it possible that eager-loading just makes sense for more than 4 queries? Because as described 1+M+N produce this amount because here is just one table.

So finally eager-loading is not a good way to populate my data instead of single requests?

7

You should do joinWith for all relations you’re using if you want less queries.