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 ApiQuery 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 ApiQuery, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 38 | class ApiQuery extends ApiBase { |
||
| 39 | |||
| 40 | /** |
||
| 41 | * List of Api Query prop modules |
||
| 42 | * @var array |
||
| 43 | */ |
||
| 44 | private static $QueryPropModules = [ |
||
| 45 | 'categories' => 'ApiQueryCategories', |
||
| 46 | 'categoryinfo' => 'ApiQueryCategoryInfo', |
||
| 47 | 'contributors' => 'ApiQueryContributors', |
||
| 48 | 'deletedrevisions' => 'ApiQueryDeletedRevisions', |
||
| 49 | 'duplicatefiles' => 'ApiQueryDuplicateFiles', |
||
| 50 | 'extlinks' => 'ApiQueryExternalLinks', |
||
| 51 | 'fileusage' => 'ApiQueryBacklinksprop', |
||
| 52 | 'images' => 'ApiQueryImages', |
||
| 53 | 'imageinfo' => 'ApiQueryImageInfo', |
||
| 54 | 'info' => 'ApiQueryInfo', |
||
| 55 | 'links' => 'ApiQueryLinks', |
||
| 56 | 'linkshere' => 'ApiQueryBacklinksprop', |
||
| 57 | 'iwlinks' => 'ApiQueryIWLinks', |
||
| 58 | 'langlinks' => 'ApiQueryLangLinks', |
||
| 59 | 'pageprops' => 'ApiQueryPageProps', |
||
| 60 | 'redirects' => 'ApiQueryBacklinksprop', |
||
| 61 | 'revisions' => 'ApiQueryRevisions', |
||
| 62 | 'stashimageinfo' => 'ApiQueryStashImageInfo', |
||
| 63 | 'templates' => 'ApiQueryLinks', |
||
| 64 | 'transcludedin' => 'ApiQueryBacklinksprop', |
||
| 65 | ]; |
||
| 66 | |||
| 67 | /** |
||
| 68 | * List of Api Query list modules |
||
| 69 | * @var array |
||
| 70 | */ |
||
| 71 | private static $QueryListModules = [ |
||
| 72 | 'allcategories' => 'ApiQueryAllCategories', |
||
| 73 | 'alldeletedrevisions' => 'ApiQueryAllDeletedRevisions', |
||
| 74 | 'allfileusages' => 'ApiQueryAllLinks', |
||
| 75 | 'allimages' => 'ApiQueryAllImages', |
||
| 76 | 'alllinks' => 'ApiQueryAllLinks', |
||
| 77 | 'allpages' => 'ApiQueryAllPages', |
||
| 78 | 'allredirects' => 'ApiQueryAllLinks', |
||
| 79 | 'allrevisions' => 'ApiQueryAllRevisions', |
||
| 80 | 'mystashedfiles' => 'ApiQueryMyStashedFiles', |
||
| 81 | 'alltransclusions' => 'ApiQueryAllLinks', |
||
| 82 | 'allusers' => 'ApiQueryAllUsers', |
||
| 83 | 'backlinks' => 'ApiQueryBacklinks', |
||
| 84 | 'blocks' => 'ApiQueryBlocks', |
||
| 85 | 'categorymembers' => 'ApiQueryCategoryMembers', |
||
| 86 | 'deletedrevs' => 'ApiQueryDeletedrevs', |
||
| 87 | 'embeddedin' => 'ApiQueryBacklinks', |
||
| 88 | 'exturlusage' => 'ApiQueryExtLinksUsage', |
||
| 89 | 'filearchive' => 'ApiQueryFilearchive', |
||
| 90 | 'imageusage' => 'ApiQueryBacklinks', |
||
| 91 | 'iwbacklinks' => 'ApiQueryIWBacklinks', |
||
| 92 | 'langbacklinks' => 'ApiQueryLangBacklinks', |
||
| 93 | 'logevents' => 'ApiQueryLogEvents', |
||
| 94 | 'pageswithprop' => 'ApiQueryPagesWithProp', |
||
| 95 | 'pagepropnames' => 'ApiQueryPagePropNames', |
||
| 96 | 'prefixsearch' => 'ApiQueryPrefixSearch', |
||
| 97 | 'protectedtitles' => 'ApiQueryProtectedTitles', |
||
| 98 | 'querypage' => 'ApiQueryQueryPage', |
||
| 99 | 'random' => 'ApiQueryRandom', |
||
| 100 | 'recentchanges' => 'ApiQueryRecentChanges', |
||
| 101 | 'search' => 'ApiQuerySearch', |
||
| 102 | 'tags' => 'ApiQueryTags', |
||
| 103 | 'usercontribs' => 'ApiQueryContributions', |
||
| 104 | 'users' => 'ApiQueryUsers', |
||
| 105 | 'watchlist' => 'ApiQueryWatchlist', |
||
| 106 | 'watchlistraw' => 'ApiQueryWatchlistRaw', |
||
| 107 | ]; |
||
| 108 | |||
| 109 | /** |
||
| 110 | * List of Api Query meta modules |
||
| 111 | * @var array |
||
| 112 | */ |
||
| 113 | private static $QueryMetaModules = [ |
||
| 114 | 'allmessages' => 'ApiQueryAllMessages', |
||
| 115 | 'authmanagerinfo' => 'ApiQueryAuthManagerInfo', |
||
| 116 | 'siteinfo' => 'ApiQuerySiteinfo', |
||
| 117 | 'userinfo' => 'ApiQueryUserInfo', |
||
| 118 | 'filerepoinfo' => 'ApiQueryFileRepoInfo', |
||
| 119 | 'tokens' => 'ApiQueryTokens', |
||
| 120 | ]; |
||
| 121 | |||
| 122 | /** |
||
| 123 | * @var ApiPageSet |
||
| 124 | */ |
||
| 125 | private $mPageSet; |
||
| 126 | |||
| 127 | private $mParams; |
||
| 128 | private $mNamedDB = []; |
||
| 129 | private $mModuleMgr; |
||
| 130 | |||
| 131 | /** |
||
| 132 | * @param ApiMain $main |
||
| 133 | * @param string $action |
||
| 134 | */ |
||
| 135 | public function __construct( ApiMain $main, $action ) { |
||
| 154 | |||
| 155 | /** |
||
| 156 | * Overrides to return this instance's module manager. |
||
| 157 | * @return ApiModuleManager |
||
| 158 | */ |
||
| 159 | public function getModuleManager() { |
||
| 162 | |||
| 163 | /** |
||
| 164 | * Get the query database connection with the given name. |
||
| 165 | * If no such connection has been requested before, it will be created. |
||
| 166 | * Subsequent calls with the same $name will return the same connection |
||
| 167 | * as the first, regardless of the values of $db and $groups |
||
| 168 | * @param string $name Name to assign to the database connection |
||
| 169 | * @param int $db One of the DB_* constants |
||
| 170 | * @param array $groups Query groups |
||
| 171 | * @return DatabaseBase |
||
| 172 | */ |
||
| 173 | public function getNamedDB( $name, $db, $groups ) { |
||
| 180 | |||
| 181 | /** |
||
| 182 | * Gets the set of pages the user has requested (or generated) |
||
| 183 | * @return ApiPageSet |
||
| 184 | */ |
||
| 185 | public function getPageSet() { |
||
| 188 | |||
| 189 | /** |
||
| 190 | * @return ApiFormatRaw|null |
||
| 191 | */ |
||
| 192 | public function getCustomPrinter() { |
||
| 203 | |||
| 204 | /** |
||
| 205 | * Query execution happens in the following steps: |
||
| 206 | * #1 Create a PageSet object with any pages requested by the user |
||
| 207 | * #2 If using a generator, execute it to get a new ApiPageSet object |
||
| 208 | * #3 Instantiate all requested modules. |
||
| 209 | * This way the PageSet object will know what shared data is required, |
||
| 210 | * and minimize DB calls. |
||
| 211 | * #4 Output all normalization and redirect resolution information |
||
| 212 | * #5 Execute all requested modules |
||
| 213 | */ |
||
| 214 | public function execute() { |
||
| 270 | |||
| 271 | /** |
||
| 272 | * Update a cache mode string, applying the cache mode of a new module to it. |
||
| 273 | * The cache mode may increase in the level of privacy, but public modules |
||
| 274 | * added to private data do not decrease the level of privacy. |
||
| 275 | * |
||
| 276 | * @param string $cacheMode |
||
| 277 | * @param string $modCacheMode |
||
| 278 | * @return string |
||
| 279 | */ |
||
| 280 | protected function mergeCacheMode( $cacheMode, $modCacheMode ) { |
||
| 293 | |||
| 294 | /** |
||
| 295 | * Create instances of all modules requested by the client |
||
| 296 | * @param array $modules To append instantiated modules to |
||
| 297 | * @param string $param Parameter name to read modules from |
||
| 298 | */ |
||
| 299 | private function instantiateModules( &$modules, $param ) { |
||
| 317 | |||
| 318 | /** |
||
| 319 | * Appends an element for each page in the current pageSet with the |
||
| 320 | * most general information (id, title), plus any title normalizations |
||
| 321 | * and missing or invalid title/pageids/revids. |
||
| 322 | */ |
||
| 323 | private function outputGeneralPageInfo() { |
||
| 427 | |||
| 428 | /** |
||
| 429 | * @param ApiPageSet $pageSet Pages to be exported |
||
| 430 | * @param ApiResult $result Result to output to |
||
| 431 | */ |
||
| 432 | private function doExport( $pageSet, $result ) { |
||
| 470 | |||
| 471 | public function getAllowedParams( $flags = 0 ) { |
||
| 500 | |||
| 501 | /** |
||
| 502 | * Override the parent to generate help messages for all available query modules. |
||
| 503 | * @deprecated since 1.25 |
||
| 504 | * @return string |
||
| 505 | */ |
||
| 506 | public function makeHelpMsg() { |
||
| 524 | |||
| 525 | /** |
||
| 526 | * For all modules of a given group, generate help messages and join them together |
||
| 527 | * @deprecated since 1.25 |
||
| 528 | * @param string $group Module group |
||
| 529 | * @return string |
||
| 530 | */ |
||
| 531 | private function makeHelpMsgHelper( $group ) { |
||
| 555 | |||
| 556 | public function isReadMode() { |
||
| 557 | // We need to make an exception for certain meta modules that should be |
||
| 558 | // accessible even without the 'read' right. Restrict the exception as |
||
| 559 | // much as possible: no other modules allowed, and no pageset |
||
| 560 | // parameters either. We do allow the 'rawcontinue' and 'indexpageids' |
||
| 561 | // parameters since frameworks might add these unconditionally and they |
||
| 562 | // can't expose anything here. |
||
| 563 | $this->mParams = $this->extractRequestParams(); |
||
| 564 | $params = array_filter( |
||
| 565 | array_diff_key( |
||
| 566 | $this->mParams + $this->getPageSet()->extractRequestParams(), |
||
| 567 | [ 'rawcontinue' => 1, 'indexpageids' => 1 ] |
||
| 568 | ) |
||
| 569 | ); |
||
| 570 | if ( array_keys( $params ) !== [ 'meta' ] ) { |
||
| 571 | return true; |
||
| 572 | } |
||
| 573 | |||
| 574 | // Ask each module if it requires read mode. Any true => this returns |
||
| 575 | // true. |
||
| 576 | $modules = []; |
||
| 577 | $this->instantiateModules( $modules, 'meta' ); |
||
| 578 | foreach ( $modules as $module ) { |
||
| 579 | if ( $module->isReadMode() ) { |
||
| 580 | return true; |
||
| 581 | } |
||
| 582 | } |
||
| 583 | |||
| 584 | return false; |
||
| 585 | } |
||
| 586 | |||
| 587 | protected function getExamplesMessages() { |
||
| 596 | |||
| 597 | public function getHelpUrls() { |
||
| 605 | } |
||
| 606 |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the parent class: