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 BaseBuilder 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 BaseBuilder, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
17 | class BaseBuilder extends GenericBaseBuilder |
||
18 | { |
||
19 | /** |
||
20 | * @var array |
||
21 | */ |
||
22 | protected $columns = null; |
||
23 | |||
24 | /** |
||
25 | * @var array |
||
26 | */ |
||
27 | protected $actions = null; |
||
28 | |||
29 | /** |
||
30 | * @var array |
||
31 | */ |
||
32 | protected $objectActions = null; |
||
33 | |||
34 | /** |
||
35 | * @var string |
||
36 | */ |
||
37 | protected $columnClass = 'Column'; |
||
38 | |||
39 | public function getBaseAdminTemplate() |
||
43 | |||
44 | /** |
||
45 | * Return a list of columns from list.display. |
||
46 | * |
||
47 | * @return array |
||
48 | */ |
||
49 | public function getColumns() |
||
58 | |||
59 | protected function addColumn(Column $column) |
||
63 | |||
64 | protected function findColumns() |
||
73 | |||
74 | public function getColumnGroups() |
||
84 | |||
85 | /** |
||
86 | * Creates new column instance. |
||
87 | * |
||
88 | * @param string $columnName The name of the column. |
||
89 | * @param bool $withForms If true, add column form configuration. |
||
90 | * |
||
91 | * @return Column |
||
92 | */ |
||
93 | protected function createColumn($columnName, $withForms = false) |
||
203 | |||
204 | protected function getColumnClass() |
||
208 | |||
209 | public function setColumnClass($columnClass) |
||
213 | |||
214 | /** |
||
215 | * @param Column $column |
||
216 | * @param string $optionName |
||
217 | * @param string $default |
||
218 | * |
||
219 | * @return string |
||
220 | */ |
||
221 | protected function getFieldOption(Column $column, $optionName, $default = null) |
||
228 | |||
229 | protected function setUserColumnConfiguration(Column $column) |
||
238 | |||
239 | public function getFieldGuesser() |
||
243 | |||
244 | /** |
||
245 | * @return array Display column names |
||
246 | */ |
||
247 | protected function getDisplayColumns() |
||
248 | { |
||
249 | $display = $this->getVariable('display'); |
||
250 | |||
251 | // tabs |
||
252 | View Code Duplication | if (null == $display || (is_array($display) && 0 == count($display))) { |
|
253 | $tabs = $this->getVariable('tabs'); |
||
254 | |||
255 | if (null != $tabs || (is_array($tabs) && 0 < count($tabs))) { |
||
256 | $display = array(); |
||
257 | |||
258 | foreach ($tabs as $tab) { |
||
259 | $display = array_merge($display, $tab); |
||
260 | } |
||
261 | } |
||
262 | } |
||
263 | |||
264 | View Code Duplication | if (null == $display || (is_array($display) && 0 == count($display))) { |
|
265 | return $this->getAllFields(); |
||
266 | } |
||
267 | |||
268 | if (isset($display[0])) { |
||
269 | return $display; |
||
270 | } |
||
271 | |||
272 | //there is fieldsets |
||
273 | $return = array(); |
||
274 | |||
275 | foreach ($display as $fieldset => $rows_or_fields) { |
||
276 | foreach ($rows_or_fields as $fields) { |
||
277 | if (is_array($fields)) { //It s a row |
||
278 | $return = array_merge($return, $fields); |
||
279 | } else { |
||
280 | $return[$fields] = $fields; |
||
281 | } |
||
282 | } |
||
283 | } |
||
284 | |||
285 | return $return; |
||
286 | } |
||
287 | |||
288 | /** |
||
289 | * Retrieve all columns. |
||
290 | * |
||
291 | * @return array |
||
292 | */ |
||
293 | protected function getAllFields() |
||
297 | |||
298 | /** |
||
299 | * @return array |
||
300 | */ |
||
301 | public function getFieldsets() |
||
302 | { |
||
303 | $display = $this->getVariable('display'); |
||
304 | |||
305 | // tabs |
||
306 | View Code Duplication | if (null == $display || (is_array($display) && 0 == count($display))) { |
|
307 | $tabs = $this->getVariable('tabs'); |
||
308 | |||
309 | if (null != $tabs || (is_array($tabs) && 0 < count($tabs))) { |
||
310 | $display = array(); |
||
311 | |||
312 | foreach ($tabs as $tab) { |
||
313 | $display = array_merge($display, $tab); |
||
314 | } |
||
315 | } |
||
316 | } |
||
317 | |||
318 | View Code Duplication | if (null == $display || (is_array($display) && 0 == count($display))) { |
|
319 | $display = $this->getAllFields(); |
||
320 | } |
||
321 | |||
322 | if (isset($display[0])) { |
||
323 | $display = array('NONE' => $display); |
||
324 | } |
||
325 | |||
326 | foreach ($display as $fieldset => $rows_or_fields) { |
||
327 | $display[$fieldset] = $this->getRowsFromFieldset($rows_or_fields); |
||
328 | } |
||
329 | |||
330 | return $display; |
||
331 | } |
||
332 | |||
333 | protected function getRowsFromFieldset(array $rows_or_fields) |
||
347 | |||
348 | /** |
||
349 | * Get columns for tab, fieldset, row or field. |
||
350 | * |
||
351 | * @param mixed $input |
||
352 | * |
||
353 | * @return array Array of columns. |
||
354 | */ |
||
355 | public function getColumnsFor($input) |
||
367 | |||
368 | /** |
||
369 | * Return a list of action from list.actions. |
||
370 | * |
||
371 | * @return array |
||
372 | */ |
||
373 | public function getActions() |
||
382 | |||
383 | protected function setUserActionConfiguration(Action $action) |
||
410 | |||
411 | protected function addAction(Action $action) |
||
415 | |||
416 | View Code Duplication | protected function findActions() |
|
439 | |||
440 | /** |
||
441 | * Return a list of action from list.object_actions. |
||
442 | * |
||
443 | * @return array |
||
444 | */ |
||
445 | public function getObjectActions() |
||
454 | |||
455 | View Code Duplication | protected function setUserObjectActionConfiguration(Action $action) |
|
477 | |||
478 | protected function addObjectAction(Action $action) |
||
482 | |||
483 | View Code Duplication | protected function findObjectActions() |
|
503 | |||
504 | View Code Duplication | public function findGenericAction($actionName) |
|
511 | |||
512 | View Code Duplication | public function findObjectAction($actionName) |
|
519 | |||
520 | View Code Duplication | public function findBatchAction($actionName) |
|
527 | |||
528 | public function getBaseGeneratorName() |
||
532 | |||
533 | public function getNamespacePrefixWithSubfolder() |
||
538 | |||
539 | public function getRoutePrefixWithSubfolder() |
||
543 | |||
544 | public function getNamespacePrefixForTemplate() |
||
548 | |||
549 | public function getBaseActionsRoute() |
||
563 | |||
564 | public function getObjectActionsRoute() |
||
568 | |||
569 | /** |
||
570 | * Get the PK column name. |
||
571 | * |
||
572 | * @return string parameter |
||
573 | */ |
||
574 | public function getModelPrimaryKeyName() |
||
578 | |||
579 | /** |
||
580 | * Allow to add complementary strylesheets. |
||
581 | * |
||
582 | * |
||
583 | * param: |
||
584 | * stylesheets: |
||
585 | * - path/css.css |
||
586 | * - { path: path/css.css, media: all } |
||
587 | * |
||
588 | * @return array |
||
589 | */ |
||
590 | public function getStylesheets() |
||
619 | |||
620 | /** |
||
621 | * Allow to add complementary javascripts. |
||
622 | * |
||
623 | * |
||
624 | * param: |
||
625 | * javascripts: |
||
626 | * - path/js.js |
||
627 | * - { path: path/js.js } |
||
628 | * - { route: my_route, routeparams: {} } |
||
629 | * |
||
630 | * @return array |
||
631 | */ |
||
632 | public function getJavascripts() |
||
667 | } |
||
668 |
There are different options of fixing this problem.
If you want to be on the safe side, you can add an additional type-check:
If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:
Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.