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 FileBasedContent 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 FileBasedContent, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 12 | class FileBasedContent implements ContainerInjectableInterface |
||
| 13 | { |
||
| 14 | use ContainerInjectableTrait, |
||
| 15 | FBCBreadcrumbTrait, |
||
| 16 | FBCLoadAdditionalContentTrait, |
||
| 17 | FBCUtilitiesTrait; |
||
| 18 | |||
| 19 | |||
| 20 | |||
| 21 | /** |
||
| 22 | * All routes. |
||
| 23 | */ |
||
| 24 | private $index = null; |
||
| 25 | |||
| 26 | /** |
||
| 27 | * All authors. |
||
| 28 | */ |
||
| 29 | private $author = null; |
||
| 30 | |||
| 31 | /** |
||
| 32 | * All categories. |
||
| 33 | */ |
||
| 34 | private $category = null; |
||
| 35 | |||
| 36 | /** |
||
| 37 | * All routes having meta. |
||
| 38 | */ |
||
| 39 | private $meta = null; |
||
| 40 | |||
| 41 | /** |
||
| 42 | * This is the base route. |
||
| 43 | */ |
||
| 44 | private $baseRoute = null; |
||
| 45 | |||
| 46 | /** |
||
| 47 | * This is the extendede meta route, if any. |
||
| 48 | */ |
||
| 49 | private $metaRoute = null; |
||
| 50 | |||
| 51 | /** |
||
| 52 | * This is the current page, to supply pagination, if used. |
||
| 53 | */ |
||
| 54 | private $currentPage = null; |
||
| 55 | |||
| 56 | /** |
||
| 57 | * Use cache or recreate each time. |
||
| 58 | */ |
||
| 59 | private $ignoreCache = false; |
||
| 60 | |||
| 61 | /** |
||
| 62 | * File name pattern, all files must match this pattern and the first |
||
| 63 | * numbered part is optional, the second part becomes the route. |
||
| 64 | */ |
||
| 65 | private $filenamePattern = "#^(\d*)_*([^\.]+)\.md$#"; |
||
| 66 | |||
| 67 | /** |
||
| 68 | * Internal routes that is marked as internal content routes and not |
||
| 69 | * exposed as public routes. |
||
| 70 | */ |
||
| 71 | private $internalRouteDirPattern = [ |
||
| 72 | "#block/#", |
||
| 73 | ]; |
||
| 74 | |||
| 75 | private $internalRouteFilePattern = [ |
||
| 76 | "#^block[_-]{1}#", |
||
| 77 | "#^_#", |
||
| 78 | ]; |
||
| 79 | |||
| 80 | /** |
||
| 81 | * Routes that should be used in toc. |
||
| 82 | */ |
||
| 83 | private $allowedInTocPattern = "([\d]+_(\w)+)"; |
||
| 84 | |||
| 85 | |||
| 86 | |||
| 87 | /** |
||
| 88 | * Set default values from configuration. |
||
| 89 | * |
||
| 90 | * @param array $config the configuration to use. |
||
| 91 | * |
||
| 92 | * @return void |
||
| 93 | */ |
||
| 94 | 1 | public function configure(array $config) : void |
|
| 99 | |||
| 100 | |||
| 101 | |||
| 102 | /** |
||
| 103 | * Set default values from configuration. |
||
| 104 | * |
||
| 105 | * @return this. |
||
| 106 | */ |
||
| 107 | 1 | private function setDefaultsFromConfiguration() |
|
| 115 | |||
| 116 | |||
| 117 | |||
| 118 | /** |
||
| 119 | * Should the cache be used or ignored. |
||
| 120 | * |
||
| 121 | * @param boolean $use true to use the cache or false to ignore the cache |
||
| 122 | * |
||
| 123 | * @return this. |
||
| 124 | */ |
||
| 125 | public function useCache($use) |
||
| 131 | |||
| 132 | |||
| 133 | |||
| 134 | /** |
||
| 135 | * Create the index of all content into an array. |
||
| 136 | * |
||
| 137 | * @param string $type of index to load. |
||
| 138 | * |
||
| 139 | * @return void. |
||
| 140 | */ |
||
| 141 | private function load($type) |
||
| 160 | |||
| 161 | |||
| 162 | |||
| 163 | |||
| 164 | // = Create and manage index ================================== |
||
| 165 | |||
| 166 | /** |
||
| 167 | * Generate an index from the directory structure. |
||
| 168 | * |
||
| 169 | * @return array as index for all content files. |
||
| 170 | */ |
||
| 171 | private function createIndex() |
||
| 211 | |||
| 212 | |||
| 213 | |||
| 214 | /** |
||
| 215 | * Check if a filename is to be marked as an internal route.. |
||
| 216 | * |
||
| 217 | * @param string $filepath as the basepath (routepart) to the file. |
||
| 218 | * |
||
| 219 | * @return boolean true if the route content is internal, else false |
||
| 220 | */ |
||
| 221 | private function isInternalRoute($filepath) |
||
| 238 | |||
| 239 | |||
| 240 | |||
| 241 | /** |
||
| 242 | * Check if filepath should be used as part of toc. |
||
| 243 | * |
||
| 244 | * @param string $filepath as the basepath (routepart) to the file. |
||
| 245 | * |
||
| 246 | * @return boolean true if the route content shoul dbe in toc, else false |
||
| 247 | */ |
||
| 248 | private function allowInToc($filepath) |
||
| 252 | |||
| 253 | |||
| 254 | |||
| 255 | // = Create and manage meta ================================== |
||
| 256 | |||
| 257 | /** |
||
| 258 | * Generate an index for meta files. |
||
| 259 | * |
||
| 260 | * @return array as meta index. |
||
| 261 | */ |
||
| 262 | private function createMeta() |
||
| 291 | |||
| 292 | |||
| 293 | |||
| 294 | /** |
||
| 295 | * Get a reference to meta data for specific route. |
||
| 296 | * |
||
| 297 | * @param string $route current route used to access page. |
||
| 298 | * |
||
| 299 | * @return array as table of content. |
||
| 300 | */ |
||
| 301 | private function getMetaForRoute($route) |
||
| 308 | |||
| 309 | |||
| 310 | |||
| 311 | /** |
||
| 312 | * Create a table of content for routes at particular level. |
||
| 313 | * |
||
| 314 | * @param string $route base route to use. |
||
| 315 | * |
||
| 316 | * @return array as the toc. |
||
| 317 | */ |
||
| 318 | private function createBaseRouteToc($route) |
||
| 344 | |||
| 345 | |||
| 346 | |||
| 347 | // = Deal with authors ==================================== |
||
| 348 | |||
| 349 | /** |
||
| 350 | * Generate a lookup index for authors that maps into the meta entry |
||
| 351 | * for the author. |
||
| 352 | * |
||
| 353 | * @return void. |
||
| 354 | */ |
||
| 355 | private function createAuthor() |
||
| 379 | |||
| 380 | |||
| 381 | |||
| 382 | /** |
||
| 383 | * Load details for the author. |
||
| 384 | * |
||
| 385 | * @param array|string $author with details on the author(s). |
||
| 386 | * |
||
| 387 | * @return array with more details on the authors(s). |
||
| 388 | */ |
||
| 389 | View Code Duplication | private function loadAuthorDetails($author) |
|
| 413 | |||
| 414 | |||
| 415 | |||
| 416 | // = Deal with categories ==================================== |
||
| 417 | |||
| 418 | /** |
||
| 419 | * Generate a lookup index for categories that maps into the meta entry |
||
| 420 | * for the category. |
||
| 421 | * |
||
| 422 | * @return void. |
||
| 423 | */ |
||
| 424 | private function createCategory() |
||
| 442 | |||
| 443 | |||
| 444 | |||
| 445 | /** |
||
| 446 | * Find next and previous links of current content. |
||
| 447 | * |
||
| 448 | * @param array|string $author with details on the category(s). |
||
| 449 | * |
||
| 450 | * @return array with more details on the category(s). |
||
| 451 | */ |
||
| 452 | View Code Duplication | private function loadCategoryDetails($category) |
|
| 476 | |||
| 477 | |||
| 478 | |||
| 479 | |||
| 480 | // == Used by meta and breadcrumb (to get title) =========================== |
||
| 481 | // TODO REFACTOR THIS? |
||
| 482 | // Support getting only frontmatter. |
||
| 483 | // Merge with function that retrieves whole filtered since getting |
||
| 484 | // frontmatter will involve full parsing of document. |
||
| 485 | // Title is retrieved from the HTML code. |
||
| 486 | // Also do cacheing of each retrieved and parsed document |
||
| 487 | // in this cycle, to gather code that loads and parses a individual |
||
| 488 | // document. |
||
| 489 | |||
| 490 | /** |
||
| 491 | * Get the frontmatter of a document. |
||
| 492 | * |
||
| 493 | * @param string $file to get frontmatter from. |
||
| 494 | * |
||
| 495 | * @return array as frontmatter. |
||
| 496 | */ |
||
| 497 | private function getFrontmatter($file) |
||
| 509 | |||
| 510 | |||
| 511 | |||
| 512 | // == Look up route in index =================================== |
||
| 513 | |||
| 514 | /** |
||
| 515 | * Check if currrent route is a supported meta route. |
||
| 516 | * |
||
| 517 | * @param string $route current route used to access page. |
||
| 518 | * |
||
| 519 | * @return string as route. |
||
| 520 | */ |
||
| 521 | private function checkForMetaRoute($route) |
||
| 543 | |||
| 544 | |||
| 545 | |||
| 546 | /** |
||
| 547 | * Map the route to the correct key in the index. |
||
| 548 | * |
||
| 549 | * @param string $route current route used to access page. |
||
| 550 | * |
||
| 551 | * @return string as key or false if no match. |
||
| 552 | */ |
||
| 553 | private function mapRoute2IndexKey($route) |
||
| 567 | |||
| 568 | |||
| 569 | |||
| 570 | /** |
||
| 571 | * Map the route to the correct entry in the index. |
||
| 572 | * |
||
| 573 | * @param string $route current route used to access page. |
||
| 574 | * |
||
| 575 | * @return array as the matched route. |
||
| 576 | */ |
||
| 577 | private function mapRoute2Index($route) |
||
| 590 | |||
| 591 | |||
| 592 | |||
| 593 | // = Get view data by merging from meta and current frontmatter ========= |
||
| 594 | |||
| 595 | /** |
||
| 596 | * Get view by mergin information from meta and frontmatter. |
||
| 597 | * |
||
| 598 | * @param string $route current route used to access page. |
||
| 599 | * @param array $frontmatter for the content. |
||
| 600 | * @param string $key for the view to retrive. |
||
| 601 | * |
||
| 602 | * @return array with data to add as view. |
||
| 603 | */ |
||
| 604 | private function getView($route, $frontmatter, $key) |
||
| 622 | |||
| 623 | |||
| 624 | |||
| 625 | /** |
||
| 626 | * Get details on extra views. |
||
| 627 | * |
||
| 628 | * @param string $route current route used to access page. |
||
| 629 | * @param array $frontmatter for the content. |
||
| 630 | * |
||
| 631 | * @return array with page data to send to view. |
||
| 632 | */ |
||
| 633 | private function getViews($route, $frontmatter) |
||
| 659 | |||
| 660 | |||
| 661 | |||
| 662 | // == Create and load content =================================== |
||
| 663 | |||
| 664 | /** |
||
| 665 | * Map url to content, even internal content, if such mapping can be done. |
||
| 666 | * |
||
| 667 | * @param string $route route to look up. |
||
| 668 | * |
||
| 669 | * @return object with content and filtered version. |
||
| 670 | */ |
||
| 671 | private function createContentForInternalRoute($route) |
||
| 704 | |||
| 705 | |||
| 706 | |||
| 707 | /** |
||
| 708 | * Look up the route in the index and use that to retrieve the filtered |
||
| 709 | * content. |
||
| 710 | * |
||
| 711 | * @param string $route to look up. |
||
| 712 | * |
||
| 713 | * @return array with content and filtered version. |
||
| 714 | */ |
||
| 715 | private function mapRoute2Content($route) |
||
| 723 | |||
| 724 | |||
| 725 | |||
| 726 | /** |
||
| 727 | * Load content file and frontmatter, this is the first time we process |
||
| 728 | * the content. |
||
| 729 | * |
||
| 730 | * @param string $key to index with details on the route. |
||
| 731 | * |
||
| 732 | * @throws NotFoundException when mapping can not be done. |
||
| 733 | * |
||
| 734 | * @return void. |
||
| 735 | */ |
||
| 736 | private function loadFileContentPhaseOne($key) |
||
| 757 | |||
| 758 | |||
| 759 | |||
| 760 | // == Process content phase 2 =================================== |
||
| 761 | // TODO REFACTOR THIS? |
||
| 762 | |||
| 763 | /** |
||
| 764 | * Look up the route in the index and use that to retrieve the filtered |
||
| 765 | * content. |
||
| 766 | * |
||
| 767 | * @param array &$content to process. |
||
| 768 | * @param object &$filtered to use for settings. |
||
| 769 | * |
||
| 770 | * @return array with content and filtered version. |
||
| 771 | */ |
||
| 772 | private function processMainContentPhaseTwo(&$content, &$filtered) |
||
| 841 | |||
| 842 | |||
| 843 | |||
| 844 | // == Public methods ============================================ |
||
| 845 | |||
| 846 | /** |
||
| 847 | * Map url to content, even internal content, if such mapping can be done. |
||
| 848 | * |
||
| 849 | * @param string $route optional route to look up. |
||
| 850 | * |
||
| 851 | * @return object with content and filtered version. |
||
| 852 | */ |
||
| 853 | public function contentForInternalRoute($route = null) |
||
| 873 | |||
| 874 | |||
| 875 | |||
| 876 | /** |
||
| 877 | * Map url to content if such mapping can be done, exclude internal routes. |
||
| 878 | * |
||
| 879 | * @param string $route optional route to look up. |
||
| 880 | * |
||
| 881 | * @return object with content and filtered version. |
||
| 882 | */ |
||
| 883 | public function contentForRoute($route = null) |
||
| 893 | } |
||
| 894 |
In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code:
Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion: