writesdown /
app-cms
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like MenuController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use MenuController, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 30 | class MenuController extends Controller |
||
| 31 | { |
||
| 32 | /** |
||
| 33 | * @inheritdoc |
||
| 34 | */ |
||
| 35 | View Code Duplication | public function behaviors() |
|
| 59 | |||
| 60 | /** |
||
| 61 | * Displays menu page consists of CRUD for Menu and MenuItem model. |
||
| 62 | * |
||
| 63 | * @param null $id |
||
| 64 | * @return mixed |
||
| 65 | */ |
||
| 66 | public function actionIndex($id = null) |
||
| 88 | |||
| 89 | /** |
||
| 90 | * Creates a new Menu model. |
||
| 91 | * If creation is successful, the browser will be redirected to the 'index' page. |
||
| 92 | * |
||
| 93 | * @return mixed |
||
| 94 | */ |
||
| 95 | public function actionCreate() |
||
| 105 | |||
| 106 | /** |
||
| 107 | * Updates an existing Menu model. |
||
| 108 | * If update is successful, the browser will be redirected to the 'view' page. |
||
| 109 | * |
||
| 110 | * @param integer $id |
||
| 111 | * @return mixed |
||
| 112 | */ |
||
| 113 | public function actionUpdate($id) |
||
| 127 | |||
| 128 | /** |
||
| 129 | * Deletes an existing Menu model. |
||
| 130 | * If deletion is successful, the browser will be redirected to the 'index' page. |
||
| 131 | * |
||
| 132 | * @return mixed |
||
| 133 | * @throws NotFoundHttpException |
||
| 134 | * @throws \Exception |
||
| 135 | */ |
||
| 136 | public function actionDelete() |
||
| 143 | |||
| 144 | /** |
||
| 145 | * Create MenuItem models. |
||
| 146 | * |
||
| 147 | * @param int $id |
||
| 148 | * @return string |
||
| 149 | */ |
||
| 150 | public function actionCreateMenuItem($id) |
||
| 194 | |||
| 195 | /** |
||
| 196 | * Delete an existing MenuItem model via AJAX request. |
||
| 197 | * |
||
| 198 | * @throws NotFoundHttpException |
||
| 199 | * @throws \Exception |
||
| 200 | */ |
||
| 201 | public function actionDeleteMenuItem() |
||
| 216 | |||
| 217 | /** |
||
| 218 | * Save menu item recursively based on parent and child. |
||
| 219 | * |
||
| 220 | * @param array $menuOrder |
||
| 221 | * @param int $menuParent |
||
| 222 | */ |
||
| 223 | protected function saveMenuItem($menuOrder, $menuParent = 0) |
||
| 224 | { |
||
| 225 | foreach ($menuOrder as $key => $order) { |
||
| 226 | $menuItem = Yii::$app->request->post('MenuItem')[$order['id']]; |
||
| 227 | if ($model = $this->findMenuItem($order['id'])) { |
||
| 228 | $model->setAttributes($menuItem); |
||
| 229 | $model->setAttributes([ |
||
| 230 | 'parent' => $menuParent, |
||
| 231 | 'order' => $key, |
||
| 232 | ]); |
||
| 233 | if ($model->save()) { |
||
| 234 | if ($orderItems = ArrayHelper::getValue($order, 'items')) { |
||
| 235 | $this->saveMenuItem($orderItems, $model->id); |
||
| 236 | } |
||
| 237 | } |
||
| 238 | } |
||
| 239 | } |
||
| 240 | } |
||
| 241 | |||
| 242 | /** |
||
| 243 | * Finds the Menu model based on its primary key value. |
||
| 244 | * If the model is not found, a 404 HTTP exception will be thrown. |
||
| 245 | * |
||
| 246 | * @param integer $id |
||
| 247 | * @return Menu the loaded model |
||
| 248 | * @throws NotFoundHttpException if the model cannot be found |
||
| 249 | */ |
||
| 250 | protected function findModel($id) |
||
| 258 | |||
| 259 | /** |
||
| 260 | * Finds the MenuItem model based on its primary key value. |
||
| 261 | * If the model is not found, it will return false. |
||
| 262 | * |
||
| 263 | * @param integer $id |
||
| 264 | * @return MenuItem the loaded model |
||
| 265 | * @throws NotFoundHttpException if the model cannot be found |
||
| 266 | */ |
||
| 267 | protected function findMenuItem($id) |
||
| 275 | |||
| 276 | /** |
||
| 277 | * Finds the Post model based on its primary key value. |
||
| 278 | * If the model is not found, it will return false. |
||
| 279 | * |
||
| 280 | * @param integer $id |
||
| 281 | * @return Post the loaded model |
||
| 282 | * @throws NotFoundHttpException if the model cannot be found |
||
| 283 | */ |
||
| 284 | protected function findPost($id) |
||
| 292 | |||
| 293 | /** |
||
| 294 | * Finds the Term model based on its primary key value. |
||
| 295 | * If the model is not found, it will return false. |
||
| 296 | * |
||
| 297 | * @param integer $id |
||
| 298 | * @return Term the loaded model |
||
| 299 | * @throws NotFoundHttpException if the model cannot be found |
||
| 300 | */ |
||
| 301 | protected function findTerm($id) |
||
| 309 | } |
||
| 310 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.