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 RecentChange 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 RecentChange, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 63 | class RecentChange { |
||
| 64 | // Constants for the rc_source field. Extensions may also have |
||
| 65 | // their own source constants. |
||
| 66 | const SRC_EDIT = 'mw.edit'; |
||
| 67 | const SRC_NEW = 'mw.new'; |
||
| 68 | const SRC_LOG = 'mw.log'; |
||
| 69 | const SRC_EXTERNAL = 'mw.external'; // obsolete |
||
| 70 | const SRC_CATEGORIZE = 'mw.categorize'; |
||
| 71 | |||
| 72 | public $mAttribs = []; |
||
| 73 | public $mExtra = []; |
||
| 74 | |||
| 75 | /** |
||
| 76 | * @var Title |
||
| 77 | */ |
||
| 78 | public $mTitle = false; |
||
| 79 | |||
| 80 | /** |
||
| 81 | * @var User |
||
| 82 | */ |
||
| 83 | private $mPerformer = false; |
||
| 84 | |||
| 85 | public $numberofWatchingusers = 0; # Dummy to prevent error message in SpecialRecentChangesLinked |
||
| 86 | public $notificationtimestamp; |
||
| 87 | |||
| 88 | /** |
||
| 89 | * @var int Line number of recent change. Default -1. |
||
| 90 | */ |
||
| 91 | public $counter = -1; |
||
| 92 | |||
| 93 | /** |
||
| 94 | * @var array List of tags to apply |
||
| 95 | */ |
||
| 96 | private $tags = []; |
||
| 97 | |||
| 98 | /** |
||
| 99 | * @var array Array of change types |
||
| 100 | */ |
||
| 101 | private static $changeTypes = [ |
||
| 102 | 'edit' => RC_EDIT, |
||
| 103 | 'new' => RC_NEW, |
||
| 104 | 'log' => RC_LOG, |
||
| 105 | 'external' => RC_EXTERNAL, |
||
| 106 | 'categorize' => RC_CATEGORIZE, |
||
| 107 | ]; |
||
| 108 | |||
| 109 | # Factory methods |
||
| 110 | |||
| 111 | /** |
||
| 112 | * @param mixed $row |
||
| 113 | * @return RecentChange |
||
| 114 | */ |
||
| 115 | public static function newFromRow( $row ) { |
||
| 121 | |||
| 122 | /** |
||
| 123 | * Parsing text to RC_* constants |
||
| 124 | * @since 1.24 |
||
| 125 | * @param string|array $type |
||
| 126 | * @throws MWException |
||
| 127 | * @return int|array RC_TYPE |
||
| 128 | */ |
||
| 129 | public static function parseToRCType( $type ) { |
||
| 144 | |||
| 145 | /** |
||
| 146 | * Parsing RC_* constants to human-readable test |
||
| 147 | * @since 1.24 |
||
| 148 | * @param int $rcType |
||
| 149 | * @return string $type |
||
| 150 | */ |
||
| 151 | public static function parseFromRCType( $rcType ) { |
||
| 154 | |||
| 155 | /** |
||
| 156 | * Get an array of all change types |
||
| 157 | * |
||
| 158 | * @since 1.26 |
||
| 159 | * |
||
| 160 | * @return array |
||
| 161 | */ |
||
| 162 | public static function getChangeTypes() { |
||
| 165 | |||
| 166 | /** |
||
| 167 | * Obtain the recent change with a given rc_id value |
||
| 168 | * |
||
| 169 | * @param int $rcid The rc_id value to retrieve |
||
| 170 | * @return RecentChange|null |
||
| 171 | */ |
||
| 172 | public static function newFromId( $rcid ) { |
||
| 175 | |||
| 176 | /** |
||
| 177 | * Find the first recent change matching some specific conditions |
||
| 178 | * |
||
| 179 | * @param array $conds Array of conditions |
||
| 180 | * @param mixed $fname Override the method name in profiling/logs |
||
| 181 | * @param int $dbType DB_* constant |
||
| 182 | * |
||
| 183 | * @return RecentChange|null |
||
| 184 | */ |
||
| 185 | public static function newFromConds( |
||
| 198 | |||
| 199 | /** |
||
| 200 | * Return the list of recentchanges fields that should be selected to create |
||
| 201 | * a new recentchanges object. |
||
| 202 | * @return array |
||
| 203 | */ |
||
| 204 | public static function selectFields() { |
||
| 232 | |||
| 233 | # Accessors |
||
| 234 | |||
| 235 | /** |
||
| 236 | * @param array $attribs |
||
| 237 | */ |
||
| 238 | public function setAttribs( $attribs ) { |
||
| 241 | |||
| 242 | /** |
||
| 243 | * @param array $extra |
||
| 244 | */ |
||
| 245 | public function setExtra( $extra ) { |
||
| 248 | |||
| 249 | /** |
||
| 250 | * @return Title |
||
| 251 | */ |
||
| 252 | public function &getTitle() { |
||
| 259 | |||
| 260 | /** |
||
| 261 | * Get the User object of the person who performed this change. |
||
| 262 | * |
||
| 263 | * @return User |
||
| 264 | */ |
||
| 265 | public function getPerformer() { |
||
| 276 | |||
| 277 | /** |
||
| 278 | * Writes the data in this object to the database |
||
| 279 | * @param bool $noudp |
||
| 280 | */ |
||
| 281 | public function save( $noudp = false ) { |
||
| 379 | |||
| 380 | /** |
||
| 381 | * Notify all the feeds about the change. |
||
| 382 | * @param array $feeds Optional feeds to send to, defaults to $wgRCFeeds |
||
| 383 | */ |
||
| 384 | public function notifyRCFeeds( array $feeds = null ) { |
||
| 434 | |||
| 435 | /** |
||
| 436 | * Gets the stream engine object for a given URI from $wgRCEngines |
||
| 437 | * |
||
| 438 | * @param string $uri URI to get the engine object for |
||
| 439 | * @throws MWException |
||
| 440 | * @return RCFeedEngine The engine object |
||
| 441 | */ |
||
| 442 | public static function getEngine( $uri ) { |
||
| 456 | |||
| 457 | /** |
||
| 458 | * Mark a given change as patrolled |
||
| 459 | * |
||
| 460 | * @param RecentChange|int $change RecentChange or corresponding rc_id |
||
| 461 | * @param bool $auto For automatic patrol |
||
| 462 | * @param string|string[] $tags Change tags to add to the patrol log entry |
||
| 463 | * ($user should be able to add the specified tags before this is called) |
||
| 464 | * @return array See doMarkPatrolled(), or null if $change is not an existing rc_id |
||
| 465 | */ |
||
| 466 | public static function markPatrolled( $change, $auto = false, $tags = null ) { |
||
| 479 | |||
| 480 | /** |
||
| 481 | * Mark this RecentChange as patrolled |
||
| 482 | * |
||
| 483 | * NOTE: Can also return 'rcpatroldisabled', 'hookaborted' and |
||
| 484 | * 'markedaspatrollederror-noautopatrol' as errors |
||
| 485 | * @param User $user User object doing the action |
||
| 486 | * @param bool $auto For automatic patrol |
||
| 487 | * @param string|string[] $tags Change tags to add to the patrol log entry |
||
| 488 | * ($user should be able to add the specified tags before this is called) |
||
| 489 | * @return array Array of permissions errors, see Title::getUserPermissionsErrors() |
||
| 490 | */ |
||
| 491 | public function doMarkPatrolled( User $user, $auto = false, $tags = null ) { |
||
| 536 | |||
| 537 | /** |
||
| 538 | * Mark this RecentChange patrolled, without error checking |
||
| 539 | * @return int Number of affected rows |
||
| 540 | */ |
||
| 541 | public function reallyMarkPatrolled() { |
||
| 559 | |||
| 560 | /** |
||
| 561 | * Makes an entry in the database corresponding to an edit |
||
| 562 | * |
||
| 563 | * @param string $timestamp |
||
| 564 | * @param Title $title |
||
| 565 | * @param bool $minor |
||
| 566 | * @param User $user |
||
| 567 | * @param string $comment |
||
| 568 | * @param int $oldId |
||
| 569 | * @param string $lastTimestamp |
||
| 570 | * @param bool $bot |
||
| 571 | * @param string $ip |
||
| 572 | * @param int $oldSize |
||
| 573 | * @param int $newSize |
||
| 574 | * @param int $newId |
||
| 575 | * @param int $patrol |
||
| 576 | * @param array $tags |
||
| 577 | * @return RecentChange |
||
| 578 | */ |
||
| 579 | public static function notifyEdit( |
||
| 635 | |||
| 636 | /** |
||
| 637 | * Makes an entry in the database corresponding to page creation |
||
| 638 | * Note: the title object must be loaded with the new id using resetArticleID() |
||
| 639 | * |
||
| 640 | * @param string $timestamp |
||
| 641 | * @param Title $title |
||
| 642 | * @param bool $minor |
||
| 643 | * @param User $user |
||
| 644 | * @param string $comment |
||
| 645 | * @param bool $bot |
||
| 646 | * @param string $ip |
||
| 647 | * @param int $size |
||
| 648 | * @param int $newId |
||
| 649 | * @param int $patrol |
||
| 650 | * @param array $tags |
||
| 651 | * @return RecentChange |
||
| 652 | */ |
||
| 653 | public static function notifyNew( |
||
| 708 | |||
| 709 | /** |
||
| 710 | * @param string $timestamp |
||
| 711 | * @param Title $title |
||
| 712 | * @param User $user |
||
| 713 | * @param string $actionComment |
||
| 714 | * @param string $ip |
||
| 715 | * @param string $type |
||
| 716 | * @param string $action |
||
| 717 | * @param Title $target |
||
| 718 | * @param string $logComment |
||
| 719 | * @param string $params |
||
| 720 | * @param int $newId |
||
| 721 | * @param string $actionCommentIRC |
||
| 722 | * @return bool |
||
| 723 | */ |
||
| 724 | public static function notifyLog( $timestamp, &$title, &$user, $actionComment, $ip, $type, |
||
| 739 | |||
| 740 | /** |
||
| 741 | * @param string $timestamp |
||
| 742 | * @param Title $title |
||
| 743 | * @param User $user |
||
| 744 | * @param string $actionComment |
||
| 745 | * @param string $ip |
||
| 746 | * @param string $type |
||
| 747 | * @param string $action |
||
| 748 | * @param Title $target |
||
| 749 | * @param string $logComment |
||
| 750 | * @param string $params |
||
| 751 | * @param int $newId |
||
| 752 | * @param string $actionCommentIRC |
||
| 753 | * @param int $revId Id of associated revision, if any |
||
| 754 | * @param bool $isPatrollable Whether this log entry is patrollable |
||
| 755 | * @return RecentChange |
||
| 756 | */ |
||
| 757 | public static function newLogEntry( $timestamp, &$title, &$user, $actionComment, $ip, |
||
| 825 | |||
| 826 | /** |
||
| 827 | * Constructs a RecentChange object for the given categorization |
||
| 828 | * This does not call save() on the object and thus does not write to the db |
||
| 829 | * |
||
| 830 | * @since 1.27 |
||
| 831 | * |
||
| 832 | * @param string $timestamp Timestamp of the recent change to occur |
||
| 833 | * @param Title $categoryTitle Title of the category a page is being added to or removed from |
||
| 834 | * @param User $user User object of the user that made the change |
||
| 835 | * @param string $comment Change summary |
||
| 836 | * @param Title $pageTitle Title of the page that is being added or removed |
||
| 837 | * @param int $oldRevId Parent revision ID of this change |
||
| 838 | * @param int $newRevId Revision ID of this change |
||
| 839 | * @param string $lastTimestamp Parent revision timestamp of this change |
||
| 840 | * @param bool $bot true, if the change was made by a bot |
||
| 841 | * @param string $ip IP address of the user, if the change was made anonymously |
||
| 842 | * @param int $deleted Indicates whether the change has been deleted |
||
| 843 | * |
||
| 844 | * @return RecentChange |
||
| 845 | */ |
||
| 846 | public static function newForCategorization( |
||
| 900 | |||
| 901 | /** |
||
| 902 | * Get a parameter value |
||
| 903 | * |
||
| 904 | * @since 1.27 |
||
| 905 | * |
||
| 906 | * @param string $name parameter name |
||
| 907 | * @return mixed |
||
| 908 | */ |
||
| 909 | public function getParam( $name ) { |
||
| 913 | |||
| 914 | /** |
||
| 915 | * Initialises the members of this object from a mysql row object |
||
| 916 | * |
||
| 917 | * @param mixed $row |
||
| 918 | */ |
||
| 919 | public function loadFromRow( $row ) { |
||
| 924 | |||
| 925 | /** |
||
| 926 | * Get an attribute value |
||
| 927 | * |
||
| 928 | * @param string $name Attribute name |
||
| 929 | * @return mixed |
||
| 930 | */ |
||
| 931 | public function getAttribute( $name ) { |
||
| 934 | |||
| 935 | /** |
||
| 936 | * @return array |
||
| 937 | */ |
||
| 938 | public function getAttributes() { |
||
| 941 | |||
| 942 | /** |
||
| 943 | * Gets the end part of the diff URL associated with this object |
||
| 944 | * Blank if no diff link should be displayed |
||
| 945 | * @param bool $forceCur |
||
| 946 | * @return string |
||
| 947 | */ |
||
| 948 | public function diffLinkTrail( $forceCur ) { |
||
| 963 | |||
| 964 | /** |
||
| 965 | * Returns the change size (HTML). |
||
| 966 | * The lengths can be given optionally. |
||
| 967 | * @param int $old |
||
| 968 | * @param int $new |
||
| 969 | * @return string |
||
| 970 | */ |
||
| 971 | public function getCharacterDifference( $old = 0, $new = 0 ) { |
||
| 984 | |||
| 985 | private static function checkIPAddress( $ip ) { |
||
| 1001 | |||
| 1002 | /** |
||
| 1003 | * Check whether the given timestamp is new enough to have a RC row with a given tolerance |
||
| 1004 | * as the recentchanges table might not be cleared out regularly (so older entries might exist) |
||
| 1005 | * or rows which will be deleted soon shouldn't be included. |
||
| 1006 | * |
||
| 1007 | * @param mixed $timestamp MWTimestamp compatible timestamp |
||
| 1008 | * @param int $tolerance Tolerance in seconds |
||
| 1009 | * @return bool |
||
| 1010 | */ |
||
| 1011 | public static function isInRCLifespan( $timestamp, $tolerance = 0 ) { |
||
| 1016 | |||
| 1017 | /** |
||
| 1018 | * Parses and returns the rc_params attribute |
||
| 1019 | * |
||
| 1020 | * @since 1.26 |
||
| 1021 | * |
||
| 1022 | * @return mixed|bool false on failed unserialization |
||
| 1023 | */ |
||
| 1024 | public function parseParams() { |
||
| 1033 | |||
| 1034 | /** |
||
| 1035 | * Tags to append to the recent change, |
||
| 1036 | * and associated revision/log |
||
| 1037 | * |
||
| 1038 | * @since 1.28 |
||
| 1039 | * |
||
| 1040 | * @param string|array $tags |
||
| 1041 | */ |
||
| 1042 | public function addTags( $tags ) { |
||
| 1049 | } |
||
| 1050 |
Loading history...
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.