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 CategoryViewer 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 CategoryViewer, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
24 | class CategoryViewer extends ContextSource { |
||
25 | /** @var int */ |
||
26 | public $limit; |
||
27 | |||
28 | /** @var array */ |
||
29 | public $from; |
||
30 | |||
31 | /** @var array */ |
||
32 | public $until; |
||
33 | |||
34 | /** @var string[] */ |
||
35 | public $articles; |
||
36 | |||
37 | /** @var array */ |
||
38 | public $articles_start_char; |
||
39 | |||
40 | /** @var array */ |
||
41 | public $children; |
||
42 | |||
43 | /** @var array */ |
||
44 | public $children_start_char; |
||
45 | |||
46 | /** @var bool */ |
||
47 | public $showGallery; |
||
48 | |||
49 | /** @var array */ |
||
50 | public $imgsNoGallery_start_char; |
||
51 | |||
52 | /** @var array */ |
||
53 | public $imgsNoGallery; |
||
54 | |||
55 | /** @var array */ |
||
56 | public $nextPage; |
||
57 | |||
58 | /** @var array */ |
||
59 | protected $prevPage; |
||
60 | |||
61 | /** @var array */ |
||
62 | public $flip; |
||
63 | |||
64 | /** @var Title */ |
||
65 | public $title; |
||
66 | |||
67 | /** @var Collation */ |
||
68 | public $collation; |
||
69 | |||
70 | /** @var ImageGallery */ |
||
71 | public $gallery; |
||
72 | |||
73 | /** @var Category Category object for this page. */ |
||
74 | private $cat; |
||
75 | |||
76 | /** @var array The original query array, to be used in generating paging links. */ |
||
77 | private $query; |
||
78 | |||
79 | /** |
||
80 | * @since 1.19 $context is a second, required parameter |
||
81 | * @param Title $title |
||
82 | * @param IContextSource $context |
||
83 | * @param array $from An array with keys page, subcat, |
||
84 | * and file for offset of results of each section (since 1.17) |
||
85 | * @param array $until An array with 3 keys for until of each section (since 1.17) |
||
86 | * @param array $query |
||
87 | */ |
||
88 | function __construct( $title, IContextSource $context, $from = [], |
||
89 | $until = [], $query = [] |
||
90 | ) { |
||
91 | $this->title = $title; |
||
92 | $this->setContext( $context ); |
||
93 | $this->getOutput()->addModuleStyles( [ |
||
94 | 'mediawiki.action.view.categoryPage.styles' |
||
95 | ] ); |
||
96 | $this->from = $from; |
||
97 | $this->until = $until; |
||
98 | $this->limit = $context->getConfig()->get( 'CategoryPagingLimit' ); |
||
99 | $this->cat = Category::newFromTitle( $title ); |
||
|
|||
100 | $this->query = $query; |
||
101 | $this->collation = Collation::singleton(); |
||
102 | unset( $this->query['title'] ); |
||
103 | } |
||
104 | |||
105 | /** |
||
106 | * Format the category data list. |
||
107 | * |
||
108 | * @return string HTML output |
||
109 | */ |
||
110 | public function getHTML() { |
||
111 | |||
112 | $this->showGallery = $this->getConfig()->get( 'CategoryMagicGallery' ) |
||
113 | && !$this->getOutput()->mNoGallery; |
||
114 | |||
115 | $this->clearCategoryState(); |
||
116 | $this->doCategoryQuery(); |
||
117 | $this->finaliseCategoryState(); |
||
118 | |||
119 | $r = $this->getSubcategorySection() . |
||
120 | $this->getPagesSection() . |
||
121 | $this->getImageSection(); |
||
122 | |||
123 | if ( $r == '' ) { |
||
124 | // If there is no category content to display, only |
||
125 | // show the top part of the navigation links. |
||
126 | // @todo FIXME: Cannot be completely suppressed because it |
||
127 | // is unknown if 'until' or 'from' makes this |
||
128 | // give 0 results. |
||
129 | $r = $r . $this->getCategoryTop(); |
||
130 | } else { |
||
131 | $r = $this->getCategoryTop() . |
||
132 | $r . |
||
133 | $this->getCategoryBottom(); |
||
134 | } |
||
135 | |||
136 | // Give a proper message if category is empty |
||
137 | if ( $r == '' ) { |
||
138 | $r = $this->msg( 'category-empty' )->parseAsBlock(); |
||
139 | } |
||
140 | |||
141 | $lang = $this->getLanguage(); |
||
142 | $attribs = [ |
||
143 | 'class' => 'mw-category-generated', |
||
144 | 'lang' => $lang->getHtmlCode(), |
||
145 | 'dir' => $lang->getDir() |
||
146 | ]; |
||
147 | # put a div around the headings which are in the user language |
||
148 | $r = Html::openElement( 'div', $attribs ) . $r . '</div>'; |
||
149 | |||
150 | return $r; |
||
151 | } |
||
152 | |||
153 | function clearCategoryState() { |
||
154 | $this->articles = []; |
||
155 | $this->articles_start_char = []; |
||
156 | $this->children = []; |
||
157 | $this->children_start_char = []; |
||
158 | if ( $this->showGallery ) { |
||
159 | // Note that null for mode is taken to mean use default. |
||
160 | $mode = $this->getRequest()->getVal( 'gallerymode', null ); |
||
161 | try { |
||
162 | $this->gallery = ImageGalleryBase::factory( $mode, $this->getContext() ); |
||
163 | } catch ( Exception $e ) { |
||
164 | // User specified something invalid, fallback to default. |
||
165 | $this->gallery = ImageGalleryBase::factory( false, $this->getContext() ); |
||
166 | } |
||
167 | |||
168 | $this->gallery->setHideBadImages(); |
||
169 | } else { |
||
170 | $this->imgsNoGallery = []; |
||
171 | $this->imgsNoGallery_start_char = []; |
||
172 | } |
||
173 | } |
||
174 | |||
175 | /** |
||
176 | * Add a subcategory to the internal lists, using a Category object |
||
177 | * @param Category $cat |
||
178 | * @param string $sortkey |
||
179 | * @param int $pageLength |
||
180 | */ |
||
181 | function addSubcategoryObject( Category $cat, $sortkey, $pageLength ) { |
||
182 | // Subcategory; strip the 'Category' namespace from the link text. |
||
183 | $title = $cat->getTitle(); |
||
184 | |||
185 | $this->children[] = $this->generateLink( |
||
186 | 'subcat', |
||
187 | $title, |
||
188 | $title->isRedirect(), |
||
189 | htmlspecialchars( $title->getText() ) |
||
190 | ); |
||
191 | |||
192 | $this->children_start_char[] = |
||
193 | $this->getSubcategorySortChar( $cat->getTitle(), $sortkey ); |
||
194 | } |
||
195 | |||
196 | function generateLink( $type, Title $title, $isRedirect, $html = null ) { |
||
197 | $link = null; |
||
198 | Hooks::run( 'CategoryViewer::generateLink', [ $type, $title, $html, &$link ] ); |
||
199 | if ( $link === null ) { |
||
200 | $link = Linker::link( $title, $html ); |
||
201 | } |
||
202 | if ( $isRedirect ) { |
||
203 | $link = '<span class="redirect-in-category">' . $link . '</span>'; |
||
204 | } |
||
205 | |||
206 | return $link; |
||
207 | } |
||
208 | |||
209 | /** |
||
210 | * Get the character to be used for sorting subcategories. |
||
211 | * If there's a link from Category:A to Category:B, the sortkey of the resulting |
||
212 | * entry in the categorylinks table is Category:A, not A, which it SHOULD be. |
||
213 | * Workaround: If sortkey == "Category:".$title, than use $title for sorting, |
||
214 | * else use sortkey... |
||
215 | * |
||
216 | * @param Title $title |
||
217 | * @param string $sortkey The human-readable sortkey (before transforming to icu or whatever). |
||
218 | * @return string |
||
219 | */ |
||
220 | function getSubcategorySortChar( $title, $sortkey ) { |
||
221 | global $wgContLang; |
||
222 | |||
223 | if ( $title->getPrefixedText() == $sortkey ) { |
||
224 | $word = $title->getDBkey(); |
||
225 | } else { |
||
226 | $word = $sortkey; |
||
227 | } |
||
228 | |||
229 | $firstChar = $this->collation->getFirstLetter( $word ); |
||
230 | |||
231 | return $wgContLang->convert( $firstChar ); |
||
232 | } |
||
233 | |||
234 | /** |
||
235 | * Add a page in the image namespace |
||
236 | * @param Title $title |
||
237 | * @param string $sortkey |
||
238 | * @param int $pageLength |
||
239 | * @param bool $isRedirect |
||
240 | */ |
||
241 | function addImage( Title $title, $sortkey, $pageLength, $isRedirect = false ) { |
||
242 | global $wgContLang; |
||
243 | if ( $this->showGallery ) { |
||
244 | $flip = $this->flip['file']; |
||
245 | if ( $flip ) { |
||
246 | $this->gallery->insert( $title ); |
||
247 | } else { |
||
248 | $this->gallery->add( $title ); |
||
249 | } |
||
250 | } else { |
||
251 | $this->imgsNoGallery[] = $this->generateLink( 'image', $title, $isRedirect ); |
||
252 | |||
253 | $this->imgsNoGallery_start_char[] = $wgContLang->convert( |
||
254 | $this->collation->getFirstLetter( $sortkey ) ); |
||
255 | } |
||
256 | } |
||
257 | |||
258 | /** |
||
259 | * Add a miscellaneous page |
||
260 | * @param Title $title |
||
261 | * @param string $sortkey |
||
262 | * @param int $pageLength |
||
263 | * @param bool $isRedirect |
||
264 | */ |
||
265 | function addPage( $title, $sortkey, $pageLength, $isRedirect = false ) { |
||
266 | global $wgContLang; |
||
267 | |||
268 | $this->articles[] = $this->generateLink( 'page', $title, $isRedirect ); |
||
269 | |||
270 | $this->articles_start_char[] = $wgContLang->convert( |
||
271 | $this->collation->getFirstLetter( $sortkey ) ); |
||
272 | } |
||
273 | |||
274 | function finaliseCategoryState() { |
||
275 | if ( $this->flip['subcat'] ) { |
||
276 | $this->children = array_reverse( $this->children ); |
||
277 | $this->children_start_char = array_reverse( $this->children_start_char ); |
||
278 | } |
||
279 | if ( $this->flip['page'] ) { |
||
280 | $this->articles = array_reverse( $this->articles ); |
||
281 | $this->articles_start_char = array_reverse( $this->articles_start_char ); |
||
282 | } |
||
283 | if ( !$this->showGallery && $this->flip['file'] ) { |
||
284 | $this->imgsNoGallery = array_reverse( $this->imgsNoGallery ); |
||
285 | $this->imgsNoGallery_start_char = array_reverse( $this->imgsNoGallery_start_char ); |
||
286 | } |
||
287 | } |
||
288 | |||
289 | function doCategoryQuery() { |
||
290 | $dbr = wfGetDB( DB_REPLICA, 'category' ); |
||
291 | |||
292 | $this->nextPage = [ |
||
293 | 'page' => null, |
||
294 | 'subcat' => null, |
||
295 | 'file' => null, |
||
296 | ]; |
||
297 | $this->prevPage = [ |
||
298 | 'page' => null, |
||
299 | 'subcat' => null, |
||
300 | 'file' => null, |
||
301 | ]; |
||
302 | |||
303 | $this->flip = [ 'page' => false, 'subcat' => false, 'file' => false ]; |
||
304 | |||
305 | foreach ( [ 'page', 'subcat', 'file' ] as $type ) { |
||
306 | # Get the sortkeys for start/end, if applicable. Note that if |
||
307 | # the collation in the database differs from the one |
||
308 | # set in $wgCategoryCollation, pagination might go totally haywire. |
||
309 | $extraConds = [ 'cl_type' => $type ]; |
||
310 | if ( isset( $this->from[$type] ) && $this->from[$type] !== null ) { |
||
311 | $extraConds[] = 'cl_sortkey >= ' |
||
312 | . $dbr->addQuotes( $this->collation->getSortKey( $this->from[$type] ) ); |
||
313 | } elseif ( isset( $this->until[$type] ) && $this->until[$type] !== null ) { |
||
314 | $extraConds[] = 'cl_sortkey < ' |
||
315 | . $dbr->addQuotes( $this->collation->getSortKey( $this->until[$type] ) ); |
||
316 | $this->flip[$type] = true; |
||
317 | } |
||
318 | |||
319 | $res = $dbr->select( |
||
320 | [ 'page', 'categorylinks', 'category' ], |
||
321 | array_merge( |
||
322 | LinkCache::getSelectFields(), |
||
323 | [ |
||
324 | 'page_namespace', |
||
325 | 'page_title', |
||
326 | 'cl_sortkey', |
||
327 | 'cat_id', |
||
328 | 'cat_title', |
||
329 | 'cat_subcats', |
||
330 | 'cat_pages', |
||
331 | 'cat_files', |
||
332 | 'cl_sortkey_prefix', |
||
333 | 'cl_collation' |
||
334 | ] |
||
335 | ), |
||
336 | array_merge( [ 'cl_to' => $this->title->getDBkey() ], $extraConds ), |
||
337 | __METHOD__, |
||
338 | [ |
||
339 | 'USE INDEX' => [ 'categorylinks' => 'cl_sortkey' ], |
||
340 | 'LIMIT' => $this->limit + 1, |
||
341 | 'ORDER BY' => $this->flip[$type] ? 'cl_sortkey DESC' : 'cl_sortkey', |
||
342 | ], |
||
343 | [ |
||
344 | 'categorylinks' => [ 'INNER JOIN', 'cl_from = page_id' ], |
||
345 | 'category' => [ 'LEFT JOIN', [ |
||
346 | 'cat_title = page_title', |
||
347 | 'page_namespace' => NS_CATEGORY |
||
348 | ] ] |
||
349 | ] |
||
350 | ); |
||
351 | |||
352 | Hooks::run( 'CategoryViewer::doCategoryQuery', [ $type, $res ] ); |
||
353 | $linkCache = MediaWikiServices::getInstance()->getLinkCache(); |
||
354 | |||
355 | $count = 0; |
||
356 | foreach ( $res as $row ) { |
||
357 | $title = Title::newFromRow( $row ); |
||
358 | $linkCache->addGoodLinkObjFromRow( $title, $row ); |
||
359 | |||
360 | if ( $row->cl_collation === '' ) { |
||
361 | // Hack to make sure that while updating from 1.16 schema |
||
362 | // and db is inconsistent, that the sky doesn't fall. |
||
363 | // See r83544. Could perhaps be removed in a couple decades... |
||
364 | $humanSortkey = $row->cl_sortkey; |
||
365 | } else { |
||
366 | $humanSortkey = $title->getCategorySortkey( $row->cl_sortkey_prefix ); |
||
367 | } |
||
368 | |||
369 | if ( ++$count > $this->limit ) { |
||
370 | # We've reached the one extra which shows that there |
||
371 | # are additional pages to be had. Stop here... |
||
372 | $this->nextPage[$type] = $humanSortkey; |
||
373 | break; |
||
374 | } |
||
375 | if ( $count == $this->limit ) { |
||
376 | $this->prevPage[$type] = $humanSortkey; |
||
377 | } |
||
378 | |||
379 | if ( $title->getNamespace() == NS_CATEGORY ) { |
||
380 | $cat = Category::newFromRow( $row, $title ); |
||
381 | $this->addSubcategoryObject( $cat, $humanSortkey, $row->page_len ); |
||
382 | } elseif ( $title->getNamespace() == NS_FILE ) { |
||
383 | $this->addImage( $title, $humanSortkey, $row->page_len, $row->page_is_redirect ); |
||
384 | } else { |
||
385 | $this->addPage( $title, $humanSortkey, $row->page_len, $row->page_is_redirect ); |
||
386 | } |
||
387 | } |
||
388 | } |
||
389 | } |
||
390 | |||
391 | /** |
||
392 | * @return string |
||
393 | */ |
||
394 | function getCategoryTop() { |
||
395 | $r = $this->getCategoryBottom(); |
||
396 | return $r === '' |
||
397 | ? $r |
||
398 | : "<br style=\"clear:both;\"/>\n" . $r; |
||
399 | } |
||
400 | |||
401 | /** |
||
402 | * @return string |
||
403 | */ |
||
404 | function getSubcategorySection() { |
||
405 | # Don't show subcategories section if there are none. |
||
406 | $r = ''; |
||
407 | $rescnt = count( $this->children ); |
||
408 | $dbcnt = $this->cat->getSubcatCount(); |
||
409 | // This function should be called even if the result isn't used, it has side-effects |
||
410 | $countmsg = $this->getCountMessage( $rescnt, $dbcnt, 'subcat' ); |
||
411 | |||
412 | View Code Duplication | if ( $rescnt > 0 ) { |
|
413 | # Showing subcategories |
||
414 | $r .= "<div id=\"mw-subcategories\">\n"; |
||
415 | $r .= '<h2>' . $this->msg( 'subcategories' )->parse() . "</h2>\n"; |
||
416 | $r .= $countmsg; |
||
417 | $r .= $this->getSectionPagingLinks( 'subcat' ); |
||
418 | $r .= $this->formatList( $this->children, $this->children_start_char ); |
||
419 | $r .= $this->getSectionPagingLinks( 'subcat' ); |
||
420 | $r .= "\n</div>"; |
||
421 | } |
||
422 | return $r; |
||
423 | } |
||
424 | |||
425 | /** |
||
426 | * @return string |
||
427 | */ |
||
428 | function getPagesSection() { |
||
454 | |||
455 | /** |
||
456 | * @return string |
||
457 | */ |
||
458 | function getImageSection() { |
||
459 | $r = ''; |
||
460 | $rescnt = $this->showGallery ? $this->gallery->count() : count( $this->imgsNoGallery ); |
||
461 | $dbcnt = $this->cat->getFileCount(); |
||
462 | // This function should be called even if the result isn't used, it has side-effects |
||
463 | $countmsg = $this->getCountMessage( $rescnt, $dbcnt, 'file' ); |
||
464 | |||
465 | if ( $rescnt > 0 ) { |
||
466 | $r .= "<div id=\"mw-category-media\">\n"; |
||
485 | |||
486 | /** |
||
487 | * Get the paging links for a section (subcats/pages/files), to go at the top and bottom |
||
488 | * of the output. |
||
489 | * |
||
490 | * @param string $type 'page', 'subcat', or 'file' |
||
491 | * @return string HTML output, possibly empty if there are no other pages |
||
492 | */ |
||
493 | private function getSectionPagingLinks( $type ) { |
||
514 | |||
515 | /** |
||
516 | * @return string |
||
517 | */ |
||
518 | function getCategoryBottom() { |
||
521 | |||
522 | /** |
||
523 | * Format a list of articles chunked by letter, either as a |
||
524 | * bullet list or a columnar format, depending on the length. |
||
525 | * |
||
526 | * @param array $articles |
||
527 | * @param array $articles_start_char |
||
528 | * @param int $cutoff |
||
529 | * @return string |
||
530 | * @private |
||
531 | */ |
||
532 | function formatList( $articles, $articles_start_char, $cutoff = 6 ) { |
||
548 | |||
549 | /** |
||
550 | * Format a list of articles chunked by letter in a three-column list, ordered |
||
551 | * vertically. This is used for categories with a significant number of pages. |
||
552 | * |
||
553 | * TODO: Take the headers into account when creating columns, so they're |
||
554 | * more visually equal. |
||
555 | * |
||
556 | * TODO: shortList and columnList are similar, need merging |
||
557 | * |
||
558 | * @param string[] $articles HTML links to each article |
||
559 | * @param string[] $articles_start_char The header characters for each article |
||
560 | * @return string HTML to output |
||
561 | * @private |
||
562 | */ |
||
563 | static function columnList( $articles, $articles_start_char ) { |
||
595 | |||
596 | /** |
||
597 | * Format a list of articles chunked by letter in a bullet list. This is used |
||
598 | * for categories with a small number of pages (when columns aren't needed). |
||
599 | * @param string[] $articles HTML links to each article |
||
600 | * @param string[] $articles_start_char The header characters for each article |
||
601 | * @return string HTML to output |
||
602 | * @private |
||
603 | */ |
||
604 | static function shortList( $articles, $articles_start_char ) { |
||
618 | |||
619 | /** |
||
620 | * Create paging links, as a helper method to getSectionPagingLinks(). |
||
621 | * |
||
622 | * @param string $first The 'until' parameter for the generated URL |
||
623 | * @param string $last The 'from' parameter for the generated URL |
||
624 | * @param string $type A prefix for parameters, 'page' or 'subcat' or |
||
625 | * 'file' |
||
626 | * @return string HTML |
||
627 | */ |
||
628 | private function pagingLinks( $first, $last, $type = '' ) { |
||
659 | |||
660 | /** |
||
661 | * Takes a title, and adds the fragment identifier that |
||
662 | * corresponds to the correct segment of the category. |
||
663 | * |
||
664 | * @param Title $title The title (usually $this->title) |
||
665 | * @param string $section Which section |
||
666 | * @throws MWException |
||
667 | * @return Title |
||
668 | */ |
||
669 | private function addFragmentToTitle( $title, $section ) { |
||
688 | |||
689 | /** |
||
690 | * What to do if the category table conflicts with the number of results |
||
691 | * returned? This function says what. Each type is considered independently |
||
692 | * of the other types. |
||
693 | * |
||
694 | * @param int $rescnt The number of items returned by our database query. |
||
695 | * @param int $dbcnt The number of items according to the category table. |
||
696 | * @param string $type 'subcat', 'article', or 'file' |
||
697 | * @return string A message giving the number of items, to output to HTML. |
||
698 | */ |
||
699 | private function getCountMessage( $rescnt, $dbcnt, $type ) { |
||
751 | } |
||
752 |
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
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. 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.