x-tools /
xtools
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 ApiHelper 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 ApiHelper, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 19 | class ApiHelper extends HelperBase |
||
| 20 | { |
||
| 21 | /** @var MediawikiApi The API object. */ |
||
| 22 | private $api; |
||
| 23 | |||
| 24 | /** @var LabsHelper The Labs helper. */ |
||
| 25 | private $labsHelper; |
||
| 26 | |||
| 27 | /** @var CacheItemPoolInterface The cache. */ |
||
| 28 | protected $cache; |
||
| 29 | |||
| 30 | /** @var ContainerInterface The DI container. */ |
||
| 31 | protected $container; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * ApiHelper constructor. |
||
| 35 | * @param ContainerInterface $container |
||
| 36 | * @param LabsHelper $labsHelper |
||
| 37 | */ |
||
| 38 | public function __construct(ContainerInterface $container, LabsHelper $labsHelper) |
||
| 44 | |||
| 45 | /** |
||
| 46 | * Set up the MediawikiApi object for the given project. |
||
| 47 | * |
||
| 48 | * @param string $project |
||
| 49 | */ |
||
| 50 | private function setUp($project) |
||
| 57 | |||
| 58 | /** |
||
| 59 | * Get the given user's groups on the given project. |
||
| 60 | * @deprecated Use User::getGroups() instead. |
||
| 61 | * @param string $project |
||
| 62 | * @param string $username |
||
| 63 | * @return string[] |
||
| 64 | */ |
||
| 65 | View Code Duplication | public function groups($project, $username) |
|
| 83 | |||
| 84 | /** |
||
| 85 | * Get the given user's globally-applicable groups. |
||
| 86 | * @deprecated Use User::getGlobalGroups() instead. |
||
| 87 | * @param string $project |
||
| 88 | * @param string $username |
||
| 89 | * @return string[] |
||
| 90 | */ |
||
| 91 | View Code Duplication | public function globalGroups($project, $username) |
|
| 109 | |||
| 110 | /** |
||
| 111 | * Get a list of administrators for the given project. |
||
| 112 | * @TODO Move to the Project class? |
||
| 113 | * @param string $project |
||
| 114 | * @return string[] |
||
| 115 | */ |
||
| 116 | public function getAdmins($project) |
||
| 162 | |||
| 163 | /** |
||
| 164 | * Get basic info about a page via the API |
||
| 165 | * @param string $project Full domain of project (en.wikipedia.org) |
||
| 166 | * @param string $page Page title |
||
| 167 | * @param boolean $followRedir Whether or not to resolve redirects |
||
| 168 | * @return array Associative array of data |
||
| 169 | */ |
||
| 170 | public function getBasicPageInfo($project, $page, $followRedir) |
||
| 204 | |||
| 205 | /** |
||
| 206 | * Get HTML display titles of a set of pages (or the normal title if there's no display title). |
||
| 207 | * This will send t/50 API requests where t is the number of titles supplied. |
||
| 208 | * @param string $project The project. |
||
| 209 | * @param string[] $pageTitles The titles to fetch. |
||
| 210 | * @return string[] Keys are the original supplied title, and values are the display titles. |
||
| 211 | */ |
||
| 212 | public function displayTitles($project, $pageTitles) |
||
| 250 | |||
| 251 | /** |
||
| 252 | * Make mass API requests to MediaWiki API |
||
| 253 | * The API normally limits to 500 pages, but gives you a 'continue' value |
||
| 254 | * to finish iterating through the resource. |
||
| 255 | * Adapted from https://github.com/MusikAnimal/pageviews |
||
| 256 | * @param array $params Associative array of params to pass to API |
||
| 257 | * @param string $project Project to query, e.g. en.wikipedia.org |
||
| 258 | * @param string|func $dataKey The key for the main chunk of data, in the query hash |
||
| 259 | * (e.g. 'categorymembers' for API:Categorymembers). |
||
| 260 | * If this is a function it is given the response data, |
||
| 261 | * and expected to return the data we want to concatentate. |
||
| 262 | * @param string [$continueKey] the key to look in the continue hash, if present |
||
| 263 | * (e.g. 'cmcontinue' for API:Categorymembers) |
||
| 264 | * @param integer [$limit] Max number of pages to fetch |
||
| 265 | * @return array Associative array with data |
||
| 266 | */ |
||
| 267 | public function massApi($params, $project, $dataKey, $continueKey = 'continue', $limit = 5000) |
||
| 293 | |||
| 294 | /** |
||
| 295 | * Internal function used by massApi() to make recursive calls |
||
| 296 | * @param array &$data Everything we need to keep track of, as defined in massApi() |
||
| 297 | * @return null Nothing. $data['promise']->then is used to continue flow of |
||
| 298 | * execution after all recursive calls are complete |
||
| 299 | */ |
||
| 300 | private function massApiInternal(&$data) |
||
| 364 | } |
||
| 365 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.