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 QueryPage 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 QueryPage, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 30 | abstract class QueryPage extends SpecialPage { |
||
| 31 | /** @var bool Whether or not we want plain listoutput rather than an ordered list */ |
||
| 32 | protected $listoutput = false; |
||
| 33 | |||
| 34 | /** @var int The offset and limit in use, as passed to the query() function */ |
||
| 35 | protected $offset = 0; |
||
| 36 | |||
| 37 | /** @var int */ |
||
| 38 | protected $limit = 0; |
||
| 39 | |||
| 40 | /** |
||
| 41 | * The number of rows returned by the query. Reading this variable |
||
| 42 | * only makes sense in functions that are run after the query has been |
||
| 43 | * done, such as preprocessResults() and formatRow(). |
||
| 44 | */ |
||
| 45 | protected $numRows; |
||
| 46 | |||
| 47 | protected $cachedTimestamp = null; |
||
| 48 | |||
| 49 | /** |
||
| 50 | * Whether to show prev/next links |
||
| 51 | */ |
||
| 52 | protected $shownavigation = true; |
||
| 53 | |||
| 54 | /** |
||
| 55 | * Get a list of query page classes and their associated special pages, |
||
| 56 | * for periodic updates. |
||
| 57 | * |
||
| 58 | * DO NOT CHANGE THIS LIST without testing that |
||
| 59 | * maintenance/updateSpecialPages.php still works. |
||
| 60 | * @return array |
||
| 61 | */ |
||
| 62 | public static function getPages() { |
||
| 108 | |||
| 109 | /** |
||
| 110 | * A mutator for $this->listoutput; |
||
| 111 | * |
||
| 112 | * @param bool $bool |
||
| 113 | */ |
||
| 114 | function setListoutput( $bool ) { |
||
| 117 | |||
| 118 | /** |
||
| 119 | * Subclasses return an SQL query here, formatted as an array with the |
||
| 120 | * following keys: |
||
| 121 | * tables => Table(s) for passing to Database::select() |
||
| 122 | * fields => Field(s) for passing to Database::select(), may be * |
||
| 123 | * conds => WHERE conditions |
||
| 124 | * options => options |
||
| 125 | * join_conds => JOIN conditions |
||
| 126 | * |
||
| 127 | * Note that the query itself should return the following three columns: |
||
| 128 | * 'namespace', 'title', and 'value'. 'value' is used for sorting. |
||
| 129 | * |
||
| 130 | * These may be stored in the querycache table for expensive queries, |
||
| 131 | * and that cached data will be returned sometimes, so the presence of |
||
| 132 | * extra fields can't be relied upon. The cached 'value' column will be |
||
| 133 | * an integer; non-numeric values are useful only for sorting the |
||
| 134 | * initial query (except if they're timestamps, see usesTimestamps()). |
||
| 135 | * |
||
| 136 | * Don't include an ORDER or LIMIT clause, they will be added. |
||
| 137 | * |
||
| 138 | * If this function is not overridden or returns something other than |
||
| 139 | * an array, getSQL() will be used instead. This is for backwards |
||
| 140 | * compatibility only and is strongly deprecated. |
||
| 141 | * @return array |
||
| 142 | * @since 1.18 |
||
| 143 | */ |
||
| 144 | public function getQueryInfo() { |
||
| 147 | |||
| 148 | /** |
||
| 149 | * For back-compat, subclasses may return a raw SQL query here, as a string. |
||
| 150 | * This is strongly deprecated; getQueryInfo() should be overridden instead. |
||
| 151 | * @throws MWException |
||
| 152 | * @return string |
||
| 153 | */ |
||
| 154 | function getSQL() { |
||
| 159 | |||
| 160 | /** |
||
| 161 | * Subclasses return an array of fields to order by here. Don't append |
||
| 162 | * DESC to the field names, that'll be done automatically if |
||
| 163 | * sortDescending() returns true. |
||
| 164 | * @return array |
||
| 165 | * @since 1.18 |
||
| 166 | */ |
||
| 167 | function getOrderFields() { |
||
| 170 | |||
| 171 | /** |
||
| 172 | * Does this query return timestamps rather than integers in its |
||
| 173 | * 'value' field? If true, this class will convert 'value' to a |
||
| 174 | * UNIX timestamp for caching. |
||
| 175 | * NOTE: formatRow() may get timestamps in TS_MW (mysql), TS_DB (pgsql) |
||
| 176 | * or TS_UNIX (querycache) format, so be sure to always run them |
||
| 177 | * through wfTimestamp() |
||
| 178 | * @return bool |
||
| 179 | * @since 1.18 |
||
| 180 | */ |
||
| 181 | public function usesTimestamps() { |
||
| 184 | |||
| 185 | /** |
||
| 186 | * Override to sort by increasing values |
||
| 187 | * |
||
| 188 | * @return bool |
||
| 189 | */ |
||
| 190 | function sortDescending() { |
||
| 193 | |||
| 194 | /** |
||
| 195 | * Is this query expensive (for some definition of expensive)? Then we |
||
| 196 | * don't let it run in miser mode. $wgDisableQueryPages causes all query |
||
| 197 | * pages to be declared expensive. Some query pages are always expensive. |
||
| 198 | * |
||
| 199 | * @return bool |
||
| 200 | */ |
||
| 201 | public function isExpensive() { |
||
| 204 | |||
| 205 | /** |
||
| 206 | * Is the output of this query cacheable? Non-cacheable expensive pages |
||
| 207 | * will be disabled in miser mode and will not have their results written |
||
| 208 | * to the querycache table. |
||
| 209 | * @return bool |
||
| 210 | * @since 1.18 |
||
| 211 | */ |
||
| 212 | public function isCacheable() { |
||
| 215 | |||
| 216 | /** |
||
| 217 | * Whether or not the output of the page in question is retrieved from |
||
| 218 | * the database cache. |
||
| 219 | * |
||
| 220 | * @return bool |
||
| 221 | */ |
||
| 222 | public function isCached() { |
||
| 225 | |||
| 226 | /** |
||
| 227 | * Sometime we don't want to build rss / atom feeds. |
||
| 228 | * |
||
| 229 | * @return bool |
||
| 230 | */ |
||
| 231 | function isSyndicated() { |
||
| 234 | |||
| 235 | /** |
||
| 236 | * Formats the results of the query for display. The skin is the current |
||
| 237 | * skin; you can use it for making links. The result is a single row of |
||
| 238 | * result data. You should be able to grab SQL results off of it. |
||
| 239 | * If the function returns false, the line output will be skipped. |
||
| 240 | * @param Skin $skin |
||
| 241 | * @param object $result Result row |
||
| 242 | * @return string|bool String or false to skip |
||
| 243 | */ |
||
| 244 | abstract function formatResult( $skin, $result ); |
||
| 245 | |||
| 246 | /** |
||
| 247 | * The content returned by this function will be output before any result |
||
| 248 | * |
||
| 249 | * @return string |
||
| 250 | */ |
||
| 251 | function getPageHeader() { |
||
| 254 | |||
| 255 | /** |
||
| 256 | * Outputs some kind of an informative message (via OutputPage) to let the |
||
| 257 | * user know that the query returned nothing and thus there's nothing to |
||
| 258 | * show. |
||
| 259 | * |
||
| 260 | * @since 1.26 |
||
| 261 | */ |
||
| 262 | protected function showEmptyText() { |
||
| 265 | |||
| 266 | /** |
||
| 267 | * If using extra form wheely-dealies, return a set of parameters here |
||
| 268 | * as an associative array. They will be encoded and added to the paging |
||
| 269 | * links (prev/next/lengths). |
||
| 270 | * |
||
| 271 | * @return array |
||
| 272 | */ |
||
| 273 | function linkParameters() { |
||
| 276 | |||
| 277 | /** |
||
| 278 | * Some special pages (for example SpecialListusers used to) might not return the |
||
| 279 | * current object formatted, but return the previous one instead. |
||
| 280 | * Setting this to return true will ensure formatResult() is called |
||
| 281 | * one more time to make sure that the very last result is formatted |
||
| 282 | * as well. |
||
| 283 | * |
||
| 284 | * @deprecated since 1.27 |
||
| 285 | * |
||
| 286 | * @return bool |
||
| 287 | */ |
||
| 288 | function tryLastResult() { |
||
| 291 | |||
| 292 | /** |
||
| 293 | * Clear the cache and save new results |
||
| 294 | * |
||
| 295 | * @param int|bool $limit Limit for SQL statement |
||
| 296 | * @param bool $ignoreErrors Whether to ignore database errors |
||
| 297 | * @throws DBError|Exception |
||
| 298 | * @return bool|int |
||
| 299 | */ |
||
| 300 | public function recache( $limit, $ignoreErrors = true ) { |
||
| 373 | |||
| 374 | /** |
||
| 375 | * Get a DB connection to be used for slow recache queries |
||
| 376 | * @return IDatabase |
||
| 377 | */ |
||
| 378 | function getRecacheDB() { |
||
| 381 | |||
| 382 | /** |
||
| 383 | * Run the query and return the result |
||
| 384 | * @param int|bool $limit Numerical limit or false for no limit |
||
| 385 | * @param int|bool $offset Numerical offset or false for no offset |
||
| 386 | * @return ResultWrapper |
||
| 387 | * @since 1.18 |
||
| 388 | */ |
||
| 389 | public function reallyDoQuery( $limit, $offset = false ) { |
||
| 433 | |||
| 434 | /** |
||
| 435 | * Somewhat deprecated, you probably want to be using execute() |
||
| 436 | * @param int|bool $offset |
||
| 437 | * @param int|bool $limit |
||
| 438 | * @return ResultWrapper |
||
| 439 | */ |
||
| 440 | public function doQuery( $offset = false, $limit = false ) { |
||
| 447 | |||
| 448 | /** |
||
| 449 | * Fetch the query results from the query cache |
||
| 450 | * @param int|bool $limit Numerical limit or false for no limit |
||
| 451 | * @param int|bool $offset Numerical offset or false for no offset |
||
| 452 | * @return ResultWrapper |
||
| 453 | * @since 1.18 |
||
| 454 | */ |
||
| 455 | public function fetchFromCache( $limit, $offset = false ) { |
||
| 477 | |||
| 478 | public function getCachedTimestamp() { |
||
| 487 | |||
| 488 | /** |
||
| 489 | * Returns limit and offset, as returned by $this->getRequest()->getLimitOffset(). |
||
| 490 | * Subclasses may override this to further restrict or modify limit and offset. |
||
| 491 | * |
||
| 492 | * @note Restricts the offset parameter, as most query pages have inefficient paging |
||
| 493 | * |
||
| 494 | * Its generally expected that the returned limit will not be 0, and the returned |
||
| 495 | * offset will be less than the max results. |
||
| 496 | * |
||
| 497 | * @since 1.26 |
||
| 498 | * @return int[] list( $limit, $offset ) |
||
| 499 | */ |
||
| 500 | protected function getLimitOffset() { |
||
| 511 | |||
| 512 | /** |
||
| 513 | * What is limit to fetch from DB |
||
| 514 | * |
||
| 515 | * Used to make it appear the DB stores less results then it actually does |
||
| 516 | * @param int $uiLimit Limit from UI |
||
| 517 | * @param int $uiOffset Offset from UI |
||
| 518 | * @return int Limit to use for DB (not including extra row to see if at end) |
||
| 519 | */ |
||
| 520 | protected function getDBLimit( $uiLimit, $uiOffset ) { |
||
| 529 | |||
| 530 | /** |
||
| 531 | * Get max number of results we can return in miser mode. |
||
| 532 | * |
||
| 533 | * Most QueryPage subclasses use inefficient paging, so limit the max amount we return |
||
| 534 | * This matters for uncached query pages that might otherwise accept an offset of 3 million |
||
| 535 | * |
||
| 536 | * @since 1.27 |
||
| 537 | * @return int |
||
| 538 | */ |
||
| 539 | protected function getMaxResults() { |
||
| 543 | |||
| 544 | /** |
||
| 545 | * This is the actual workhorse. It does everything needed to make a |
||
| 546 | * real, honest-to-gosh query page. |
||
| 547 | * @param string $par |
||
| 548 | */ |
||
| 549 | public function execute( $par ) { |
||
| 657 | |||
| 658 | /** |
||
| 659 | * Format and output report results using the given information plus |
||
| 660 | * OutputPage |
||
| 661 | * |
||
| 662 | * @param OutputPage $out OutputPage to print to |
||
| 663 | * @param Skin $skin User skin to use |
||
| 664 | * @param IDatabase $dbr Database (read) connection to use |
||
| 665 | * @param ResultWrapper $res Result pointer |
||
| 666 | * @param int $num Number of available result rows |
||
| 667 | * @param int $offset Paging offset |
||
| 668 | */ |
||
| 669 | protected function outputResults( $out, $skin, $dbr, $res, $num, $offset ) { |
||
| 713 | |||
| 714 | /** |
||
| 715 | * @param int $offset |
||
| 716 | * @return string |
||
| 717 | */ |
||
| 718 | function openList( $offset ) { |
||
| 721 | |||
| 722 | /** |
||
| 723 | * @return string |
||
| 724 | */ |
||
| 725 | function closeList() { |
||
| 728 | |||
| 729 | /** |
||
| 730 | * Do any necessary preprocessing of the result object. |
||
| 731 | * @param IDatabase $db |
||
| 732 | * @param ResultWrapper $res |
||
| 733 | */ |
||
| 734 | function preprocessResults( $db, $res ) { |
||
| 736 | |||
| 737 | /** |
||
| 738 | * Similar to above, but packaging in a syndicated feed instead of a web page |
||
| 739 | * @param string $class |
||
| 740 | * @param int $limit |
||
| 741 | * @return bool |
||
| 742 | */ |
||
| 743 | function doFeed( $class = '', $limit = 50 ) { |
||
| 774 | |||
| 775 | /** |
||
| 776 | * Override for custom handling. If the titles/links are ok, just do |
||
| 777 | * feedItemDesc() |
||
| 778 | * @param object $row |
||
| 779 | * @return FeedItem|null |
||
| 780 | */ |
||
| 781 | function feedResult( $row ) { |
||
| 805 | |||
| 806 | function feedItemDesc( $row ) { |
||
| 809 | |||
| 810 | function feedItemAuthor( $row ) { |
||
| 813 | |||
| 814 | View Code Duplication | function feedTitle() { |
|
| 820 | |||
| 821 | function feedDesc() { |
||
| 824 | |||
| 825 | function feedUrl() { |
||
| 828 | |||
| 829 | /** |
||
| 830 | * Creates a new LinkBatch object, adds all pages from the passed ResultWrapper (MUST include |
||
| 831 | * title and optional the namespace field) and executes the batch. This operation will pre-cache |
||
| 832 | * LinkCache information like page existence and information for stub color and redirect hints. |
||
| 833 | * |
||
| 834 | * @param ResultWrapper $res The ResultWrapper object to process. Needs to include the title |
||
| 835 | * field and namespace field, if the $ns parameter isn't set. |
||
| 836 | * @param null $ns Use this namespace for the given titles in the ResultWrapper object, |
||
| 837 | * instead of the namespace value of $res. |
||
| 838 | */ |
||
| 839 | protected function executeLBFromResultWrapper( ResultWrapper $res, $ns = null ) { |
||
| 852 | } |
||
| 853 |
Loading history...
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVarassignment in line 1 and the$higherassignment in line 2 are dead. The first because$myVaris never used and the second because$higheris always overwritten for every possible time line.