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 Admin 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 Admin, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
25 | class Admin |
||
26 | { |
||
27 | /** |
||
28 | * Set module directory |
||
29 | * |
||
30 | * @var string |
||
31 | */ |
||
32 | public $tplModule = 'system'; |
||
33 | |||
34 | /** |
||
35 | * Template call for each render parts |
||
36 | * |
||
37 | * @var array |
||
38 | */ |
||
39 | public $tplFile = array( |
||
40 | 'index' => 'admin_index.tpl', |
||
41 | 'about' => 'admin_about.tpl', |
||
42 | 'infobox' => 'admin_infobox.tpl', |
||
43 | 'bread' => 'admin_breadcrumb.tpl', |
||
44 | 'button' => 'admin_buttons.tpl', |
||
45 | 'tips' => 'admin_tips.tpl', |
||
46 | 'nav' => 'admin_navigation.tpl', |
||
47 | ); |
||
48 | |||
49 | /** |
||
50 | * Tips to display in admin page |
||
51 | * |
||
52 | * @var string |
||
53 | */ |
||
54 | private $tips = ''; |
||
55 | |||
56 | /** |
||
57 | * List of button |
||
58 | * |
||
59 | * @var array |
||
60 | */ |
||
61 | private $itemButton = array(); |
||
62 | |||
63 | /** |
||
64 | * List of Info Box |
||
65 | * |
||
66 | * @var array |
||
67 | */ |
||
68 | private $itemInfoBox = array(); |
||
69 | |||
70 | /** |
||
71 | * List of line of an Info Box |
||
72 | * |
||
73 | * @var array |
||
74 | */ |
||
75 | private $itemConfigBoxLine = array(); |
||
76 | |||
77 | /** |
||
78 | * Breadcrumb data |
||
79 | * |
||
80 | * @var array |
||
81 | */ |
||
82 | private $bread = array(); |
||
83 | |||
84 | /** |
||
85 | * Current module object |
||
86 | * |
||
87 | * @var \Xoops\Core\Kernel\Handlers\XoopsModule $module |
||
88 | */ |
||
89 | private $module = null; |
||
90 | |||
91 | /** |
||
92 | * Constructor |
||
93 | */ |
||
94 | public function __construct() |
||
100 | |||
101 | /** |
||
102 | * Add breadcrumb menu |
||
103 | * |
||
104 | * @param string $title title |
||
105 | * @param string $link url |
||
106 | * @param bool $home is home |
||
107 | * |
||
108 | * @return void |
||
109 | */ |
||
110 | public function addBreadcrumbLink($title = '', $link = '', $home = false) |
||
118 | |||
119 | /** |
||
120 | * Add config line |
||
121 | * |
||
122 | * @param string $value line value - a string or array of values |
||
123 | * @param string $type type of line default, folder, chmod, extension, module |
||
124 | * Or, type value for itemConfigBoxLine -- accept, warning, error |
||
125 | * |
||
126 | * @return bool |
||
127 | */ |
||
128 | public function addConfigBoxLine($value = '', $type = 'default') |
||
237 | |||
238 | // simplified config aliases |
||
239 | |||
240 | /** |
||
241 | * Add error to config box |
||
242 | * |
||
243 | * @param string $value the error message |
||
244 | * |
||
245 | * @return bool |
||
246 | */ |
||
247 | public function addConfigError($value = '') |
||
251 | |||
252 | /** |
||
253 | * Add accept (OK) message to config box |
||
254 | * |
||
255 | * @param string $value the OK message |
||
256 | * |
||
257 | * @return bool |
||
258 | */ |
||
259 | public function addConfigAccept($value = '') |
||
263 | |||
264 | /** |
||
265 | * Add warning to config box |
||
266 | * |
||
267 | * @param string $value the warning message |
||
268 | * |
||
269 | * @return bool |
||
270 | */ |
||
271 | public function addConfigWarning($value = '') |
||
275 | |||
276 | /** |
||
277 | * Check for installed module and version and to configuration box |
||
278 | * |
||
279 | * @param string $moddir module directory name |
||
280 | * @param integer $minversion minimum acceptable module version (100 = V1.00) |
||
281 | * |
||
282 | * @return bool true if requested version of the module is available |
||
283 | */ |
||
284 | View Code Duplication | public function addConfigModuleVersion($moddir, $minversion) |
|
315 | |||
316 | /** |
||
317 | * Add Info box |
||
318 | * Template in htdocs/modules/system/templates/admin/admin_infobox.tpl |
||
319 | * |
||
320 | * @param string $title title |
||
321 | * @param string $type type will be used as icon name in title in template |
||
322 | * @param string $extra extra added to the div element that surrounds the box |
||
323 | * |
||
324 | * @return bool |
||
325 | */ |
||
326 | public function addInfoBox($title, $type = 'default', $extra = '') |
||
334 | |||
335 | /** |
||
336 | * Add line to the info box |
||
337 | * |
||
338 | * @param string $text title |
||
339 | * @param string $type type |
||
340 | * @param string $color color |
||
341 | * |
||
342 | * @return bool |
||
343 | */ |
||
344 | public function addInfoBoxLine($text = '', $type = 'default', $color = 'inherit') |
||
358 | |||
359 | /** |
||
360 | * Add Item button |
||
361 | * |
||
362 | * @param string $title title |
||
363 | * @param string $link link |
||
364 | * @param string $icon icon |
||
365 | * @param string $extra extra |
||
366 | * |
||
367 | * @return bool |
||
368 | */ |
||
369 | View Code Duplication | public function addItemButton($title, $link, $icon = 'add', $extra = '') |
|
378 | |||
379 | /** |
||
380 | * Add a tips |
||
381 | * |
||
382 | * @param string $text text |
||
383 | * |
||
384 | * @return void |
||
385 | */ |
||
386 | public function addTips($text = '') |
||
390 | |||
391 | /** |
||
392 | * Construct template path |
||
393 | * |
||
394 | * @param string $type type |
||
395 | * |
||
396 | * @return string |
||
397 | */ |
||
398 | private function getTplPath($type = '') |
||
402 | |||
403 | /** |
||
404 | * renderBreadcrumb |
||
405 | * |
||
406 | * @return string |
||
407 | */ |
||
408 | public function renderBreadcrumb() |
||
414 | |||
415 | /** |
||
416 | * displayBreadcrumb |
||
417 | * |
||
418 | * @return void |
||
419 | */ |
||
420 | public function displayBreadcrumb() |
||
424 | |||
425 | /** |
||
426 | * Render all items buttons |
||
427 | * |
||
428 | * @param string $position position |
||
429 | * @param string $delimiter delimiter |
||
430 | * |
||
431 | * @return string |
||
432 | */ |
||
433 | public function renderButton($position = "floatright", $delimiter = " ") |
||
442 | |||
443 | /** |
||
444 | * displayButton |
||
445 | * |
||
446 | * @param string $position position |
||
447 | * @param string $delimiter delimiter |
||
448 | * |
||
449 | * @return void |
||
450 | */ |
||
451 | public function displayButton($position = "floatright", $delimiter = " ") |
||
455 | |||
456 | /** |
||
457 | * Render InfoBox |
||
458 | * |
||
459 | * @return string |
||
460 | */ |
||
461 | public function renderInfoBox() |
||
467 | |||
468 | /** |
||
469 | * displayInfoBox |
||
470 | * |
||
471 | * @return void |
||
472 | */ |
||
473 | public function displayInfoBox() |
||
477 | |||
478 | /** |
||
479 | * Render index page for admin |
||
480 | * |
||
481 | * @return string |
||
482 | */ |
||
483 | public function renderIndex() |
||
614 | |||
615 | /** |
||
616 | * displayIndex |
||
617 | * |
||
618 | * @return void |
||
619 | */ |
||
620 | public function displayIndex() |
||
624 | |||
625 | /** |
||
626 | * Render navigation to admin page |
||
627 | * |
||
628 | * @param string $menu current menu |
||
629 | * |
||
630 | * @return array |
||
631 | */ |
||
632 | public function renderNavigation($menu = '') |
||
653 | |||
654 | /** |
||
655 | * displayNavigation |
||
656 | * |
||
657 | * @param string $menu current menu |
||
658 | * |
||
659 | * @return void |
||
660 | */ |
||
661 | public function displayNavigation($menu = '') |
||
668 | |||
669 | /** |
||
670 | * Render tips to admin page |
||
671 | * |
||
672 | * @return string |
||
673 | */ |
||
674 | public function renderTips() |
||
680 | |||
681 | /** |
||
682 | * displayTips |
||
683 | * |
||
684 | * @return void |
||
685 | */ |
||
686 | public function displayTips() |
||
690 | |||
691 | /** |
||
692 | * Render about page |
||
693 | * |
||
694 | * @param bool $logo_xoops show logo |
||
695 | * |
||
696 | * @return bool|mixed|string |
||
697 | */ |
||
698 | public function renderAbout($logo_xoops = true) |
||
769 | |||
770 | /** |
||
771 | * displayAbout |
||
772 | * |
||
773 | * @param bool $logo_xoops display logo |
||
774 | * |
||
775 | * @return void |
||
776 | */ |
||
777 | public function displayAbout($logo_xoops = true) |
||
781 | } |
||
782 |
When comparing two booleans, it is generally considered safer to use the strict comparison operator.