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 FilesReportPlugin 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 FilesReportPlugin, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 42 | class FilesReportPlugin extends ServerPlugin { |
||
| 43 | |||
| 44 | // namespace |
||
| 45 | const NS_OWNCLOUD = 'http://owncloud.org/ns'; |
||
| 46 | const REPORT_NAME = '{http://owncloud.org/ns}filter-files'; |
||
| 47 | const SYSTEMTAG_PROPERTYNAME = '{http://owncloud.org/ns}systemtag'; |
||
| 48 | |||
| 49 | /** |
||
| 50 | * Reference to main server object |
||
| 51 | * |
||
| 52 | * @var \Sabre\DAV\Server |
||
| 53 | */ |
||
| 54 | private $server; |
||
| 55 | |||
| 56 | /** |
||
| 57 | * @var Tree |
||
| 58 | */ |
||
| 59 | private $tree; |
||
| 60 | |||
| 61 | /** |
||
| 62 | * @var View |
||
| 63 | */ |
||
| 64 | private $fileView; |
||
| 65 | |||
| 66 | /** |
||
| 67 | * @var ISystemTagManager |
||
| 68 | */ |
||
| 69 | private $tagManager; |
||
| 70 | |||
| 71 | /** |
||
| 72 | * @var ISystemTagObjectMapper |
||
| 73 | */ |
||
| 74 | private $tagMapper; |
||
| 75 | |||
| 76 | /** |
||
| 77 | * Manager for private tags |
||
| 78 | * |
||
| 79 | * @var ITagManager |
||
| 80 | */ |
||
| 81 | private $fileTagger; |
||
| 82 | |||
| 83 | /** |
||
| 84 | * @var IUserSession |
||
| 85 | */ |
||
| 86 | private $userSession; |
||
| 87 | |||
| 88 | /** |
||
| 89 | * @var IGroupManager |
||
| 90 | */ |
||
| 91 | private $groupManager; |
||
| 92 | |||
| 93 | /** |
||
| 94 | * @var Folder |
||
| 95 | */ |
||
| 96 | private $userFolder; |
||
| 97 | |||
| 98 | /** |
||
| 99 | * @param Tree $tree |
||
| 100 | * @param View $view |
||
| 101 | * @param ISystemTagManager $tagManager |
||
| 102 | * @param ISystemTagObjectMapper $tagMapper |
||
| 103 | * @param ITagManager $fileTagger manager for private tags |
||
| 104 | * @param IUserSession $userSession |
||
| 105 | * @param IGroupManager $groupManager |
||
| 106 | * @param Folder $userFolder |
||
| 107 | */ |
||
| 108 | public function __construct(Tree $tree, |
||
| 126 | |||
| 127 | /** |
||
| 128 | * This initializes the plugin. |
||
| 129 | * |
||
| 130 | * This function is called by \Sabre\DAV\Server, after |
||
| 131 | * addPlugin is called. |
||
| 132 | * |
||
| 133 | * This method should set up the required event subscriptions. |
||
| 134 | * |
||
| 135 | * @param \Sabre\DAV\Server $server |
||
| 136 | * @return void |
||
| 137 | */ |
||
| 138 | View Code Duplication | public function initialize(\Sabre\DAV\Server $server) { |
|
| 146 | |||
| 147 | /** |
||
| 148 | * Returns a list of reports this plugin supports. |
||
| 149 | * |
||
| 150 | * This will be used in the {DAV:}supported-report-set property. |
||
| 151 | * |
||
| 152 | * @param string $uri |
||
| 153 | * @return array |
||
| 154 | */ |
||
| 155 | public function getSupportedReportSet($uri) { |
||
| 158 | |||
| 159 | /** |
||
| 160 | * REPORT operations to look for files |
||
| 161 | * |
||
| 162 | * @param string $reportName |
||
| 163 | * @param mixed $report |
||
| 164 | * @param string $uri |
||
| 165 | * @return bool |
||
| 166 | * @throws BadRequest |
||
| 167 | * @throws PreconditionFailed |
||
| 168 | * @internal param $ [] $report |
||
| 169 | */ |
||
| 170 | public function onReport($reportName, $report, $uri) { |
||
| 215 | |||
| 216 | private function slice($results, $report) { |
||
| 224 | |||
| 225 | /** |
||
| 226 | * Returns the base uri of the files root by removing |
||
| 227 | * the subpath from the URI |
||
| 228 | * |
||
| 229 | * @param string $uri URI from this request |
||
| 230 | * @param string $subPath subpath to remove from the URI |
||
| 231 | * |
||
| 232 | * @return string files base uri |
||
| 233 | */ |
||
| 234 | private function getFilesBaseUri($uri, $subPath) { |
||
| 248 | |||
| 249 | /** |
||
| 250 | * Find file ids matching the given filter rules |
||
| 251 | * |
||
| 252 | * @param array $filterRules |
||
| 253 | * @return array array of unique file id results |
||
| 254 | * |
||
| 255 | * @throws TagNotFoundException whenever a tag was not found |
||
| 256 | */ |
||
| 257 | protected function processFilterRules($filterRules) { |
||
| 280 | |||
| 281 | private function getSystemTagFileIds($systemTagIds) { |
||
| 323 | |||
| 324 | /** |
||
| 325 | * Prepare propfind response for the given nodes |
||
| 326 | * |
||
| 327 | * @param string $filesUri $filesUri URI leading to root of the files URI, |
||
| 328 | * with a leading slash but no trailing slash |
||
| 329 | * @param string[] $requestedProps requested properties |
||
| 330 | * @param Node[] nodes nodes for which to fetch and prepare responses |
||
| 331 | * @return Response[] |
||
| 332 | */ |
||
| 333 | public function prepareResponses($filesUri, $requestedProps, $nodes) { |
||
| 347 | |||
| 348 | /** |
||
| 349 | * Find Sabre nodes by file ids |
||
| 350 | * |
||
| 351 | * @param Node $rootNode root node for search |
||
| 352 | * @param array $fileIds file ids |
||
| 353 | * @return Node[] array of Sabre nodes |
||
| 354 | */ |
||
| 355 | public function findNodesByFileIds($rootNode, $fileIds) { |
||
| 375 | |||
| 376 | private function makeSabreNode(\OCP\Files\Node $filesNode) { |
||
| 384 | |||
| 385 | /** |
||
| 386 | * Returns whether the currently logged in user is an administrator |
||
| 387 | */ |
||
| 388 | View Code Duplication | private function isAdmin() { |
|
| 395 | } |
||
| 396 |
This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.
If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.
In this case you can add the
@ignorePhpDoc annotation to the duplicate definition and it will be ignored.