elkarte /
Elkarte
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 Menu 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 Menu, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 27 | class Menu |
||
| 28 | { |
||
| 29 | /** |
||
| 30 | * Instance of HttpReq |
||
| 31 | * @var HttpReq |
||
| 32 | */ |
||
| 33 | protected $req; |
||
| 34 | |||
| 35 | /** |
||
| 36 | * Will hold the created $context |
||
| 37 | * @var array |
||
| 38 | */ |
||
| 39 | protected $menu_context = []; |
||
| 40 | |||
| 41 | /** |
||
| 42 | * Used for profile menu for own / any |
||
| 43 | * @var string |
||
| 44 | */ |
||
| 45 | protected $permission_set; |
||
| 46 | |||
| 47 | /** |
||
| 48 | * If we found the menu item selected |
||
| 49 | * @var bool |
||
| 50 | */ |
||
| 51 | protected $found_section = false; |
||
| 52 | |||
| 53 | /** |
||
| 54 | * If we can't find the selection, we pick for them |
||
| 55 | * @var string |
||
| 56 | */ |
||
| 57 | protected $current_area = ''; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * The current subaction of the system |
||
| 61 | * @var string |
||
| 62 | */ |
||
| 63 | protected $current_subaction = ''; |
||
| 64 | |||
| 65 | /** |
||
| 66 | * Will hold the selected menu data that is returned to the caller |
||
| 67 | * @var array |
||
| 68 | */ |
||
| 69 | private $include_data = []; |
||
| 70 | |||
| 71 | /** |
||
| 72 | * Unique menu number |
||
| 73 | * @var int |
||
| 74 | */ |
||
| 75 | private $max_menu_id; |
||
| 76 | |||
| 77 | /** |
||
| 78 | * Holds menu options set by AddOptions |
||
| 79 | * @var array |
||
| 80 | */ |
||
| 81 | public $menuOptions = []; |
||
| 82 | |||
| 83 | /** |
||
| 84 | * Holds menu definition structure set by AddAreas |
||
| 85 | * @var array |
||
| 86 | */ |
||
| 87 | public $menuData = []; |
||
| 88 | |||
| 89 | /** |
||
| 90 | * Initial processing for the menu |
||
| 91 | */ |
||
| 92 | public function __construct(HttpReq $req = null) |
||
| 111 | |||
| 112 | /** |
||
| 113 | * Create a menu |
||
| 114 | * |
||
| 115 | * @return array |
||
| 116 | * @throws \Elk_Exception |
||
| 117 | */ |
||
| 118 | public function prepareMenu(): array |
||
| 147 | |||
| 148 | /** |
||
| 149 | * Prepares tabs for the template. |
||
| 150 | * |
||
| 151 | * This should be called after the area is dispatched, because areas |
||
| 152 | * are usually in their own file. Those files, once dispatched to, hold |
||
| 153 | * some data for the tabs which must be specially combined with subaction |
||
| 154 | * data for everything to work properly. |
||
| 155 | * |
||
| 156 | * Seems complicated, yes. |
||
| 157 | */ |
||
| 158 | public function prepareTabData(): void |
||
| 177 | |||
| 178 | /** |
||
| 179 | * Performs a sanity check that a menu was created successfully |
||
| 180 | * |
||
| 181 | * - If it fails to find valid data, will reset max_menu_id and any menu context created |
||
| 182 | * |
||
| 183 | * @return bool |
||
| 184 | */ |
||
| 185 | private function validateData(): bool |
||
| 195 | |||
| 196 | /** |
||
| 197 | * Process the array of MenuOptions passed to the class |
||
| 198 | */ |
||
| 199 | protected function processMenuOptions(): void |
||
| 212 | |||
| 213 | /** |
||
| 214 | * Build the menuOption additional parameters for use in the url |
||
| 215 | */ |
||
| 216 | private function buildAdditionalParams(): void |
||
| 236 | |||
| 237 | /** |
||
| 238 | * Process the menuData array passed to the class |
||
| 239 | * |
||
| 240 | * - Only processes areas that are enabled and that the user has permissions |
||
| 241 | */ |
||
| 242 | protected function processMenuData(): void |
||
| 257 | |||
| 258 | /** |
||
| 259 | * Determines if the user has the permissions to access the section/area |
||
| 260 | * |
||
| 261 | * If said item did not provide any permission to check, full |
||
| 262 | * unfettered access is assumed. |
||
| 263 | * |
||
| 264 | * The profile areas are a bit different in that each permission is |
||
| 265 | * divided into two sets: "own" for owner and "any" for everyone else. |
||
| 266 | * |
||
| 267 | * @param MenuItem $obj area or section being checked |
||
| 268 | * |
||
| 269 | * @return bool |
||
| 270 | */ |
||
| 271 | private function checkPermissions(MenuItem $obj): bool |
||
| 286 | |||
| 287 | /** |
||
| 288 | * Checks if the area has a label or not |
||
| 289 | * |
||
| 290 | * @return bool |
||
| 291 | */ |
||
| 292 | private function areaHasLabel(string $area_id, MenuArea $area): bool |
||
| 298 | |||
| 299 | /** |
||
| 300 | * Main processing for creating the menu items for all sections |
||
| 301 | * |
||
| 302 | * @param string $section_id |
||
| 303 | * @param MenuSection $section |
||
| 304 | */ |
||
| 305 | protected function processSectionAreas(string $section_id, MenuSection $section): void |
||
| 337 | |||
| 338 | /** |
||
| 339 | * Checks the menu item to see if it is the currently selected one |
||
| 340 | * |
||
| 341 | * @param string $section_id |
||
| 342 | * @param string $area_id |
||
| 343 | * @param MenuArea $area |
||
| 344 | */ |
||
| 345 | private function checkCurrentSection(string $section_id, string $area_id, MenuArea $area): void |
||
| 356 | |||
| 357 | /** |
||
| 358 | * @param string $section_id |
||
| 359 | * @param string $area_id |
||
| 360 | * @param MenuArea $area |
||
| 361 | */ |
||
| 362 | private function setFirstAreaCurrent(string $section_id, string $area_id, MenuArea $area): void |
||
| 370 | |||
| 371 | /** |
||
| 372 | * Simply sets the current area |
||
| 373 | * |
||
| 374 | * @param string $section_id |
||
| 375 | * @param string $area_id |
||
| 376 | * @param MenuArea $area |
||
| 377 | */ |
||
| 378 | private function setAreaCurrent(string $section_id, string $area_id, MenuArea $area): void |
||
| 385 | |||
| 386 | /** |
||
| 387 | * @param MenuItem $obj |
||
| 388 | * @param integer $idx |
||
| 389 | * |
||
| 390 | * @return string |
||
| 391 | */ |
||
| 392 | private function parseCounter(MenuItem $obj, int $idx): string |
||
| 405 | |||
| 406 | /** |
||
| 407 | * Sets the various section ID items |
||
| 408 | * |
||
| 409 | * What it does: |
||
| 410 | * - If the ID is not set, sets it and sets the section title |
||
| 411 | * - Sets the section title |
||
| 412 | * |
||
| 413 | * @param string $section_id |
||
| 414 | * @param MenuSection $section |
||
| 415 | */ |
||
| 416 | private function setSectionContext(string $section_id, MenuSection $section): void |
||
| 426 | |||
| 427 | /** |
||
| 428 | * Sets the various area items |
||
| 429 | * |
||
| 430 | * What it does: |
||
| 431 | * - If the ID is not set, sets it and sets the area title |
||
| 432 | * - Sets the area title |
||
| 433 | * |
||
| 434 | * @param string $section_id |
||
| 435 | * @param string $area_id |
||
| 436 | * @param MenuArea $area |
||
| 437 | */ |
||
| 438 | private function setAreaContext(string $section_id, string $area_id, MenuArea $area): void |
||
| 446 | |||
| 447 | /** |
||
| 448 | * Set the URL for the menu item |
||
| 449 | * |
||
| 450 | * @param string $section_id |
||
| 451 | * @param string $area_id |
||
| 452 | * @param MenuArea $area |
||
| 453 | */ |
||
| 454 | private function setAreaUrl(string $section_id, string $area_id, MenuArea $area): void |
||
| 462 | |||
| 463 | /** |
||
| 464 | * Set the menu icon |
||
| 465 | * |
||
| 466 | * @param string $section_id |
||
| 467 | * @param string $area_id |
||
| 468 | * @param MenuArea $area |
||
| 469 | */ |
||
| 470 | private function setAreaIcon(string $section_id, string $area_id, MenuArea $area): void |
||
| 492 | |||
| 493 | /** |
||
| 494 | * Processes all of the subsections for a menu item |
||
| 495 | * |
||
| 496 | * @param string $section_id |
||
| 497 | * @param string $area_id |
||
| 498 | * @param MenuArea $area |
||
| 499 | */ |
||
| 500 | protected function processAreaSubsections(string $section_id, string $area_id, MenuArea $area): void |
||
| 543 | |||
| 544 | /** |
||
| 545 | * @param string $section_id |
||
| 546 | * @param string $area_id |
||
| 547 | * @param string $sub_id |
||
| 548 | * @param MenuSubsection $sub |
||
| 549 | */ |
||
| 550 | private function setSubsSectionUrl(string $section_id, string $area_id, string $sub_id, MenuSubsection $sub): void |
||
| 558 | |||
| 559 | /** |
||
| 560 | * Set the current subsection |
||
| 561 | * |
||
| 562 | * @param string $sub_id |
||
| 563 | * @param MenuSubsection $sub |
||
| 564 | */ |
||
| 565 | private function setCurrentSubSection(string $sub_id, MenuSubsection $sub): void |
||
| 578 | |||
| 579 | /** |
||
| 580 | * Checks and updates base and section urls |
||
| 581 | */ |
||
| 582 | private function setBaseUrl(): void |
||
| 590 | |||
| 591 | /** |
||
| 592 | * Checks and updates base and section urls |
||
| 593 | */ |
||
| 594 | private function setActiveButtons(): void |
||
| 610 | |||
| 611 | /** |
||
| 612 | * Add the base menu options for this menu |
||
| 613 | * |
||
| 614 | * @param array $menuOptions an array of options that can be used to override some default behaviours. |
||
| 615 | * It can accept the following indexes: |
||
| 616 | * - action => overrides the default action |
||
| 617 | * - current_area => overrides the current area |
||
| 618 | * - extra_url_parameters => an array or pairs or parameters to be added to the url |
||
| 619 | * - disable_url_session_check => (boolean) if true the session var/id are omitted from |
||
| 620 | * the url |
||
| 621 | * - base_url => an alternative base url |
||
| 622 | * - menu_type => alternative menu types? |
||
| 623 | * - can_toggle_drop_down => (boolean) if the menu can "toggle" |
||
| 624 | * - template_name => an alternative template to load (instead of Generic) |
||
| 625 | * - layer_name => alternative layer name for the menu |
||
| 626 | * - hook => hook name to call integrate_ . 'hook name' . '_areas' |
||
| 627 | * - default_include_dir => directory to include for function support |
||
| 628 | */ |
||
| 629 | public function addOptions(array $menuOptions): void |
||
| 633 | |||
| 634 | /** |
||
| 635 | * @param string $id |
||
| 636 | * @param MenuSection $section |
||
| 637 | * |
||
| 638 | * @return $this |
||
| 639 | */ |
||
| 640 | public function addSection(string $id, MenuSection $section): Menu |
||
| 646 | |||
| 647 | private function buildTemplateVars(): void |
||
| 662 | |||
| 663 | /** |
||
| 664 | * Finalizes items so the computed menu can be used |
||
| 665 | * |
||
| 666 | * What it does: |
||
| 667 | * - Sets the menu layer in the template stack |
||
| 668 | * - Loads context with the computed menu context |
||
| 669 | * - Sets current subaction and current max menu id |
||
| 670 | */ |
||
| 671 | public function setContext(): void |
||
| 688 | |||
| 689 | /** |
||
| 690 | * Delete a menu. |
||
| 691 | * |
||
| 692 | * Checks to see if this menu been loaded into context |
||
| 693 | * and, if so, resets $context['max_menu_id'] back to the |
||
| 694 | * last known menu (if any) and remove the template layer |
||
| 695 | * if there aren't any other known menus. |
||
| 696 | */ |
||
| 697 | public function destroy(): void |
||
| 719 | } |
||
| 720 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountIdthat can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theidproperty of an instance of theAccountclass. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.