wikimedia /
mediawiki
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 |
||
| 30 | class LinksUpdate extends SqlDataUpdate implements EnqueueableDataUpdate { |
||
| 31 | // @todo make members protected, but make sure extensions don't break |
||
| 32 | |||
| 33 | /** @var int Page ID of the article linked from */ |
||
| 34 | public $mId; |
||
| 35 | |||
| 36 | /** @var Title Title object of the article linked from */ |
||
| 37 | public $mTitle; |
||
| 38 | |||
| 39 | /** @var ParserOutput */ |
||
| 40 | public $mParserOutput; |
||
| 41 | |||
| 42 | /** @var array Map of title strings to IDs for the links in the document */ |
||
| 43 | public $mLinks; |
||
| 44 | |||
| 45 | /** @var array DB keys of the images used, in the array key only */ |
||
| 46 | public $mImages; |
||
| 47 | |||
| 48 | /** @var array Map of title strings to IDs for the template references, including broken ones */ |
||
| 49 | public $mTemplates; |
||
| 50 | |||
| 51 | /** @var array URLs of external links, array key only */ |
||
| 52 | public $mExternals; |
||
| 53 | |||
| 54 | /** @var array Map of category names to sort keys */ |
||
| 55 | public $mCategories; |
||
| 56 | |||
| 57 | /** @var array Map of language codes to titles */ |
||
| 58 | public $mInterlangs; |
||
| 59 | |||
| 60 | /** @var array 2-D map of (prefix => DBK => 1) */ |
||
| 61 | public $mInterwikis; |
||
| 62 | |||
| 63 | /** @var array Map of arbitrary name to value */ |
||
| 64 | public $mProperties; |
||
| 65 | |||
| 66 | /** @var bool Whether to queue jobs for recursive updates */ |
||
| 67 | public $mRecursive; |
||
| 68 | |||
| 69 | /** @var Revision Revision for which this update has been triggered */ |
||
| 70 | private $mRevision; |
||
| 71 | |||
| 72 | /** |
||
| 73 | * @var null|array Added links if calculated. |
||
| 74 | */ |
||
| 75 | private $linkInsertions = null; |
||
| 76 | |||
| 77 | /** |
||
| 78 | * @var null|array Deleted links if calculated. |
||
| 79 | */ |
||
| 80 | private $linkDeletions = null; |
||
| 81 | |||
| 82 | /** |
||
| 83 | * @var null|array Added properties if calculated. |
||
| 84 | */ |
||
| 85 | private $propertyInsertions = null; |
||
| 86 | |||
| 87 | /** |
||
| 88 | * @var null|array Deleted properties if calculated. |
||
| 89 | */ |
||
| 90 | private $propertyDeletions = null; |
||
| 91 | |||
| 92 | /** |
||
| 93 | * @var User|null |
||
| 94 | */ |
||
| 95 | private $user; |
||
| 96 | |||
| 97 | /** |
||
| 98 | * Constructor |
||
| 99 | * |
||
| 100 | * @param Title $title Title of the page we're updating |
||
| 101 | * @param ParserOutput $parserOutput Output from a full parse of this page |
||
| 102 | * @param bool $recursive Queue jobs for recursive updates? |
||
| 103 | * @throws MWException |
||
| 104 | */ |
||
| 105 | function __construct( Title $title, ParserOutput $parserOutput, $recursive = true ) { |
||
| 154 | |||
| 155 | /** |
||
| 156 | * Update link tables with outgoing links from an updated article |
||
| 157 | * |
||
| 158 | * @note: this is managed by DeferredUpdates::execute(). Do not run this in a transaction. |
||
| 159 | */ |
||
| 160 | public function doUpdate() { |
||
| 175 | |||
| 176 | /** |
||
| 177 | * Acquire a lock for performing link table updates for a page on a DB |
||
| 178 | * |
||
| 179 | * @param IDatabase $dbw |
||
| 180 | * @param integer $pageId |
||
| 181 | * @param string $why One of (job, atomicity) |
||
| 182 | * @return ScopedCallback |
||
| 183 | * @throws RuntimeException |
||
| 184 | * @since 1.27 |
||
| 185 | */ |
||
| 186 | public static function acquirePageLock( IDatabase $dbw, $pageId, $why = 'atomicity' ) { |
||
| 195 | |||
| 196 | protected function doIncrementalUpdate() { |
||
| 265 | |||
| 266 | /** |
||
| 267 | * Queue recursive jobs for this page |
||
| 268 | * |
||
| 269 | * Which means do LinksUpdate on all pages that include the current page, |
||
| 270 | * using the job queue. |
||
| 271 | */ |
||
| 272 | protected function queueRecursiveJobs() { |
||
| 291 | |||
| 292 | /** |
||
| 293 | * Queue a RefreshLinks job for any table. |
||
| 294 | * |
||
| 295 | * @param Title $title Title to do job for |
||
| 296 | * @param string $table Table to use (e.g. 'templatelinks') |
||
| 297 | */ |
||
| 298 | public static function queueRecursiveJobsForTable( Title $title, $table ) { |
||
| 313 | |||
| 314 | /** |
||
| 315 | * @param array $cats |
||
| 316 | */ |
||
| 317 | function invalidateCategories( $cats ) { |
||
| 320 | |||
| 321 | /** |
||
| 322 | * Update all the appropriate counts in the category table. |
||
| 323 | * @param array $added Associative array of category name => sort key |
||
| 324 | * @param array $deleted Associative array of category name => sort key |
||
| 325 | */ |
||
| 326 | function updateCategoryCounts( $added, $deleted ) { |
||
| 332 | |||
| 333 | /** |
||
| 334 | * @param array $images |
||
| 335 | */ |
||
| 336 | function invalidateImageDescriptions( $images ) { |
||
| 339 | |||
| 340 | /** |
||
| 341 | * Update a table by doing a delete query then an insert query |
||
| 342 | * @param string $table Table name |
||
| 343 | * @param string $prefix Field name prefix |
||
| 344 | * @param array $deletions |
||
| 345 | * @param array $insertions Rows to insert |
||
| 346 | */ |
||
| 347 | private function incrTableUpdate( $table, $prefix, $deletions, $insertions ) { |
||
| 418 | |||
| 419 | /** |
||
| 420 | * Get an array of pagelinks insertions for passing to the DB |
||
| 421 | * Skips the titles specified by the 2-D array $existing |
||
| 422 | * @param array $existing |
||
| 423 | * @return array |
||
| 424 | */ |
||
| 425 | View Code Duplication | private function getLinkInsertions( $existing = [] ) { |
|
| 443 | |||
| 444 | /** |
||
| 445 | * Get an array of template insertions. Like getLinkInsertions() |
||
| 446 | * @param array $existing |
||
| 447 | * @return array |
||
| 448 | */ |
||
| 449 | View Code Duplication | private function getTemplateInsertions( $existing = [] ) { |
|
| 465 | |||
| 466 | /** |
||
| 467 | * Get an array of image insertions |
||
| 468 | * Skips the names specified in $existing |
||
| 469 | * @param array $existing |
||
| 470 | * @return array |
||
| 471 | */ |
||
| 472 | private function getImageInsertions( $existing = [] ) { |
||
| 485 | |||
| 486 | /** |
||
| 487 | * Get an array of externallinks insertions. Skips the names specified in $existing |
||
| 488 | * @param array $existing |
||
| 489 | * @return array |
||
| 490 | */ |
||
| 491 | private function getExternalInsertions( $existing = [] ) { |
||
| 507 | |||
| 508 | /** |
||
| 509 | * Get an array of category insertions |
||
| 510 | * |
||
| 511 | * @param array $existing Mapping existing category names to sort keys. If both |
||
| 512 | * match a link in $this, the link will be omitted from the output |
||
| 513 | * |
||
| 514 | * @return array |
||
| 515 | */ |
||
| 516 | private function getCategoryInsertions( $existing = [] ) { |
||
| 552 | |||
| 553 | /** |
||
| 554 | * Get an array of interlanguage link insertions |
||
| 555 | * |
||
| 556 | * @param array $existing Mapping existing language codes to titles |
||
| 557 | * |
||
| 558 | * @return array |
||
| 559 | */ |
||
| 560 | private function getInterlangInsertions( $existing = [] ) { |
||
| 573 | |||
| 574 | /** |
||
| 575 | * Get an array of page property insertions |
||
| 576 | * @param array $existing |
||
| 577 | * @return array |
||
| 578 | */ |
||
| 579 | function getPropertyInsertions( $existing = [] ) { |
||
| 589 | |||
| 590 | /** |
||
| 591 | * Returns an associative array to be used for inserting a row into |
||
| 592 | * the page_props table. Besides the given property name, this will |
||
| 593 | * include the page id from $this->mId and any property value from |
||
| 594 | * $this->mProperties. |
||
| 595 | * |
||
| 596 | * The array returned will include the pp_sortkey field if this |
||
| 597 | * is present in the database (as indicated by $wgPagePropsHaveSortkey). |
||
| 598 | * The sortkey value is currently determined by getPropertySortKeyValue(). |
||
| 599 | * |
||
| 600 | * @note this assumes that $this->mProperties[$prop] is defined. |
||
| 601 | * |
||
| 602 | * @param string $prop The name of the property. |
||
| 603 | * |
||
| 604 | * @return array |
||
| 605 | */ |
||
| 606 | private function getPagePropRowData( $prop ) { |
||
| 623 | |||
| 624 | /** |
||
| 625 | * Determines the sort key for the given property value. |
||
| 626 | * This will return $value if it is a float or int, |
||
| 627 | * 1 or resp. 0 if it is a bool, and null otherwise. |
||
| 628 | * |
||
| 629 | * @note In the future, we may allow the sortkey to be specified explicitly |
||
| 630 | * in ParserOutput::setProperty. |
||
| 631 | * |
||
| 632 | * @param mixed $value |
||
| 633 | * |
||
| 634 | * @return float|null |
||
| 635 | */ |
||
| 636 | private function getPropertySortKeyValue( $value ) { |
||
| 643 | |||
| 644 | /** |
||
| 645 | * Get an array of interwiki insertions for passing to the DB |
||
| 646 | * Skips the titles specified by the 2-D array $existing |
||
| 647 | * @param array $existing |
||
| 648 | * @return array |
||
| 649 | */ |
||
| 650 | private function getInterwikiInsertions( $existing = [] ) { |
||
| 668 | |||
| 669 | /** |
||
| 670 | * Given an array of existing links, returns those links which are not in $this |
||
| 671 | * and thus should be deleted. |
||
| 672 | * @param array $existing |
||
| 673 | * @return array |
||
| 674 | */ |
||
| 675 | View Code Duplication | private function getLinkDeletions( $existing ) { |
|
| 687 | |||
| 688 | /** |
||
| 689 | * Given an array of existing templates, returns those templates which are not in $this |
||
| 690 | * and thus should be deleted. |
||
| 691 | * @param array $existing |
||
| 692 | * @return array |
||
| 693 | */ |
||
| 694 | View Code Duplication | private function getTemplateDeletions( $existing ) { |
|
| 706 | |||
| 707 | /** |
||
| 708 | * Given an array of existing images, returns those images which are not in $this |
||
| 709 | * and thus should be deleted. |
||
| 710 | * @param array $existing |
||
| 711 | * @return array |
||
| 712 | */ |
||
| 713 | private function getImageDeletions( $existing ) { |
||
| 716 | |||
| 717 | /** |
||
| 718 | * Given an array of existing external links, returns those links which are not |
||
| 719 | * in $this and thus should be deleted. |
||
| 720 | * @param array $existing |
||
| 721 | * @return array |
||
| 722 | */ |
||
| 723 | private function getExternalDeletions( $existing ) { |
||
| 726 | |||
| 727 | /** |
||
| 728 | * Given an array of existing categories, returns those categories which are not in $this |
||
| 729 | * and thus should be deleted. |
||
| 730 | * @param array $existing |
||
| 731 | * @return array |
||
| 732 | */ |
||
| 733 | private function getCategoryDeletions( $existing ) { |
||
| 736 | |||
| 737 | /** |
||
| 738 | * Given an array of existing interlanguage links, returns those links which are not |
||
| 739 | * in $this and thus should be deleted. |
||
| 740 | * @param array $existing |
||
| 741 | * @return array |
||
| 742 | */ |
||
| 743 | private function getInterlangDeletions( $existing ) { |
||
| 746 | |||
| 747 | /** |
||
| 748 | * Get array of properties which should be deleted. |
||
| 749 | * @param array $existing |
||
| 750 | * @return array |
||
| 751 | */ |
||
| 752 | function getPropertyDeletions( $existing ) { |
||
| 755 | |||
| 756 | /** |
||
| 757 | * Given an array of existing interwiki links, returns those links which are not in $this |
||
| 758 | * and thus should be deleted. |
||
| 759 | * @param array $existing |
||
| 760 | * @return array |
||
| 761 | */ |
||
| 762 | View Code Duplication | private function getInterwikiDeletions( $existing ) { |
|
| 774 | |||
| 775 | /** |
||
| 776 | * Get an array of existing links, as a 2-D array |
||
| 777 | * |
||
| 778 | * @return array |
||
| 779 | */ |
||
| 780 | private function getExistingLinks() { |
||
| 793 | |||
| 794 | /** |
||
| 795 | * Get an array of existing templates, as a 2-D array |
||
| 796 | * |
||
| 797 | * @return array |
||
| 798 | */ |
||
| 799 | private function getExistingTemplates() { |
||
| 812 | |||
| 813 | /** |
||
| 814 | * Get an array of existing images, image names in the keys |
||
| 815 | * |
||
| 816 | * @return array |
||
| 817 | */ |
||
| 818 | private function getExistingImages() { |
||
| 828 | |||
| 829 | /** |
||
| 830 | * Get an array of existing external links, URLs in the keys |
||
| 831 | * |
||
| 832 | * @return array |
||
| 833 | */ |
||
| 834 | private function getExistingExternals() { |
||
| 844 | |||
| 845 | /** |
||
| 846 | * Get an array of existing categories, with the name in the key and sort key in the value. |
||
| 847 | * |
||
| 848 | * @return array |
||
| 849 | */ |
||
| 850 | private function getExistingCategories() { |
||
| 860 | |||
| 861 | /** |
||
| 862 | * Get an array of existing interlanguage links, with the language code in the key and the |
||
| 863 | * title in the value. |
||
| 864 | * |
||
| 865 | * @return array |
||
| 866 | */ |
||
| 867 | private function getExistingInterlangs() { |
||
| 877 | |||
| 878 | /** |
||
| 879 | * Get an array of existing inline interwiki links, as a 2-D array |
||
| 880 | * @return array (prefix => array(dbkey => 1)) |
||
| 881 | */ |
||
| 882 | protected function getExistingInterwikis() { |
||
| 895 | |||
| 896 | /** |
||
| 897 | * Get an array of existing categories, with the name in the key and sort key in the value. |
||
| 898 | * |
||
| 899 | * @return array Array of property names and values |
||
| 900 | */ |
||
| 901 | private function getExistingProperties() { |
||
| 911 | |||
| 912 | /** |
||
| 913 | * Return the title object of the page being updated |
||
| 914 | * @return Title |
||
| 915 | */ |
||
| 916 | public function getTitle() { |
||
| 919 | |||
| 920 | /** |
||
| 921 | * Returns parser output |
||
| 922 | * @since 1.19 |
||
| 923 | * @return ParserOutput |
||
| 924 | */ |
||
| 925 | public function getParserOutput() { |
||
| 928 | |||
| 929 | /** |
||
| 930 | * Return the list of images used as generated by the parser |
||
| 931 | * @return array |
||
| 932 | */ |
||
| 933 | public function getImages() { |
||
| 936 | |||
| 937 | /** |
||
| 938 | * Set the revision corresponding to this LinksUpdate |
||
| 939 | * |
||
| 940 | * @since 1.27 |
||
| 941 | * |
||
| 942 | * @param Revision $revision |
||
| 943 | */ |
||
| 944 | public function setRevision( Revision $revision ) { |
||
| 947 | |||
| 948 | /** |
||
| 949 | * @since 1.28 |
||
| 950 | * @return null|Revision |
||
| 951 | */ |
||
| 952 | public function getRevision() { |
||
| 955 | |||
| 956 | /** |
||
| 957 | * Set the User who triggered this LinksUpdate |
||
| 958 | * |
||
| 959 | * @since 1.27 |
||
| 960 | * @param User $user |
||
| 961 | */ |
||
| 962 | public function setTriggeringUser( User $user ) { |
||
| 965 | |||
| 966 | /** |
||
| 967 | * @since 1.27 |
||
| 968 | * @return null|User |
||
| 969 | */ |
||
| 970 | public function getTriggeringUser() { |
||
| 973 | |||
| 974 | /** |
||
| 975 | * Invalidate any necessary link lists related to page property changes |
||
| 976 | * @param array $changed |
||
| 977 | */ |
||
| 978 | private function invalidateProperties( $changed ) { |
||
| 993 | |||
| 994 | /** |
||
| 995 | * Fetch page links added by this LinksUpdate. Only available after the update is complete. |
||
| 996 | * @since 1.22 |
||
| 997 | * @return null|array Array of Titles |
||
| 998 | */ |
||
| 999 | public function getAddedLinks() { |
||
| 1010 | |||
| 1011 | /** |
||
| 1012 | * Fetch page links removed by this LinksUpdate. Only available after the update is complete. |
||
| 1013 | * @since 1.22 |
||
| 1014 | * @return null|array Array of Titles |
||
| 1015 | */ |
||
| 1016 | public function getRemovedLinks() { |
||
| 1029 | |||
| 1030 | /** |
||
| 1031 | * Fetch page properties added by this LinksUpdate. |
||
| 1032 | * Only available after the update is complete. |
||
| 1033 | * @since 1.28 |
||
| 1034 | * @return null|array |
||
| 1035 | */ |
||
| 1036 | public function getAddedProperties() { |
||
| 1039 | |||
| 1040 | /** |
||
| 1041 | * Fetch page properties removed by this LinksUpdate. |
||
| 1042 | * Only available after the update is complete. |
||
| 1043 | * @since 1.28 |
||
| 1044 | * @return null|array |
||
| 1045 | */ |
||
| 1046 | public function getRemovedProperties() { |
||
| 1049 | |||
| 1050 | /** |
||
| 1051 | * Update links table freshness |
||
| 1052 | */ |
||
| 1053 | protected function updateLinksTimestamp() { |
||
| 1064 | |||
| 1065 | public function getAsJobSpecification() { |
||
| 1097 | } |
||
| 1098 |
Loading history...
There are different options of fixing this problem.
If you want to be on the safe side, you can add an additional type-check:
If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:
Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.