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 ArchivedFile 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 ArchivedFile, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
29 | class ArchivedFile { |
||
30 | /** @var int Filearchive row ID */ |
||
31 | private $id; |
||
32 | |||
33 | /** @var string File name */ |
||
34 | private $name; |
||
35 | |||
36 | /** @var string FileStore storage group */ |
||
37 | private $group; |
||
38 | |||
39 | /** @var string FileStore SHA-1 key */ |
||
40 | private $key; |
||
41 | |||
42 | /** @var int File size in bytes */ |
||
43 | private $size; |
||
44 | |||
45 | /** @var int Size in bytes */ |
||
46 | private $bits; |
||
47 | |||
48 | /** @var int Width */ |
||
49 | private $width; |
||
50 | |||
51 | /** @var int Height */ |
||
52 | private $height; |
||
53 | |||
54 | /** @var string Metadata string */ |
||
55 | private $metadata; |
||
56 | |||
57 | /** @var string MIME type */ |
||
58 | private $mime; |
||
59 | |||
60 | /** @var string Media type */ |
||
61 | private $media_type; |
||
62 | |||
63 | /** @var string Upload description */ |
||
64 | private $description; |
||
65 | |||
66 | /** @var int User ID of uploader */ |
||
67 | private $user; |
||
68 | |||
69 | /** @var string User name of uploader */ |
||
70 | private $user_text; |
||
71 | |||
72 | /** @var string Time of upload */ |
||
73 | private $timestamp; |
||
74 | |||
75 | /** @var bool Whether or not all this has been loaded from the database (loadFromXxx) */ |
||
76 | private $dataLoaded; |
||
77 | |||
78 | /** @var int Bitfield akin to rev_deleted */ |
||
79 | private $deleted; |
||
80 | |||
81 | /** @var string SHA-1 hash of file content */ |
||
82 | private $sha1; |
||
83 | |||
84 | /** @var string Number of pages of a multipage document, or false for |
||
85 | * documents which aren't multipage documents |
||
86 | */ |
||
87 | private $pageCount; |
||
88 | |||
89 | /** @var string Original base filename */ |
||
90 | private $archive_name; |
||
91 | |||
92 | /** @var MediaHandler */ |
||
93 | protected $handler; |
||
94 | |||
95 | /** @var Title */ |
||
96 | protected $title; # image title |
||
97 | |||
98 | /** |
||
99 | * @throws MWException |
||
100 | * @param Title $title |
||
101 | * @param int $id |
||
102 | * @param string $key |
||
103 | * @param string $sha1 |
||
104 | */ |
||
105 | function __construct( $title, $id = 0, $key = '', $sha1 = '' ) { |
||
148 | |||
149 | /** |
||
150 | * Loads a file object from the filearchive table |
||
151 | * @throws MWException |
||
152 | * @return bool|null True on success or null |
||
153 | */ |
||
154 | public function load() { |
||
202 | |||
203 | /** |
||
204 | * Loads a file object from the filearchive table |
||
205 | * |
||
206 | * @param stdClass $row |
||
207 | * @return ArchivedFile |
||
208 | */ |
||
209 | public static function newFromRow( $row ) { |
||
215 | |||
216 | /** |
||
217 | * Fields in the filearchive table |
||
218 | * @return array |
||
219 | */ |
||
220 | static function selectFields() { |
||
244 | |||
245 | /** |
||
246 | * Load ArchivedFile object fields from a DB row. |
||
247 | * |
||
248 | * @param stdClass $row Object database row |
||
249 | * @since 1.21 |
||
250 | */ |
||
251 | public function loadFromRow( $row ) { |
||
279 | |||
280 | /** |
||
281 | * Return the associated title object |
||
282 | * |
||
283 | * @return Title |
||
284 | */ |
||
285 | public function getTitle() { |
||
291 | |||
292 | /** |
||
293 | * Return the file name |
||
294 | * |
||
295 | * @return string |
||
296 | */ |
||
297 | public function getName() { |
||
304 | |||
305 | /** |
||
306 | * @return int |
||
307 | */ |
||
308 | public function getID() { |
||
313 | |||
314 | /** |
||
315 | * @return bool |
||
316 | */ |
||
317 | public function exists() { |
||
322 | |||
323 | /** |
||
324 | * Return the FileStore key |
||
325 | * @return string |
||
326 | */ |
||
327 | public function getKey() { |
||
332 | |||
333 | /** |
||
334 | * Return the FileStore key (overriding base File class) |
||
335 | * @return string |
||
336 | */ |
||
337 | public function getStorageKey() { |
||
340 | |||
341 | /** |
||
342 | * Return the FileStore storage group |
||
343 | * @return string |
||
344 | */ |
||
345 | public function getGroup() { |
||
348 | |||
349 | /** |
||
350 | * Return the width of the image |
||
351 | * @return int |
||
352 | */ |
||
353 | public function getWidth() { |
||
358 | |||
359 | /** |
||
360 | * Return the height of the image |
||
361 | * @return int |
||
362 | */ |
||
363 | public function getHeight() { |
||
368 | |||
369 | /** |
||
370 | * Get handler-specific metadata |
||
371 | * @return string |
||
372 | */ |
||
373 | public function getMetadata() { |
||
378 | |||
379 | /** |
||
380 | * Return the size of the image file, in bytes |
||
381 | * @return int |
||
382 | */ |
||
383 | public function getSize() { |
||
388 | |||
389 | /** |
||
390 | * Return the bits of the image file, in bytes |
||
391 | * @return int |
||
392 | */ |
||
393 | public function getBits() { |
||
398 | |||
399 | /** |
||
400 | * Returns the MIME type of the file. |
||
401 | * @return string |
||
402 | */ |
||
403 | public function getMimeType() { |
||
408 | |||
409 | /** |
||
410 | * Get a MediaHandler instance for this file |
||
411 | * @return MediaHandler |
||
412 | */ |
||
413 | function getHandler() { |
||
420 | |||
421 | /** |
||
422 | * Returns the number of pages of a multipage document, or false for |
||
423 | * documents which aren't multipage documents |
||
424 | * @return bool|int |
||
425 | */ |
||
426 | View Code Duplication | function pageCount() { |
|
438 | |||
439 | /** |
||
440 | * Return the type of the media in the file. |
||
441 | * Use the value returned by this function with the MEDIATYPE_xxx constants. |
||
442 | * @return string |
||
443 | */ |
||
444 | public function getMediaType() { |
||
449 | |||
450 | /** |
||
451 | * Return upload timestamp. |
||
452 | * |
||
453 | * @return string |
||
454 | */ |
||
455 | public function getTimestamp() { |
||
460 | |||
461 | /** |
||
462 | * Get the SHA-1 base 36 hash of the file |
||
463 | * |
||
464 | * @return string |
||
465 | * @since 1.21 |
||
466 | */ |
||
467 | function getSha1() { |
||
472 | |||
473 | /** |
||
474 | * Returns ID or name of user who uploaded the file |
||
475 | * |
||
476 | * @note Prior to MediaWiki 1.23, this method always |
||
477 | * returned the user id, and was inconsistent with |
||
478 | * the rest of the file classes. |
||
479 | * @param string $type 'text' or 'id' |
||
480 | * @return int|string |
||
481 | * @throws MWException |
||
482 | */ |
||
483 | public function getUser( $type = 'text' ) { |
||
494 | |||
495 | /** |
||
496 | * Return the user name of the uploader. |
||
497 | * |
||
498 | * @deprecated since 1.23 Use getUser( 'text' ) instead. |
||
499 | * @return string |
||
500 | */ |
||
501 | public function getUserText() { |
||
510 | |||
511 | /** |
||
512 | * Return upload description. |
||
513 | * |
||
514 | * @return string |
||
515 | */ |
||
516 | public function getDescription() { |
||
524 | |||
525 | /** |
||
526 | * Return the user ID of the uploader. |
||
527 | * |
||
528 | * @return int |
||
529 | */ |
||
530 | public function getRawUser() { |
||
535 | |||
536 | /** |
||
537 | * Return the user name of the uploader. |
||
538 | * |
||
539 | * @return string |
||
540 | */ |
||
541 | public function getRawUserText() { |
||
546 | |||
547 | /** |
||
548 | * Return upload description. |
||
549 | * |
||
550 | * @return string |
||
551 | */ |
||
552 | public function getRawDescription() { |
||
557 | |||
558 | /** |
||
559 | * Returns the deletion bitfield |
||
560 | * @return int |
||
561 | */ |
||
562 | public function getVisibility() { |
||
567 | |||
568 | /** |
||
569 | * for file or revision rows |
||
570 | * |
||
571 | * @param int $field One of DELETED_* bitfield constants |
||
572 | * @return bool |
||
573 | */ |
||
574 | public function isDeleted( $field ) { |
||
579 | |||
580 | /** |
||
581 | * Determine if the current user is allowed to view a particular |
||
582 | * field of this FileStore image file, if it's marked as deleted. |
||
583 | * @param int $field |
||
584 | * @param null|User $user User object to check, or null to use $wgUser |
||
585 | * @return bool |
||
586 | */ |
||
587 | public function userCan( $field, User $user = null ) { |
||
593 | } |
||
594 |
Our type inference engine has found an assignment to a property that is incompatible with the declared type of that property.
Either this assignment is in error or the assigned type should be added to the documentation/type hint for that property..