Complex classes like Hierarchy 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 Hierarchy, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 13 | class Hierarchy extends DataExtension { |
||
| 14 | |||
| 15 | protected $markedNodes; |
||
| 16 | |||
| 17 | protected $markingFilter; |
||
| 18 | |||
| 19 | /** @var int */ |
||
| 20 | protected $_cache_numChildren; |
||
| 21 | |||
| 22 | /** |
||
| 23 | * The lower bounds for the amount of nodes to mark. If set, the logic will expand nodes until it reaches at least |
||
| 24 | * this number, and then stops. Root nodes will always show regardless of this settting. Further nodes can be |
||
| 25 | * lazy-loaded via ajax. This isn't a hard limit. Example: On a value of 10, with 20 root nodes, each having 30 |
||
| 26 | * children, the actual node count will be 50 (all root nodes plus first expanded child). |
||
| 27 | * |
||
| 28 | * @config |
||
| 29 | * @var int |
||
| 30 | */ |
||
| 31 | private static $node_threshold_total = 50; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * Limit on the maximum children a specific node can display. Serves as a hard limit to avoid exceeding available |
||
| 35 | * server resources in generating the tree, and browser resources in rendering it. Nodes with children exceeding |
||
| 36 | * this value typically won't display any children, although this is configurable through the $nodeCountCallback |
||
| 37 | * parameter in {@link getChildrenAsUL()}. "Root" nodes will always show all children, regardless of this setting. |
||
| 38 | * |
||
| 39 | * @config |
||
| 40 | * @var int |
||
| 41 | */ |
||
| 42 | private static $node_threshold_leaf = 250; |
||
| 43 | |||
| 44 | public static function get_extra_config($class, $extension, $args) { |
||
| 49 | |||
| 50 | /** |
||
| 51 | * Validate the owner object - check for existence of infinite loops. |
||
| 52 | * |
||
| 53 | * @param ValidationResult $validationResult |
||
| 54 | */ |
||
| 55 | public function validate(ValidationResult $validationResult) { |
||
| 84 | |||
| 85 | /** |
||
| 86 | * Returns the children of this DataObject as an XHTML UL. This will be called recursively on each child, so if they |
||
| 87 | * have children they will be displayed as a UL inside a LI. |
||
| 88 | * |
||
| 89 | * @param string $attributes Attributes to add to the UL |
||
| 90 | * @param string|callable $titleEval PHP code to evaluate to start each child - this should include '<li>' |
||
| 91 | * @param string $extraArg Extra arguments that will be passed on to children, for if they |
||
| 92 | * overload this function |
||
| 93 | * @param bool $limitToMarked Display only marked children |
||
| 94 | * @param string $childrenMethod The name of the method used to get children from each object |
||
| 95 | * @param bool $rootCall Set to true for this first call, and then to false for calls inside |
||
| 96 | * the recursion. You should not change this. |
||
| 97 | * @param int $nodeCountThreshold See {@link self::$node_threshold_total} |
||
| 98 | * @param callable $nodeCountCallback Called with the node count, which gives the callback an opportunity to |
||
| 99 | * intercept the query. Useful e.g. to avoid excessive children listings |
||
| 100 | * (Arguments: $parent, $numChildren) |
||
| 101 | * |
||
| 102 | * @return string |
||
| 103 | */ |
||
| 104 | public function getChildrenAsUL($attributes = "", $titleEval = '"<li>" . $child->Title . "</li>"', $extraArg = null, |
||
| 186 | |||
| 187 | /** |
||
| 188 | * Mark a segment of the tree, by calling mark(). |
||
| 189 | * |
||
| 190 | * The method performs a breadth-first traversal until the number of nodes is more than minCount. This is used to |
||
| 191 | * get a limited number of tree nodes to show in the CMS initially. |
||
| 192 | * |
||
| 193 | * This method returns the number of nodes marked. After this method is called other methods can check |
||
| 194 | * {@link isExpanded()} and {@link isMarked()} on individual nodes. |
||
| 195 | * |
||
| 196 | * @param int $nodeCountThreshold See {@link getChildrenAsUL()} |
||
| 197 | * @return int The actual number of nodes marked. |
||
| 198 | */ |
||
| 199 | public function markPartialTree($nodeCountThreshold = 30, $context = null, |
||
| 218 | |||
| 219 | /** |
||
| 220 | * Filter the marking to only those object with $node->$parameterName == $parameterValue |
||
| 221 | * |
||
| 222 | * @param string $parameterName The parameter on each node to check when marking. |
||
| 223 | * @param mixed $parameterValue The value the parameter must be to be marked. |
||
| 224 | */ |
||
| 225 | public function setMarkingFilter($parameterName, $parameterValue) { |
||
| 231 | |||
| 232 | /** |
||
| 233 | * Filter the marking to only those where the function returns true. The node in question will be passed to the |
||
| 234 | * function. |
||
| 235 | * |
||
| 236 | * @param string $funcName The name of the function to call |
||
| 237 | */ |
||
| 238 | public function setMarkingFilterFunction($funcName) { |
||
| 243 | |||
| 244 | /** |
||
| 245 | * Returns true if the marking filter matches on the given node. |
||
| 246 | * |
||
| 247 | * @param DataObject $node Node to check |
||
| 248 | * @return bool |
||
| 249 | */ |
||
| 250 | public function markingFilterMatches($node) { |
||
| 272 | |||
| 273 | /** |
||
| 274 | * Mark all children of the given node that match the marking filter. |
||
| 275 | * |
||
| 276 | * @param DataObject $node Parent node |
||
| 277 | * @param mixed $context |
||
| 278 | * @param string $childrenMethod The name of the instance method to call to get the object's list of children |
||
| 279 | * @param string $numChildrenMethod The name of the instance method to call to count the object's children |
||
| 280 | * @return DataList |
||
| 281 | */ |
||
| 282 | public function markChildren($node, $context = null, $childrenMethod = "AllChildrenIncludingDeleted", |
||
| 309 | |||
| 310 | /** |
||
| 311 | * Ensure marked nodes that have children are also marked expanded. Call this after marking but before iterating |
||
| 312 | * over the tree. |
||
| 313 | * |
||
| 314 | * @param string $numChildrenMethod The name of the instance method to call to count the object's children |
||
| 315 | */ |
||
| 316 | protected function markingFinished($numChildrenMethod = "numChildren") { |
||
| 326 | |||
| 327 | /** |
||
| 328 | * Return CSS classes of 'unexpanded', 'closed', both, or neither, as well as a 'jstree-*' state depending on the |
||
| 329 | * marking of this DataObject. |
||
| 330 | * |
||
| 331 | * @param string $numChildrenMethod The name of the instance method to call to count the object's children |
||
| 332 | * @return string |
||
| 333 | */ |
||
| 334 | public function markingClasses($numChildrenMethod="numChildren") { |
||
| 350 | |||
| 351 | /** |
||
| 352 | * Mark the children of the DataObject with the given ID. |
||
| 353 | * |
||
| 354 | * @param int $id ID of parent node |
||
| 355 | * @param bool $open If this is true, mark the parent node as opened |
||
| 356 | * @return bool |
||
| 357 | */ |
||
| 358 | public function markById($id, $open = false) { |
||
| 369 | |||
| 370 | /** |
||
| 371 | * Expose the given object in the tree, by marking this page and all it ancestors. |
||
| 372 | * |
||
| 373 | * @param DataObject $childObj |
||
| 374 | */ |
||
| 375 | public function markToExpose($childObj) { |
||
| 383 | |||
| 384 | /** |
||
| 385 | * Return the IDs of all the marked nodes. |
||
| 386 | * |
||
| 387 | * @return array |
||
| 388 | */ |
||
| 389 | public function markedNodeIDs() { |
||
| 392 | |||
| 393 | /** |
||
| 394 | * Return an array of this page and its ancestors, ordered item -> root. |
||
| 395 | * |
||
| 396 | * @return SiteTree[] |
||
| 397 | */ |
||
| 398 | public function parentStack() { |
||
| 408 | |||
| 409 | /** |
||
| 410 | * Cache of DataObjects' marked statuses: [ClassName][ID] = bool |
||
| 411 | * @var array |
||
| 412 | */ |
||
| 413 | protected static $marked = array(); |
||
| 414 | |||
| 415 | /** |
||
| 416 | * Cache of DataObjects' expanded statuses: [ClassName][ID] = bool |
||
| 417 | * @var array |
||
| 418 | */ |
||
| 419 | protected static $expanded = array(); |
||
| 420 | |||
| 421 | /** |
||
| 422 | * Cache of DataObjects' opened statuses: [ClassName][ID] = bool |
||
| 423 | * @var array |
||
| 424 | */ |
||
| 425 | protected static $treeOpened = array(); |
||
| 426 | |||
| 427 | /** |
||
| 428 | * Mark this DataObject as expanded. |
||
| 429 | */ |
||
| 430 | public function markExpanded() { |
||
| 434 | |||
| 435 | /** |
||
| 436 | * Mark this DataObject as unexpanded. |
||
| 437 | */ |
||
| 438 | public function markUnexpanded() { |
||
| 442 | |||
| 443 | /** |
||
| 444 | * Mark this DataObject's tree as opened. |
||
| 445 | */ |
||
| 446 | public function markOpened() { |
||
| 450 | |||
| 451 | /** |
||
| 452 | * Mark this DataObject's tree as closed. |
||
| 453 | */ |
||
| 454 | public function markClosed() { |
||
| 459 | |||
| 460 | /** |
||
| 461 | * Check if this DataObject is marked. |
||
| 462 | * |
||
| 463 | * @return bool |
||
| 464 | */ |
||
| 465 | public function isMarked() { |
||
| 470 | |||
| 471 | /** |
||
| 472 | * Check if this DataObject is expanded. |
||
| 473 | * |
||
| 474 | * @return bool |
||
| 475 | */ |
||
| 476 | public function isExpanded() { |
||
| 481 | |||
| 482 | /** |
||
| 483 | * Check if this DataObject's tree is opened. |
||
| 484 | * |
||
| 485 | * @return bool |
||
| 486 | */ |
||
| 487 | public function isTreeOpened() { |
||
| 492 | |||
| 493 | /** |
||
| 494 | * Get a list of this DataObject's and all it's descendants IDs. |
||
| 495 | * |
||
| 496 | * @return int[] |
||
| 497 | */ |
||
| 498 | public function getDescendantIDList() { |
||
| 503 | |||
| 504 | /** |
||
| 505 | * Get a list of this DataObject's and all it's descendants ID, and put them in $idList. |
||
| 506 | * |
||
| 507 | * @param array $idList Array to put results in. |
||
| 508 | */ |
||
| 509 | public function loadDescendantIDListInto(&$idList) { |
||
| 523 | |||
| 524 | /** |
||
| 525 | * Get the children for this DataObject. |
||
| 526 | * |
||
| 527 | * @return DataList |
||
| 528 | */ |
||
| 529 | public function Children() { |
||
| 542 | |||
| 543 | /** |
||
| 544 | * Return all children, including those 'not in menus'. |
||
| 545 | * |
||
| 546 | * @return DataList |
||
| 547 | */ |
||
| 548 | public function AllChildren() { |
||
| 551 | |||
| 552 | /** |
||
| 553 | * Return all children, including those that have been deleted but are still in live. |
||
| 554 | * - Deleted children will be marked as "DeletedFromStage" |
||
| 555 | * - Added children will be marked as "AddedToStage" |
||
| 556 | * - Modified children will be marked as "ModifiedOnStage" |
||
| 557 | * - Everything else has "SameOnStage" set, as an indicator that this information has been looked up. |
||
| 558 | * |
||
| 559 | * @param mixed $context |
||
| 560 | * @return ArrayList |
||
| 561 | */ |
||
| 562 | public function AllChildrenIncludingDeleted($context = null) { |
||
| 565 | |||
| 566 | /** |
||
| 567 | * @see AllChildrenIncludingDeleted |
||
| 568 | * |
||
| 569 | * @param mixed $context |
||
| 570 | * @return ArrayList |
||
| 571 | */ |
||
| 572 | public function doAllChildrenIncludingDeleted($context = null) { |
||
| 600 | |||
| 601 | /** |
||
| 602 | * Return all the children that this page had, including pages that were deleted from both stage & live. |
||
| 603 | * |
||
| 604 | * @return DataList |
||
| 605 | * @throws Exception |
||
| 606 | */ |
||
| 607 | public function AllHistoricalChildren() { |
||
| 616 | |||
| 617 | /** |
||
| 618 | * Return the number of children that this page ever had, including pages that were deleted. |
||
| 619 | * |
||
| 620 | * @return int |
||
| 621 | * @throws Exception |
||
| 622 | */ |
||
| 623 | public function numHistoricalChildren() { |
||
| 631 | |||
| 632 | /** |
||
| 633 | * Return the number of direct children. By default, values are cached after the first invocation. Can be |
||
| 634 | * augumented by {@link augmentNumChildrenCountQuery()}. |
||
| 635 | * |
||
| 636 | * @param bool $cache Whether to retrieve values from cache |
||
| 637 | * @return int |
||
| 638 | */ |
||
| 639 | public function numChildren($cache = true) { |
||
| 650 | |||
| 651 | /** |
||
| 652 | * Return children in the stage site. |
||
| 653 | * |
||
| 654 | * @param bool $showAll Include all of the elements, even those not shown in the menus. Only applicable when |
||
| 655 | * extension is applied to {@link SiteTree}. |
||
| 656 | * @return DataList |
||
| 657 | */ |
||
| 658 | public function stageChildren($showAll = false) { |
||
| 669 | |||
| 670 | /** |
||
| 671 | * Return children in the live site, if it exists. |
||
| 672 | * |
||
| 673 | * @param bool $showAll Include all of the elements, even those not shown in the menus. Only |
||
| 674 | * applicable when extension is applied to {@link SiteTree}. |
||
| 675 | * @param bool $onlyDeletedFromStage Only return items that have been deleted from stage |
||
| 676 | * @return DataList |
||
| 677 | * @throws Exception |
||
| 678 | */ |
||
| 679 | public function liveChildren($showAll = false, $onlyDeletedFromStage = false) { |
||
| 697 | |||
| 698 | /** |
||
| 699 | * Get this object's parent, optionally filtered by an SQL clause. If the clause doesn't match the parent, nothing |
||
| 700 | * is returned. |
||
| 701 | * |
||
| 702 | * @param string $filter |
||
| 703 | * @return DataObject |
||
| 704 | */ |
||
| 705 | public function getParent($filter = null) { |
||
| 715 | |||
| 716 | /** |
||
| 717 | * Return all the parents of this class in a set ordered from the lowest to highest parent. |
||
| 718 | * |
||
| 719 | * @return ArrayList |
||
| 720 | */ |
||
| 721 | public function getAncestors() { |
||
| 731 | |||
| 732 | /** |
||
| 733 | * Returns a human-readable, flattened representation of the path to the object, using its {@link Title} attribute. |
||
| 734 | * |
||
| 735 | * @param string $separator |
||
| 736 | * @return string |
||
| 737 | */ |
||
| 738 | public function getBreadcrumbs($separator = ' » ') { |
||
| 745 | |||
| 746 | /** |
||
| 747 | * Get the next node in the tree of the type. If there is no instance of the className descended from this node, |
||
| 748 | * then search the parents. |
||
| 749 | * |
||
| 750 | * @todo Write! |
||
| 751 | * |
||
| 752 | * @param string $className Class name of the node to find |
||
| 753 | * @param DataObject $afterNode Used for recursive calls to this function |
||
| 754 | * @return DataObject |
||
| 755 | */ |
||
| 756 | public function naturalPrev($className, $afterNode = null ) { |
||
| 759 | |||
| 760 | /** |
||
| 761 | * Get the next node in the tree of the type. If there is no instance of the className descended from this node, |
||
| 762 | * then search the parents. |
||
| 763 | * @param string $className Class name of the node to find. |
||
| 764 | * @param string|int $root ID/ClassName of the node to limit the search to |
||
| 765 | * @param DataObject $afterNode Used for recursive calls to this function |
||
| 766 | * @return DataObject |
||
| 767 | */ |
||
| 768 | public function naturalNext($className = null, $root = 0, $afterNode = null ) { |
||
| 812 | |||
| 813 | /** |
||
| 814 | * Flush all Hierarchy caches: |
||
| 815 | * - Children (instance) |
||
| 816 | * - NumChildren (instance) |
||
| 817 | * - Marked (global) |
||
| 818 | * - Expanded (global) |
||
| 819 | * - TreeOpened (global) |
||
| 820 | */ |
||
| 821 | public function flushCache() { |
||
| 828 | |||
| 829 | /** |
||
| 830 | * Reset global Hierarchy caches: |
||
| 831 | * - Marked |
||
| 832 | * - Expanded |
||
| 833 | * - TreeOpened |
||
| 834 | */ |
||
| 835 | public static function reset() { |
||
| 840 | |||
| 841 | } |
||
| 842 |
If you define a variable conditionally, it can happen that it is not defined for all execution paths.
Let’s take a look at an example:
In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.
Available Fixes
Check for existence of the variable explicitly:
Define a default value for the variable:
Add a value for the missing path: