There is a page with limited access. For each role - their data, which we form requests. It turns out about this design

if (Yii::$app->accessControl->isRoot()) { $dataProvider = new ActiveDataProvider([ 'query' => User::find(), ]); } elseif (Yii::$app->accessControl->isDirector()){ $dataProvider = new ActiveDataProvider([ 'query' => User::find() ->where(['NOT IN', 'id', [1]]), ]); } elseif (Yii::$app->accessControl->isManager()){ $dataProvider = new ActiveDataProvider([ 'query' => User::accessManager() ]); } else { $dataProvider = new ActiveDataProvider([ 'query' => User::accessAdministrator() ]); }

Tell me, is it possible in some way to make this code more cultured?)

  • What's wrong with this code? This non-cultural code swears?) Well, replace if with switch. Or are you looking for a Design Pattern? - koks_rs

2 answers 2

If the simplest code doesn’t go into what is happening in the methods, then I would do this:

 if (Yii::$app->accessControl->isRoot()) { $query = User::find(); } elseif (Yii::$app->accessControl->isDirector()){ $query = User::find()->where(['NOT IN', 'id', [1]]); } elseif (Yii::$app->accessControl->isManager()){ $query = User::accessManager(); } else { $query = User::accessAdministrator(); } $dataProvider = new ActiveDataProvider([ 'query' => $query ]);

It would be possible to transfer all this logic to the method by type:

 public function index() { $dataProvider = new ActiveDataProvider([ 'query' => $this->getUsersQuery(); ]); } protected function getUsersQuery() { if (Yii::$app->accessControl->isRoot()) { $query = User::find(); } elseif (Yii::$app->accessControl->isDirector()){ $query = User::find()->where(['NOT IN', 'id', [1]]); } elseif (Yii::$app->accessControl->isManager()){ $query = User::accessManager(); } else { $query = User::accessAdministrator(); } return $query; }

But since there is no data as a whole, it is only possible to simplify the output

    You can use this technique with switch:

     switch (true) { case Yii::$app->accessControl->isRoot(): $dataProvider = new ActiveDataProvider([ 'query' => User::find(), ]); break; case Yii::$app->accessControl->isDirector(): $dataProvider = new ActiveDataProvider([ 'query' => User::find() ->where(['NOT IN', 'id', [1]]), ]); break; case Yii::$app->accessControl->isManager(): $dataProvider = new ActiveDataProvider([ 'query' => User::accessManager() ]); break; default: $dataProvider = new ActiveDataProvider([ 'query' => User::accessAdministrator() ]); break; }

    It turns out quite clearly, like a human language, it is convenient to write comments. Pushes to write short human-readable conditions in the case (if the condition is long / complex, we put it in a separate method).