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 LinksUpdate 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 LinksUpdate, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 33 | class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate { |
||
| 34 | // @todo make members protected, but make sure extensions don't break |
||
| 35 | |||
| 36 | /** @var int Page ID of the article linked from */ |
||
| 37 | public $mId; |
||
| 38 | |||
| 39 | /** @var Title Title object of the article linked from */ |
||
| 40 | public $mTitle; |
||
| 41 | |||
| 42 | /** @var ParserOutput */ |
||
| 43 | public $mParserOutput; |
||
| 44 | |||
| 45 | /** @var array Map of title strings to IDs for the links in the document */ |
||
| 46 | public $mLinks; |
||
| 47 | |||
| 48 | /** @var array DB keys of the images used, in the array key only */ |
||
| 49 | public $mImages; |
||
| 50 | |||
| 51 | /** @var array Map of title strings to IDs for the template references, including broken ones */ |
||
| 52 | public $mTemplates; |
||
| 53 | |||
| 54 | /** @var array URLs of external links, array key only */ |
||
| 55 | public $mExternals; |
||
| 56 | |||
| 57 | /** @var array Map of category names to sort keys */ |
||
| 58 | public $mCategories; |
||
| 59 | |||
| 60 | /** @var array Map of language codes to titles */ |
||
| 61 | public $mInterlangs; |
||
| 62 | |||
| 63 | /** @var array 2-D map of (prefix => DBK => 1) */ |
||
| 64 | public $mInterwikis; |
||
| 65 | |||
| 66 | /** @var array Map of arbitrary name to value */ |
||
| 67 | public $mProperties; |
||
| 68 | |||
| 69 | /** @var bool Whether to queue jobs for recursive updates */ |
||
| 70 | public $mRecursive; |
||
| 71 | |||
| 72 | /** @var Revision Revision for which this update has been triggered */ |
||
| 73 | private $mRevision; |
||
| 74 | |||
| 75 | /** |
||
| 76 | * @var null|array Added links if calculated. |
||
| 77 | */ |
||
| 78 | private $linkInsertions = null; |
||
| 79 | |||
| 80 | /** |
||
| 81 | * @var null|array Deleted links if calculated. |
||
| 82 | */ |
||
| 83 | private $linkDeletions = null; |
||
| 84 | |||
| 85 | /** |
||
| 86 | * @var null|array Added properties if calculated. |
||
| 87 | */ |
||
| 88 | private $propertyInsertions = null; |
||
| 89 | |||
| 90 | /** |
||
| 91 | * @var null|array Deleted properties if calculated. |
||
| 92 | */ |
||
| 93 | private $propertyDeletions = null; |
||
| 94 | |||
| 95 | /** |
||
| 96 | * @var User|null |
||
| 97 | */ |
||
| 98 | private $user; |
||
| 99 | |||
| 100 | /** @var IDatabase */ |
||
| 101 | private $db; |
||
| 102 | |||
| 103 | /** |
||
| 104 | * Constructor |
||
| 105 | * |
||
| 106 | * @param Title $title Title of the page we're updating |
||
| 107 | * @param ParserOutput $parserOutput Output from a full parse of this page |
||
| 108 | * @param bool $recursive Queue jobs for recursive updates? |
||
| 109 | * @throws MWException |
||
| 110 | */ |
||
| 111 | function __construct( Title $title, ParserOutput $parserOutput, $recursive = true ) { |
||
| 159 | |||
| 160 | /** |
||
| 161 | * Update link tables with outgoing links from an updated article |
||
| 162 | * |
||
| 163 | * @note: this is managed by DeferredUpdates::execute(). Do not run this in a transaction. |
||
| 164 | */ |
||
| 165 | public function doUpdate() { |
||
| 185 | |||
| 186 | /** |
||
| 187 | * Acquire a lock for performing link table updates for a page on a DB |
||
| 188 | * |
||
| 189 | * @param IDatabase $dbw |
||
| 190 | * @param integer $pageId |
||
| 191 | * @param string $why One of (job, atomicity) |
||
| 192 | * @return ScopedCallback |
||
| 193 | * @throws RuntimeException |
||
| 194 | * @since 1.27 |
||
| 195 | */ |
||
| 196 | public static function acquirePageLock( IDatabase $dbw, $pageId, $why = 'atomicity' ) { |
||
| 205 | |||
| 206 | protected function doIncrementalUpdate() { |
||
| 296 | |||
| 297 | /** |
||
| 298 | * Queue recursive jobs for this page |
||
| 299 | * |
||
| 300 | * Which means do LinksUpdate on all pages that include the current page, |
||
| 301 | * using the job queue. |
||
| 302 | */ |
||
| 303 | protected function queueRecursiveJobs() { |
||
| 322 | |||
| 323 | /** |
||
| 324 | * Queue a RefreshLinks job for any table. |
||
| 325 | * |
||
| 326 | * @param Title $title Title to do job for |
||
| 327 | * @param string $table Table to use (e.g. 'templatelinks') |
||
| 328 | */ |
||
| 329 | public static function queueRecursiveJobsForTable( Title $title, $table ) { |
||
| 344 | |||
| 345 | /** |
||
| 346 | * @param array $cats |
||
| 347 | */ |
||
| 348 | private function invalidateCategories( $cats ) { |
||
| 351 | |||
| 352 | /** |
||
| 353 | * Update all the appropriate counts in the category table. |
||
| 354 | * @param array $added Associative array of category name => sort key |
||
| 355 | * @param array $deleted Associative array of category name => sort key |
||
| 356 | */ |
||
| 357 | private function updateCategoryCounts( array $added, array $deleted ) { |
||
| 377 | |||
| 378 | /** |
||
| 379 | * @param array $images |
||
| 380 | */ |
||
| 381 | private function invalidateImageDescriptions( $images ) { |
||
| 384 | |||
| 385 | /** |
||
| 386 | * Update a table by doing a delete query then an insert query |
||
| 387 | * @param string $table Table name |
||
| 388 | * @param string $prefix Field name prefix |
||
| 389 | * @param array $deletions |
||
| 390 | * @param array $insertions Rows to insert |
||
| 391 | */ |
||
| 392 | private function incrTableUpdate( $table, $prefix, $deletions, $insertions ) { |
||
| 464 | |||
| 465 | /** |
||
| 466 | * Get an array of pagelinks insertions for passing to the DB |
||
| 467 | * Skips the titles specified by the 2-D array $existing |
||
| 468 | * @param array $existing |
||
| 469 | * @return array |
||
| 470 | */ |
||
| 471 | View Code Duplication | private function getLinkInsertions( $existing = [] ) { |
|
| 489 | |||
| 490 | /** |
||
| 491 | * Get an array of template insertions. Like getLinkInsertions() |
||
| 492 | * @param array $existing |
||
| 493 | * @return array |
||
| 494 | */ |
||
| 495 | View Code Duplication | private function getTemplateInsertions( $existing = [] ) { |
|
| 511 | |||
| 512 | /** |
||
| 513 | * Get an array of image insertions |
||
| 514 | * Skips the names specified in $existing |
||
| 515 | * @param array $existing |
||
| 516 | * @return array |
||
| 517 | */ |
||
| 518 | private function getImageInsertions( $existing = [] ) { |
||
| 531 | |||
| 532 | /** |
||
| 533 | * Get an array of externallinks insertions. Skips the names specified in $existing |
||
| 534 | * @param array $existing |
||
| 535 | * @return array |
||
| 536 | */ |
||
| 537 | private function getExternalInsertions( $existing = [] ) { |
||
| 553 | |||
| 554 | /** |
||
| 555 | * Get an array of category insertions |
||
| 556 | * |
||
| 557 | * @param array $existing Mapping existing category names to sort keys. If both |
||
| 558 | * match a link in $this, the link will be omitted from the output |
||
| 559 | * |
||
| 560 | * @return array |
||
| 561 | */ |
||
| 562 | private function getCategoryInsertions( $existing = [] ) { |
||
| 598 | |||
| 599 | /** |
||
| 600 | * Get an array of interlanguage link insertions |
||
| 601 | * |
||
| 602 | * @param array $existing Mapping existing language codes to titles |
||
| 603 | * |
||
| 604 | * @return array |
||
| 605 | */ |
||
| 606 | private function getInterlangInsertions( $existing = [] ) { |
||
| 619 | |||
| 620 | /** |
||
| 621 | * Get an array of page property insertions |
||
| 622 | * @param array $existing |
||
| 623 | * @return array |
||
| 624 | */ |
||
| 625 | function getPropertyInsertions( $existing = [] ) { |
||
| 635 | |||
| 636 | /** |
||
| 637 | * Returns an associative array to be used for inserting a row into |
||
| 638 | * the page_props table. Besides the given property name, this will |
||
| 639 | * include the page id from $this->mId and any property value from |
||
| 640 | * $this->mProperties. |
||
| 641 | * |
||
| 642 | * The array returned will include the pp_sortkey field if this |
||
| 643 | * is present in the database (as indicated by $wgPagePropsHaveSortkey). |
||
| 644 | * The sortkey value is currently determined by getPropertySortKeyValue(). |
||
| 645 | * |
||
| 646 | * @note this assumes that $this->mProperties[$prop] is defined. |
||
| 647 | * |
||
| 648 | * @param string $prop The name of the property. |
||
| 649 | * |
||
| 650 | * @return array |
||
| 651 | */ |
||
| 652 | private function getPagePropRowData( $prop ) { |
||
| 669 | |||
| 670 | /** |
||
| 671 | * Determines the sort key for the given property value. |
||
| 672 | * This will return $value if it is a float or int, |
||
| 673 | * 1 or resp. 0 if it is a bool, and null otherwise. |
||
| 674 | * |
||
| 675 | * @note In the future, we may allow the sortkey to be specified explicitly |
||
| 676 | * in ParserOutput::setProperty. |
||
| 677 | * |
||
| 678 | * @param mixed $value |
||
| 679 | * |
||
| 680 | * @return float|null |
||
| 681 | */ |
||
| 682 | private function getPropertySortKeyValue( $value ) { |
||
| 689 | |||
| 690 | /** |
||
| 691 | * Get an array of interwiki insertions for passing to the DB |
||
| 692 | * Skips the titles specified by the 2-D array $existing |
||
| 693 | * @param array $existing |
||
| 694 | * @return array |
||
| 695 | */ |
||
| 696 | private function getInterwikiInsertions( $existing = [] ) { |
||
| 714 | |||
| 715 | /** |
||
| 716 | * Given an array of existing links, returns those links which are not in $this |
||
| 717 | * and thus should be deleted. |
||
| 718 | * @param array $existing |
||
| 719 | * @return array |
||
| 720 | */ |
||
| 721 | View Code Duplication | private function getLinkDeletions( $existing ) { |
|
| 733 | |||
| 734 | /** |
||
| 735 | * Given an array of existing templates, returns those templates which are not in $this |
||
| 736 | * and thus should be deleted. |
||
| 737 | * @param array $existing |
||
| 738 | * @return array |
||
| 739 | */ |
||
| 740 | View Code Duplication | private function getTemplateDeletions( $existing ) { |
|
| 752 | |||
| 753 | /** |
||
| 754 | * Given an array of existing images, returns those images which are not in $this |
||
| 755 | * and thus should be deleted. |
||
| 756 | * @param array $existing |
||
| 757 | * @return array |
||
| 758 | */ |
||
| 759 | private function getImageDeletions( $existing ) { |
||
| 762 | |||
| 763 | /** |
||
| 764 | * Given an array of existing external links, returns those links which are not |
||
| 765 | * in $this and thus should be deleted. |
||
| 766 | * @param array $existing |
||
| 767 | * @return array |
||
| 768 | */ |
||
| 769 | private function getExternalDeletions( $existing ) { |
||
| 772 | |||
| 773 | /** |
||
| 774 | * Given an array of existing categories, returns those categories which are not in $this |
||
| 775 | * and thus should be deleted. |
||
| 776 | * @param array $existing |
||
| 777 | * @return array |
||
| 778 | */ |
||
| 779 | private function getCategoryDeletions( $existing ) { |
||
| 782 | |||
| 783 | /** |
||
| 784 | * Given an array of existing interlanguage links, returns those links which are not |
||
| 785 | * in $this and thus should be deleted. |
||
| 786 | * @param array $existing |
||
| 787 | * @return array |
||
| 788 | */ |
||
| 789 | private function getInterlangDeletions( $existing ) { |
||
| 792 | |||
| 793 | /** |
||
| 794 | * Get array of properties which should be deleted. |
||
| 795 | * @param array $existing |
||
| 796 | * @return array |
||
| 797 | */ |
||
| 798 | function getPropertyDeletions( $existing ) { |
||
| 801 | |||
| 802 | /** |
||
| 803 | * Given an array of existing interwiki links, returns those links which are not in $this |
||
| 804 | * and thus should be deleted. |
||
| 805 | * @param array $existing |
||
| 806 | * @return array |
||
| 807 | */ |
||
| 808 | View Code Duplication | private function getInterwikiDeletions( $existing ) { |
|
| 820 | |||
| 821 | /** |
||
| 822 | * Get an array of existing links, as a 2-D array |
||
| 823 | * |
||
| 824 | * @return array |
||
| 825 | */ |
||
| 826 | private function getExistingLinks() { |
||
| 839 | |||
| 840 | /** |
||
| 841 | * Get an array of existing templates, as a 2-D array |
||
| 842 | * |
||
| 843 | * @return array |
||
| 844 | */ |
||
| 845 | private function getExistingTemplates() { |
||
| 858 | |||
| 859 | /** |
||
| 860 | * Get an array of existing images, image names in the keys |
||
| 861 | * |
||
| 862 | * @return array |
||
| 863 | */ |
||
| 864 | private function getExistingImages() { |
||
| 874 | |||
| 875 | /** |
||
| 876 | * Get an array of existing external links, URLs in the keys |
||
| 877 | * |
||
| 878 | * @return array |
||
| 879 | */ |
||
| 880 | private function getExistingExternals() { |
||
| 890 | |||
| 891 | /** |
||
| 892 | * Get an array of existing categories, with the name in the key and sort key in the value. |
||
| 893 | * |
||
| 894 | * @return array |
||
| 895 | */ |
||
| 896 | private function getExistingCategories() { |
||
| 906 | |||
| 907 | /** |
||
| 908 | * Get an array of existing interlanguage links, with the language code in the key and the |
||
| 909 | * title in the value. |
||
| 910 | * |
||
| 911 | * @return array |
||
| 912 | */ |
||
| 913 | private function getExistingInterlangs() { |
||
| 923 | |||
| 924 | /** |
||
| 925 | * Get an array of existing inline interwiki links, as a 2-D array |
||
| 926 | * @return array (prefix => array(dbkey => 1)) |
||
| 927 | */ |
||
| 928 | private function getExistingInterwikis() { |
||
| 941 | |||
| 942 | /** |
||
| 943 | * Get an array of existing categories, with the name in the key and sort key in the value. |
||
| 944 | * |
||
| 945 | * @return array Array of property names and values |
||
| 946 | */ |
||
| 947 | private function getExistingProperties() { |
||
| 957 | |||
| 958 | /** |
||
| 959 | * Return the title object of the page being updated |
||
| 960 | * @return Title |
||
| 961 | */ |
||
| 962 | public function getTitle() { |
||
| 965 | |||
| 966 | /** |
||
| 967 | * Returns parser output |
||
| 968 | * @since 1.19 |
||
| 969 | * @return ParserOutput |
||
| 970 | */ |
||
| 971 | public function getParserOutput() { |
||
| 974 | |||
| 975 | /** |
||
| 976 | * Return the list of images used as generated by the parser |
||
| 977 | * @return array |
||
| 978 | */ |
||
| 979 | public function getImages() { |
||
| 982 | |||
| 983 | /** |
||
| 984 | * Set the revision corresponding to this LinksUpdate |
||
| 985 | * |
||
| 986 | * @since 1.27 |
||
| 987 | * |
||
| 988 | * @param Revision $revision |
||
| 989 | */ |
||
| 990 | public function setRevision( Revision $revision ) { |
||
| 993 | |||
| 994 | /** |
||
| 995 | * @since 1.28 |
||
| 996 | * @return null|Revision |
||
| 997 | */ |
||
| 998 | public function getRevision() { |
||
| 1001 | |||
| 1002 | /** |
||
| 1003 | * Set the User who triggered this LinksUpdate |
||
| 1004 | * |
||
| 1005 | * @since 1.27 |
||
| 1006 | * @param User $user |
||
| 1007 | */ |
||
| 1008 | public function setTriggeringUser( User $user ) { |
||
| 1011 | |||
| 1012 | /** |
||
| 1013 | * @since 1.27 |
||
| 1014 | * @return null|User |
||
| 1015 | */ |
||
| 1016 | public function getTriggeringUser() { |
||
| 1019 | |||
| 1020 | /** |
||
| 1021 | * Invalidate any necessary link lists related to page property changes |
||
| 1022 | * @param array $changed |
||
| 1023 | */ |
||
| 1024 | private function invalidateProperties( $changed ) { |
||
| 1039 | |||
| 1040 | /** |
||
| 1041 | * Fetch page links added by this LinksUpdate. Only available after the update is complete. |
||
| 1042 | * @since 1.22 |
||
| 1043 | * @return null|array Array of Titles |
||
| 1044 | */ |
||
| 1045 | public function getAddedLinks() { |
||
| 1056 | |||
| 1057 | /** |
||
| 1058 | * Fetch page links removed by this LinksUpdate. Only available after the update is complete. |
||
| 1059 | * @since 1.22 |
||
| 1060 | * @return null|array Array of Titles |
||
| 1061 | */ |
||
| 1062 | public function getRemovedLinks() { |
||
| 1075 | |||
| 1076 | /** |
||
| 1077 | * Fetch page properties added by this LinksUpdate. |
||
| 1078 | * Only available after the update is complete. |
||
| 1079 | * @since 1.28 |
||
| 1080 | * @return null|array |
||
| 1081 | */ |
||
| 1082 | public function getAddedProperties() { |
||
| 1085 | |||
| 1086 | /** |
||
| 1087 | * Fetch page properties removed by this LinksUpdate. |
||
| 1088 | * Only available after the update is complete. |
||
| 1089 | * @since 1.28 |
||
| 1090 | * @return null|array |
||
| 1091 | */ |
||
| 1092 | public function getRemovedProperties() { |
||
| 1095 | |||
| 1096 | /** |
||
| 1097 | * Update links table freshness |
||
| 1098 | */ |
||
| 1099 | private function updateLinksTimestamp() { |
||
| 1110 | |||
| 1111 | /** |
||
| 1112 | * @return IDatabase |
||
| 1113 | */ |
||
| 1114 | private function getDB() { |
||
| 1121 | |||
| 1122 | public function getAsJobSpecification() { |
||
| 1154 | } |
||
| 1155 |
Let’s assume that you have a directory layout like this:
. |-- OtherDir | |-- Bar.php | `-- Foo.php `-- SomeDir `-- Foo.phpand let’s assume the following content of
Bar.php:If both files
OtherDir/Foo.phpandSomeDir/Foo.phpare loaded in the same runtime, you will see a PHP error such as the following:PHP Fatal error: Cannot use SomeDir\Foo as Foo because the name is already in use in OtherDir/Foo.phpHowever, as
OtherDir/Foo.phpdoes not necessarily have to be loaded and the error is only triggered if it is loaded beforeOtherDir/Bar.php, this problem might go unnoticed for a while. In order to prevent this error from surfacing, you must import the namespace with a different alias: