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 LogPage 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 LogPage, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 32 | class LogPage { |
||
| 33 | const DELETED_ACTION = 1; |
||
| 34 | const DELETED_COMMENT = 2; |
||
| 35 | const DELETED_USER = 4; |
||
| 36 | const DELETED_RESTRICTED = 8; |
||
| 37 | |||
| 38 | // Convenience fields |
||
| 39 | const SUPPRESSED_USER = 12; |
||
| 40 | const SUPPRESSED_ACTION = 9; |
||
| 41 | |||
| 42 | /** @var bool */ |
||
| 43 | public $updateRecentChanges; |
||
| 44 | |||
| 45 | /** @var bool */ |
||
| 46 | public $sendToUDP; |
||
| 47 | |||
| 48 | /** @var string Plaintext version of the message for IRC */ |
||
| 49 | private $ircActionText; |
||
| 50 | |||
| 51 | /** @var string Plaintext version of the message */ |
||
| 52 | private $actionText; |
||
| 53 | |||
| 54 | /** @var string One of '', 'block', 'protect', 'rights', 'delete', |
||
| 55 | * 'upload', 'move' |
||
| 56 | */ |
||
| 57 | private $type; |
||
| 58 | |||
| 59 | /** @var string One of '', 'block', 'protect', 'rights', 'delete', |
||
| 60 | * 'upload', 'move', 'move_redir' */ |
||
| 61 | private $action; |
||
| 62 | |||
| 63 | /** @var string Comment associated with action */ |
||
| 64 | private $comment; |
||
| 65 | |||
| 66 | /** @var string Blob made of a parameters array */ |
||
| 67 | private $params; |
||
| 68 | |||
| 69 | /** @var User The user doing the action */ |
||
| 70 | private $doer; |
||
| 71 | |||
| 72 | /** @var Title */ |
||
| 73 | private $target; |
||
| 74 | |||
| 75 | /** |
||
| 76 | * Constructor |
||
| 77 | * |
||
| 78 | * @param string $type One of '', 'block', 'protect', 'rights', 'delete', |
||
| 79 | * 'upload', 'move' |
||
| 80 | * @param bool $rc Whether to update recent changes as well as the logging table |
||
| 81 | * @param string $udp Pass 'UDP' to send to the UDP feed if NOT sent to RC |
||
| 82 | */ |
||
| 83 | public function __construct( $type, $rc = true, $udp = 'skipUDP' ) { |
||
| 88 | |||
| 89 | /** |
||
| 90 | * @return int The log_id of the inserted log entry |
||
| 91 | */ |
||
| 92 | protected function saveContent() { |
||
| 144 | |||
| 145 | /** |
||
| 146 | * Get the RC comment from the last addEntry() call |
||
| 147 | * |
||
| 148 | * @return string |
||
| 149 | */ |
||
| 150 | View Code Duplication | public function getRcComment() { |
|
| 164 | |||
| 165 | /** |
||
| 166 | * Get the RC comment from the last addEntry() call for IRC |
||
| 167 | * |
||
| 168 | * @return string |
||
| 169 | */ |
||
| 170 | View Code Duplication | public function getRcCommentIRC() { |
|
| 184 | |||
| 185 | /** |
||
| 186 | * Get the comment from the last addEntry() call |
||
| 187 | * @return string |
||
| 188 | */ |
||
| 189 | public function getComment() { |
||
| 192 | |||
| 193 | /** |
||
| 194 | * Get the list of valid log types |
||
| 195 | * |
||
| 196 | * @return array Array of strings |
||
| 197 | */ |
||
| 198 | public static function validTypes() { |
||
| 203 | |||
| 204 | /** |
||
| 205 | * Is $type a valid log type |
||
| 206 | * |
||
| 207 | * @param string $type Log type to check |
||
| 208 | * @return bool |
||
| 209 | */ |
||
| 210 | public static function isLogType( $type ) { |
||
| 213 | |||
| 214 | /** |
||
| 215 | * Generate text for a log entry. |
||
| 216 | * Only LogFormatter should call this function. |
||
| 217 | * |
||
| 218 | * @param string $type Log type |
||
| 219 | * @param string $action Log action |
||
| 220 | * @param Title|null $title Title object or null |
||
| 221 | * @param Skin|null $skin Skin object or null. If null, we want to use the wiki |
||
| 222 | * content language, since that will go to the IRC feed. |
||
| 223 | * @param array $params Parameters |
||
| 224 | * @param bool $filterWikilinks Whether to filter wiki links |
||
| 225 | * @return string HTML |
||
| 226 | */ |
||
| 227 | public static function actionText( $type, $action, $title = null, $skin = null, |
||
| 287 | |||
| 288 | /** |
||
| 289 | * @todo Document |
||
| 290 | * @param string $type |
||
| 291 | * @param Language|null $lang |
||
| 292 | * @param Title $title |
||
| 293 | * @param array $params |
||
| 294 | * @return string |
||
| 295 | */ |
||
| 296 | protected static function getTitleLink( $type, $lang, $title, &$params ) { |
||
| 321 | |||
| 322 | /** |
||
| 323 | * Add a log entry |
||
| 324 | * |
||
| 325 | * @param string $action One of '', 'block', 'protect', 'rights', 'delete', |
||
| 326 | * 'upload', 'move', 'move_redir' |
||
| 327 | * @param Title $target Title object |
||
| 328 | * @param string $comment Description associated |
||
| 329 | * @param array $params Parameters passed later to wfMessage function |
||
| 330 | * @param null|int|User $doer The user doing the action. null for $wgUser |
||
| 331 | * |
||
| 332 | * @return int The log_id of the inserted log entry |
||
| 333 | */ |
||
| 334 | public function addEntry( $action, $target, $comment, $params = [], $doer = null ) { |
||
| 383 | |||
| 384 | /** |
||
| 385 | * Add relations to log_search table |
||
| 386 | * |
||
| 387 | * @param string $field |
||
| 388 | * @param array $values |
||
| 389 | * @param int $logid |
||
| 390 | * @return bool |
||
| 391 | */ |
||
| 392 | public function addRelations( $field, $values, $logid ) { |
||
| 412 | |||
| 413 | /** |
||
| 414 | * Create a blob from a parameter array |
||
| 415 | * |
||
| 416 | * @param array $params |
||
| 417 | * @return string |
||
| 418 | */ |
||
| 419 | public static function makeParamBlob( $params ) { |
||
| 422 | |||
| 423 | /** |
||
| 424 | * Extract a parameter array from a blob |
||
| 425 | * |
||
| 426 | * @param string $blob |
||
| 427 | * @return array |
||
| 428 | */ |
||
| 429 | public static function extractParams( $blob ) { |
||
| 436 | |||
| 437 | /** |
||
| 438 | * Name of the log. |
||
| 439 | * @return Message |
||
| 440 | * @since 1.19 |
||
| 441 | */ |
||
| 442 | public function getName() { |
||
| 454 | |||
| 455 | /** |
||
| 456 | * Description of this log type. |
||
| 457 | * @return Message |
||
| 458 | * @since 1.19 |
||
| 459 | */ |
||
| 460 | public function getDescription() { |
||
| 471 | |||
| 472 | /** |
||
| 473 | * Returns the right needed to read this log type. |
||
| 474 | * @return string |
||
| 475 | * @since 1.19 |
||
| 476 | */ |
||
| 477 | public function getRestriction() { |
||
| 488 | |||
| 489 | /** |
||
| 490 | * Tells if this log is not viewable by all. |
||
| 491 | * @return bool |
||
| 492 | * @since 1.19 |
||
| 493 | */ |
||
| 494 | public function isRestricted() { |
||
| 499 | } |
||
| 500 |
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: