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 ShareByCircleProvider 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 ShareByCircleProvider, and based on these observations, apply Extract Interface, too.
| 1 | <?php | ||
| 50 | class ShareByCircleProvider extends CircleProviderRequestBuilder implements IShareProvider { | ||
| 51 | |||
| 52 | private $misc; | ||
| 53 | |||
| 54 | |||
| 55 | /** @var ILogger */ | ||
| 56 | private $logger; | ||
| 57 | |||
| 58 | /** @var ISecureRandom */ | ||
| 59 | private $secureRandom; | ||
| 60 | |||
| 61 | /** @var IUserManager */ | ||
| 62 | private $userManager; | ||
| 63 | |||
| 64 | /** @var IRootFolder */ | ||
| 65 | private $rootFolder; | ||
| 66 | |||
| 67 | /** @var IL10N */ | ||
| 68 | private $l; | ||
| 69 | |||
| 70 | /** @var IURLGenerator */ | ||
| 71 | private $urlGenerator; | ||
| 72 | |||
| 73 | |||
| 74 | /** | ||
| 75 | * DefaultShareProvider constructor. | ||
| 76 | * | ||
| 77 | * @param IDBConnection $connection | ||
| 78 | * @param ISecureRandom $secureRandom | ||
| 79 | * @param IUserManager $userManager | ||
| 80 | * @param IRootFolder $rootFolder | ||
| 81 | * @param IL10N $l | ||
| 82 | * @param ILogger $logger | ||
| 83 | * @param IURLGenerator $urlGenerator | ||
| 84 | */ | ||
| 85 | public function __construct( | ||
| 102 | |||
| 103 | |||
| 104 | /** | ||
| 105 | * Return the identifier of this provider. | ||
| 106 | * | ||
| 107 | * @return string | ||
| 108 | */ | ||
| 109 | 	public function identifier() { | ||
| 112 | |||
| 113 | |||
| 114 | /** | ||
| 115 | * Create a share if it does not exist already. | ||
| 116 | * | ||
| 117 | * @param IShare $share | ||
| 118 | * | ||
| 119 | * @return IShare The share object | ||
| 120 | * @throws \Exception | ||
| 121 | */ | ||
| 122 | // TODO: check if user can create a share in this circle ! | ||
| 123 | 	public function create(IShare $share) { | ||
| 140 | |||
| 141 | |||
| 142 | /** | ||
| 143 | * Update a share | ||
| 144 | * permissions right, owner and initiator | ||
| 145 | * | ||
| 146 | * @param IShare $share | ||
| 147 | * | ||
| 148 | * @return IShare The share object | ||
| 149 | */ | ||
| 150 | 	public function update(IShare $share) { | ||
| 161 | |||
| 162 | |||
| 163 | /** | ||
| 164 | * Delete a share, and it's children | ||
| 165 | * | ||
| 166 | * @param IShare $share | ||
| 167 | */ | ||
| 168 | 	public function delete(IShare $share) { | ||
| 175 | |||
| 176 | |||
| 177 | /** | ||
| 178 | * Unshare a file from self as recipient. | ||
| 179 | * Because every circles share are group shares, we will set permissions to 0 | ||
| 180 | * | ||
| 181 | * @param IShare $share | ||
| 182 | * @param string $userId | ||
| 183 | */ | ||
| 184 | View Code Duplication | 	public function deleteFromSelf(IShare $share, $userId) { | |
| 193 | |||
| 194 | |||
| 195 | /** | ||
| 196 | * Move a share as a recipient. | ||
| 197 | * | ||
| 198 | * @param IShare $share | ||
| 199 | * @param string $userId | ||
| 200 | * | ||
| 201 | * @return IShare | ||
| 202 | * | ||
| 203 | */ | ||
| 204 | View Code Duplication | 	public function move(IShare $share, $userId) { | |
| 215 | |||
| 216 | |||
| 217 | /** | ||
| 218 | * return the child ID of a share | ||
| 219 | * | ||
| 220 | * @param IShare $share | ||
| 221 | * @param string $userId | ||
| 222 | * | ||
| 223 | * @return bool | ||
| 224 | */ | ||
| 225 | 	private function getShareChildId(IShare $share, $userId) { | ||
| 239 | |||
| 240 | |||
| 241 | /** | ||
| 242 | * Create a child and returns its ID | ||
| 243 | * | ||
| 244 | * @param IShare $share | ||
| 245 | * | ||
| 246 | * @return int | ||
| 247 | */ | ||
| 248 | 	private function createShare($share) { | ||
| 255 | |||
| 256 | |||
| 257 | /** | ||
| 258 | * Create a child and returns its ID | ||
| 259 | * | ||
| 260 | * @param string $userId | ||
| 261 | * @param IShare $share | ||
| 262 | * | ||
| 263 | * @return int | ||
| 264 | */ | ||
| 265 | 	private function createShareChild($userId, $share) { | ||
| 275 | |||
| 276 | |||
| 277 | /** | ||
| 278 | * Get all shares by the given user in a folder | ||
| 279 | * | ||
| 280 | * @param string $userId | ||
| 281 | * @param Folder $node | ||
| 282 | * @param bool $reshares Also get the shares where $user is the owner instead of just the | ||
| 283 | * shares where $user is the initiator | ||
| 284 | * | ||
| 285 | * @return Share[] | ||
| 286 | */ | ||
| 287 | 	public function getSharesInFolder($userId, Folder $node, $reshares) { | ||
| 288 | $qb = $this->getBaseSelectSql(); | ||
| 289 | $this->limitToShareOwner($qb, $userId, true); | ||
| 290 | $cursor = $qb->execute(); | ||
| 291 | |||
| 292 | $shares = []; | ||
| 293 | 		while ($data = $cursor->fetch()) { | ||
| 294 | $shares[$data['file_source']][] = $this->createShareObject($data); | ||
| 295 | } | ||
| 296 | $cursor->closeCursor(); | ||
| 297 | |||
| 298 | return $shares; | ||
| 299 | } | ||
| 300 | |||
| 301 | |||
| 302 | /** | ||
| 303 | * Get all shares by the given user | ||
| 304 | * | ||
| 305 | * @param string $userId | ||
| 306 | * @param int $shareType | ||
| 307 | * @param Node|null $node | ||
| 308 | * @param bool $reShares | ||
| 309 | * @param int $limit The maximum number of shares to be returned, -1 for all shares | ||
| 310 | * @param int $offset | ||
| 311 | * | ||
| 312 | * @return Share[] | ||
| 313 | */ | ||
| 314 | 	public function getSharesBy($userId, $shareType, $node, $reShares, $limit, $offset) { | ||
| 315 | $qb = $this->getBaseSelectSql(); | ||
| 316 | $this->limitToShareOwner($qb, $userId, $reShares); | ||
| 317 | |||
| 318 | 		if ($node !== null) { | ||
| 319 | $this->limitToFiles($qb, $node->getId()); | ||
| 320 | } | ||
| 321 | |||
| 322 | $this->limitToPage($qb, $limit, $offset); | ||
| 323 | $cursor = $qb->execute(); | ||
| 324 | |||
| 325 | $shares = []; | ||
| 326 | 		while ($data = $cursor->fetch()) { | ||
| 327 | $shares[] = $this->createShareObject($this->editShareEntry($data)); | ||
| 328 | } | ||
| 329 | $cursor->closeCursor(); | ||
| 330 | |||
| 331 | return $shares; | ||
| 332 | } | ||
| 333 | |||
| 334 | |||
| 335 | /** | ||
| 336 | * returns a better formatted string to display more information about | ||
| 337 | * the circle to the Sharing UI | ||
| 338 | * | ||
| 339 | * @param $data | ||
| 340 | * | ||
| 341 | * @return array<string,string> | ||
| 342 | */ | ||
| 343 | 	private function editShareEntry($data) { | ||
| 352 | |||
| 353 | |||
| 354 | /** | ||
| 355 | * Get share by its id | ||
| 356 | * | ||
| 357 | * @param int $shareId | ||
| 358 | * @param string|null $recipientId | ||
| 359 | * | ||
| 360 | * @return Share | ||
| 361 | * @throws ShareNotFound | ||
| 362 | */ | ||
| 363 | 	public function getShareById($shareId, $recipientId = null) { | ||
| 377 | |||
| 378 | |||
| 379 | /** | ||
| 380 | * Get shares for a given path | ||
| 381 | * | ||
| 382 | * @param Node $path | ||
| 383 | * | ||
| 384 | * @return IShare[]|null | ||
| 385 | */ | ||
| 386 | 	public function getSharesByPath(Node $path) { | ||
| 389 | |||
| 390 | |||
| 391 | /** | ||
| 392 | * Get shared with the given user | ||
| 393 | * | ||
| 394 | * @param string $userId get shares where this user is the recipient | ||
| 395 | * @param int $shareType | ||
| 396 | * @param Node|null $node | ||
| 397 | * @param int $limit The max number of entries returned, -1 for all | ||
| 398 | * @param int $offset | ||
| 399 | * | ||
| 400 | * @return array|IShare[] | ||
| 401 | */ | ||
| 402 | 	public function getSharedWith($userId, $shareType, $node, $limit, $offset) { | ||
| 427 | |||
| 428 | |||
| 429 | /** | ||
| 430 | * @param $data | ||
| 431 | */ | ||
| 432 | 	private static function editShareFromParentEntry(& $data) { | ||
| 441 | |||
| 442 | |||
| 443 | /** | ||
| 444 | * Get a share by token | ||
| 445 | * | ||
| 446 | * @param string $token | ||
| 447 | * | ||
| 448 | * @return IShare | ||
| 449 | * @throws ShareNotFound | ||
| 450 | */ | ||
| 451 | 	public function getShareByToken($token) { | ||
| 454 | |||
| 455 | |||
| 456 | /** | ||
| 457 | * We don't return a thing about children. | ||
| 458 | * The call to this function is deprecated and should be removed in next release of NC. | ||
| 459 | * Also, we get the children in the delete() method. | ||
| 460 | * | ||
| 461 | * @param IShare $parent | ||
| 462 | * | ||
| 463 | * @return array | ||
| 464 | */ | ||
| 465 | 	public function getChildren(IShare $parent) { | ||
| 468 | |||
| 469 | |||
| 470 | /** | ||
| 471 | * A user is deleted from the system | ||
| 472 | * So clean up the relevant shares. | ||
| 473 | * | ||
| 474 | * @param string $uid | ||
| 475 | * @param int $shareType | ||
| 476 | */ | ||
| 477 | 	public function userDeleted($uid, $shareType) { | ||
| 480 | |||
| 481 | |||
| 482 | /** | ||
| 483 | * A group is deleted from the system. | ||
| 484 | * We handle our own groups. | ||
| 485 | * | ||
| 486 | * @param string $gid | ||
| 487 | */ | ||
| 488 | 	public function groupDeleted($gid) { | ||
| 491 | |||
| 492 | |||
| 493 | /** | ||
| 494 | * A user is deleted from a group. | ||
| 495 | * We handle our own groups. | ||
| 496 | * | ||
| 497 | * @param string $uid | ||
| 498 | * @param string $gid | ||
| 499 | */ | ||
| 500 | 	public function userDeletedFromGroup($uid, $gid) { | ||
| 503 | |||
| 504 | |||
| 505 | /** | ||
| 506 | * Create a share object | ||
| 507 | * | ||
| 508 | * @param array $data | ||
| 509 | * | ||
| 510 | * @return Share | ||
| 511 | */ | ||
| 512 | 	private function createShareObject($data) { | ||
| 529 | |||
| 530 | |||
| 531 | /** | ||
| 532 | * @param IShare $share | ||
| 533 | * @param $data | ||
| 534 | */ | ||
| 535 | 	private function assignShareObjectPropertiesFromParent(& $share, $data) { | ||
| 548 | |||
| 549 | |||
| 550 | /** | ||
| 551 | * @param IShare $share | ||
| 552 | * @param $data | ||
| 553 | */ | ||
| 554 | 	private function assignShareObjectSharesProperties(& $share, $data) { | ||
| 564 | |||
| 565 | /** | ||
| 566 | * Returns whether the given database result can be interpreted as | ||
| 567 | * a share with accessible file (not trashed, not deleted) | ||
| 568 | * | ||
| 569 | * @param $data | ||
| 570 | *F | ||
| 571 | * | ||
| 572 | * @return bool | ||
| 573 | */ | ||
| 574 | 	private static function isAccessibleResult($data) { | ||
| 582 | |||
| 583 | |||
| 584 | /** | ||
| 585 | * @param IShare $share | ||
| 586 | * | ||
| 587 | * @return \Exception | ||
| 588 | */ | ||
| 589 | 	private function errorShareAlreadyExist($share) { | ||
| 601 | |||
| 602 | |||
| 603 | /** | ||
| 604 | * Get the access list to the array of provided nodes. | ||
| 605 | * | ||
| 606 | * @see IManager::getAccessList() for sample docs | ||
| 607 | * | ||
| 608 | * @param Node[] $nodes The list of nodes to get access for | ||
| 609 | * @param bool $currentAccess If current access is required (like for removed shares that might | ||
| 610 | * get revived later) | ||
| 611 | * | ||
| 612 | * @return array | ||
| 613 | * @since 12 | ||
| 614 | */ | ||
| 615 | 	public function getAccessList($nodes, $currentAccess) { | ||
| 633 | |||
| 634 | |||
| 635 | /** | ||
| 636 | * return array regarding getAccessList format. | ||
| 637 | * ie. \OC\Share20\Manager::getAccessList() | ||
| 638 | * | ||
| 639 | * @param $qb | ||
| 640 | * | ||
| 641 | * @return array | ||
| 642 | */ | ||
| 643 | 	private function parseAccessListResult($qb) { | ||
| 662 | } | ||
| 663 | 
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.