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 EventsService 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 EventsService, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 47 | class EventsService { |
||
| 48 | |||
| 49 | |||
| 50 | /** @var string */ |
||
| 51 | private $userId; |
||
| 52 | |||
| 53 | /** @var ITimeFactory */ |
||
| 54 | private $time; |
||
| 55 | |||
| 56 | /** @var IActivityManager */ |
||
| 57 | private $activityManager; |
||
| 58 | |||
| 59 | /** @var INotificationManager */ |
||
| 60 | private $notificationManager; |
||
| 61 | |||
| 62 | /** @var IUserManager */ |
||
| 63 | private $userManager; |
||
| 64 | |||
| 65 | /** @var IURLGenerator */ |
||
| 66 | private $urlGenerator; |
||
| 67 | |||
| 68 | /** @var EventDispatcher */ |
||
| 69 | private $eventDispatcher; |
||
| 70 | |||
| 71 | /** @var CirclesRequest */ |
||
| 72 | private $circlesRequest; |
||
| 73 | |||
| 74 | /** @var MembersRequest */ |
||
| 75 | private $membersRequest; |
||
| 76 | |||
| 77 | /** @var ConfigService */ |
||
| 78 | private $configService; |
||
| 79 | |||
| 80 | /** @var MiscService */ |
||
| 81 | private $miscService; |
||
| 82 | |||
| 83 | |||
| 84 | /** |
||
| 85 | * Events constructor. |
||
| 86 | * |
||
| 87 | * @param string $userId |
||
| 88 | * @param ITimeFactory $time |
||
| 89 | * @param IActivityManager $activityManager |
||
| 90 | * @param INotificationManager $notificationManager |
||
| 91 | * @param IUserManager $userManager |
||
| 92 | * @param IURLGenerator $urlGenerator |
||
| 93 | * @param EventDispatcher $eventDispatcher |
||
| 94 | * @param CirclesRequest $circlesRequest |
||
| 95 | * @param MembersRequest $membersRequest |
||
| 96 | * @param ConfigService $configService |
||
| 97 | * @param MiscService $miscService |
||
| 98 | */ |
||
| 99 | View Code Duplication | public function __construct( |
|
| 117 | |||
| 118 | |||
| 119 | /** |
||
| 120 | * onCircleCreation() |
||
| 121 | * |
||
| 122 | * Called when a circle is created. |
||
| 123 | * Broadcast an activity to the cloud |
||
| 124 | * We won't do anything if the circle is not PUBLIC or CLOSED |
||
| 125 | * |
||
| 126 | * @param Circle $circle |
||
| 127 | */ |
||
| 128 | public function onCircleCreation(Circle $circle) { |
||
| 129 | if ($circle->getType() !== Circle::CIRCLES_PUBLIC |
||
| 130 | && $circle->getType() !== Circle::CIRCLES_CLOSED) { |
||
| 131 | return; |
||
| 132 | } |
||
| 133 | |||
| 134 | if ($this->configService->getAppValue(ConfigService::CIRCLES_ACTIVITY_ON_CREATION) === '1') { |
||
| 135 | $event = $this->generateEvent('circles_as_non_member'); |
||
| 136 | $event->setSubject('circle_create', ['circle' => json_encode($circle)]); |
||
| 137 | |||
| 138 | $this->userManager->callForSeenUsers( |
||
| 139 | function($user) use ($event) { |
||
| 140 | /** @var IUser $user */ |
||
| 141 | $this->publishEvent($event, [$user]); |
||
| 142 | } |
||
| 143 | ); |
||
| 144 | } |
||
| 145 | |||
| 146 | $this->dispatch('\OCA\Circles::onCircleCreation', ['circle' => $circle]); |
||
| 147 | } |
||
| 148 | |||
| 149 | |||
| 150 | /** |
||
| 151 | * onCircleDestruction() |
||
| 152 | * |
||
| 153 | * Called when a circle is destroyed. |
||
| 154 | * Broadcast an activity on its members. |
||
| 155 | * We won't do anything if the circle is PERSONAL |
||
| 156 | * |
||
| 157 | * @param Circle $circle |
||
| 158 | */ |
||
| 159 | public function onCircleDestruction(Circle $circle) { |
||
| 160 | if ($circle->getType() === Circle::CIRCLES_PERSONAL) { |
||
| 161 | return; |
||
| 162 | } |
||
| 163 | |||
| 164 | $event = $this->generateEvent('circles_as_member'); |
||
| 165 | $event->setSubject('circle_delete', ['circle' => json_encode($circle)]); |
||
| 166 | $this->publishEvent( |
||
| 167 | $event, |
||
| 168 | $this->membersRequest->forceGetMembers($circle->getUniqueId(), Member::LEVEL_MEMBER, 0, true) |
||
| 169 | ); |
||
| 170 | |||
| 171 | $this->dispatch('\OCA\Circles::onCircleDestruction', ['circle' => $circle]); |
||
| 172 | } |
||
| 173 | |||
| 174 | |||
| 175 | /** |
||
| 176 | * onMemberNew() |
||
| 177 | * |
||
| 178 | * Called when a member is added to a circle. |
||
| 179 | * Broadcast an activity to the new member and to the moderators of the circle. |
||
| 180 | * We won't do anything if the circle is PERSONAL |
||
| 181 | * If the level is still 0, we will redirect to onMemberAlmost and manage the |
||
| 182 | * invitation/request from there |
||
| 183 | * If the level is Owner, we ignore the event. |
||
| 184 | * |
||
| 185 | * @param Circle $circle |
||
| 186 | * @param Member $member |
||
| 187 | */ |
||
| 188 | public function onMemberNew(Circle $circle, Member $member) { |
||
| 189 | if ($member->getLevel() === Member::LEVEL_OWNER |
||
| 190 | || $circle->getType() === Circle::CIRCLES_PERSONAL |
||
| 191 | ) { |
||
| 192 | return; |
||
| 193 | } |
||
| 194 | |||
| 195 | if ($member->getLevel() === Member::LEVEL_NONE) { |
||
| 196 | $this->onMemberAlmost($circle, $member); |
||
| 197 | |||
| 198 | return; |
||
| 199 | } |
||
| 200 | |||
| 201 | $event = $this->generateEvent('circles_as_member'); |
||
| 202 | $event->setSubject( |
||
| 203 | ($this->userId === $member->getUserId()) ? 'member_join' : 'member_add', |
||
| 204 | ['circle' => json_encode($circle), 'member' => json_encode($member)] |
||
| 205 | ); |
||
| 206 | |||
| 207 | $this->publishEvent( |
||
| 208 | $event, array_merge( |
||
| 209 | [$member], |
||
| 210 | $this->membersRequest->forceGetMembers( |
||
| 211 | $circle->getUniqueId(), Member::LEVEL_MODERATOR, 0, true |
||
| 212 | ) |
||
| 213 | ) |
||
| 214 | ); |
||
| 215 | |||
| 216 | $this->dispatch('\OCA\Circles::onMemberNew', ['circle' => $circle, 'member' => $member]); |
||
| 217 | |||
| 218 | $this->notificationOnMemberNew($circle, $member); |
||
| 219 | } |
||
| 220 | |||
| 221 | |||
| 222 | /** |
||
| 223 | * onMemberAlmost() |
||
| 224 | * |
||
| 225 | * Called when a member is added to a circle with level=0 |
||
| 226 | * Trigger onMemberInvitation() or onMemberInvitationRequest() based on Member Status |
||
| 227 | * |
||
| 228 | * @param Circle $circle |
||
| 229 | * @param Member $member |
||
| 230 | */ |
||
| 231 | private function onMemberAlmost(Circle $circle, Member $member) { |
||
| 245 | |||
| 246 | |||
| 247 | /** |
||
| 248 | * onMemberInvited() |
||
| 249 | * |
||
| 250 | * Called when a member is invited to a circle. |
||
| 251 | * Broadcast an activity to the invited member and to the moderators of the circle. |
||
| 252 | * |
||
| 253 | * @param Circle $circle |
||
| 254 | * @param Member $member |
||
| 255 | */ |
||
| 256 | View Code Duplication | private function onMemberInvited(Circle $circle, Member $member) { |
|
| 278 | |||
| 279 | |||
| 280 | /** |
||
| 281 | * onMemberRequesting() |
||
| 282 | * |
||
| 283 | * Called when a member request an invitation to a private circle. |
||
| 284 | * Broadcast an activity to the requester and to the moderators of the circle. |
||
| 285 | * |
||
| 286 | * @param Circle $circle |
||
| 287 | * @param Member $member |
||
| 288 | */ |
||
| 289 | View Code Duplication | private function onMemberRequesting(Circle $circle, Member $member) { |
|
| 312 | |||
| 313 | |||
| 314 | /** |
||
| 315 | * onMemberLeaving() |
||
| 316 | * |
||
| 317 | * Called when a member is removed from a circle. |
||
| 318 | * Broadcast an activity to the leaving member and to the moderators of the circle. |
||
| 319 | * We won't do anything if the circle is PERSONAL |
||
| 320 | * |
||
| 321 | * @param Circle $circle |
||
| 322 | * @param Member $member |
||
| 323 | */ |
||
| 324 | public function onMemberLeaving(Circle $circle, Member $member) { |
||
| 349 | |||
| 350 | |||
| 351 | /** |
||
| 352 | * onMemberLevel() |
||
| 353 | * |
||
| 354 | * Called when a member have his level changed. |
||
| 355 | * Broadcast an activity to all moderator of the circle, and the member if he is demoted. |
||
| 356 | * If the level is Owner, we identify the event as a Coup d'Etat and we broadcast all members. |
||
| 357 | * |
||
| 358 | * @param Circle $circle |
||
| 359 | * @param Member $member |
||
| 360 | */ |
||
| 361 | View Code Duplication | public function onMemberLevel(Circle $circle, Member $member) { |
|
| 381 | |||
| 382 | |||
| 383 | /** |
||
| 384 | * onMemberOwner() |
||
| 385 | * |
||
| 386 | * Called when the owner rights of a circle have be given to another member. |
||
| 387 | * |
||
| 388 | * @param Circle $circle |
||
| 389 | * @param Member $member |
||
| 390 | */ |
||
| 391 | public function onMemberOwner(Circle $circle, Member $member) { |
||
| 405 | |||
| 406 | |||
| 407 | /** |
||
| 408 | * onGroupLink() |
||
| 409 | * |
||
| 410 | * Called when a group is linked to a circle. |
||
| 411 | * Broadcast an activity to the member of the linked group and to the moderators of the circle. |
||
| 412 | * We won't do anything if the circle is PERSONAL |
||
| 413 | * |
||
| 414 | * @param Circle $circle |
||
| 415 | * @param Member $group |
||
| 416 | */ |
||
| 417 | View Code Duplication | public function onGroupLink(Circle $circle, Member $group) { |
|
| 437 | |||
| 438 | |||
| 439 | /** |
||
| 440 | * onGroupUnlink() |
||
| 441 | * |
||
| 442 | * Called when a group is unlinked from a circle. |
||
| 443 | * Broadcast an activity to the member of the unlinked group and to the moderators of the |
||
| 444 | * circle. We won't do anything if the circle is PERSONAL |
||
| 445 | * |
||
| 446 | * @param Circle $circle |
||
| 447 | * @param Member $group |
||
| 448 | */ |
||
| 449 | View Code Duplication | public function onGroupUnlink(Circle $circle, Member $group) { |
|
| 469 | |||
| 470 | |||
| 471 | /** |
||
| 472 | * onGroupLevel() |
||
| 473 | * |
||
| 474 | * Called when a linked group have his level changed. |
||
| 475 | * Broadcast an activity to all moderator of the circle, and the group members in case of |
||
| 476 | * demotion. |
||
| 477 | * |
||
| 478 | * @param Circle $circle |
||
| 479 | * @param Member $group |
||
| 480 | */ |
||
| 481 | View Code Duplication | public function onGroupLevel(Circle $circle, Member $group) { |
|
| 501 | |||
| 502 | |||
| 503 | /** |
||
| 504 | * onLinkRequestSent() |
||
| 505 | * |
||
| 506 | * Called when a request to generate a link with a remote circle is sent. |
||
| 507 | * Broadcast an activity to the moderators of the circle. |
||
| 508 | * |
||
| 509 | * @param Circle $circle |
||
| 510 | * @param FederatedLink $link |
||
| 511 | */ |
||
| 512 | View Code Duplication | public function onLinkRequestSent(Circle $circle, FederatedLink $link) { |
|
| 525 | |||
| 526 | |||
| 527 | /** |
||
| 528 | * onLinkRequestReceived() |
||
| 529 | * |
||
| 530 | * Called when a request to generate a link from a remote host is received. |
||
| 531 | * Broadcast an activity to the moderators of the circle. |
||
| 532 | * |
||
| 533 | * @param Circle $circle |
||
| 534 | * @param FederatedLink $link |
||
| 535 | */ |
||
| 536 | View Code Duplication | public function onLinkRequestReceived(Circle $circle, FederatedLink $link) { |
|
| 549 | |||
| 550 | |||
| 551 | /** |
||
| 552 | * onLinkRequestRejected() |
||
| 553 | * |
||
| 554 | * Called when a request to generate a link from a remote host is dismissed. |
||
| 555 | * Broadcast an activity to the moderators of the circle. |
||
| 556 | * |
||
| 557 | * @param Circle $circle |
||
| 558 | * @param FederatedLink $link |
||
| 559 | */ |
||
| 560 | View Code Duplication | public function onLinkRequestRejected(Circle $circle, FederatedLink $link) { |
|
| 573 | |||
| 574 | |||
| 575 | /** |
||
| 576 | * onLinkRequestCanceled() |
||
| 577 | * |
||
| 578 | * Called when a request to generate a link from a remote host is dismissed. |
||
| 579 | * Broadcast an activity to the moderators of the circle. |
||
| 580 | * |
||
| 581 | * @param Circle $circle |
||
| 582 | * @param FederatedLink $link |
||
| 583 | */ |
||
| 584 | View Code Duplication | public function onLinkRequestCanceled(Circle $circle, FederatedLink $link) { |
|
| 597 | |||
| 598 | |||
| 599 | /** |
||
| 600 | * onLinkRequestAccepted() |
||
| 601 | * |
||
| 602 | * Called when a request to generate a link from a remote host is accepted. |
||
| 603 | * Broadcast an activity to the moderators of the circle. |
||
| 604 | * |
||
| 605 | * @param Circle $circle |
||
| 606 | * @param FederatedLink $link |
||
| 607 | */ |
||
| 608 | View Code Duplication | public function onLinkRequestAccepted(Circle $circle, FederatedLink $link) { |
|
| 621 | |||
| 622 | |||
| 623 | /** |
||
| 624 | * onLinkRequestAccepting() |
||
| 625 | * |
||
| 626 | * Called when a link is Up and Running. |
||
| 627 | * Broadcast an activity to the moderators of the circle. |
||
| 628 | * |
||
| 629 | * @param Circle $circle |
||
| 630 | * @param FederatedLink $link |
||
| 631 | */ |
||
| 632 | View Code Duplication | public function onLinkRequestAccepting(Circle $circle, FederatedLink $link) { |
|
| 645 | |||
| 646 | |||
| 647 | /** |
||
| 648 | * onLinkUp() |
||
| 649 | * |
||
| 650 | * Called when a link is Up and Running. |
||
| 651 | * Broadcast an activity to the moderators of the circle. |
||
| 652 | * |
||
| 653 | * @param Circle $circle |
||
| 654 | * @param FederatedLink $link |
||
| 655 | */ |
||
| 656 | View Code Duplication | public function onLinkUp(Circle $circle, FederatedLink $link) { |
|
| 669 | |||
| 670 | |||
| 671 | /** |
||
| 672 | * onLinkDown() |
||
| 673 | * |
||
| 674 | * Called when a link is closed (usually by remote). |
||
| 675 | * Broadcast an activity to the moderators of the circle. |
||
| 676 | * |
||
| 677 | * @param Circle $circle |
||
| 678 | * @param FederatedLink $link |
||
| 679 | */ |
||
| 680 | View Code Duplication | public function onLinkDown(Circle $circle, FederatedLink $link) { |
|
| 693 | |||
| 694 | |||
| 695 | /** |
||
| 696 | * onLinkRemove() |
||
| 697 | * |
||
| 698 | * Called when a link is removed. |
||
| 699 | * Subject is based on the current status of the Link. |
||
| 700 | * Broadcast an activity to the moderators of the circle. |
||
| 701 | * |
||
| 702 | * @param Circle $circle |
||
| 703 | * @param FederatedLink $link |
||
| 704 | */ |
||
| 705 | public function onLinkRemove(Circle $circle, FederatedLink $link) { |
||
| 729 | |||
| 730 | /** |
||
| 731 | * onSettingsChange() |
||
| 732 | * |
||
| 733 | * Called when the circle's settings are changed |
||
| 734 | * |
||
| 735 | * @param Circle $circle |
||
| 736 | * @param array $oldSettings |
||
| 737 | */ |
||
| 738 | public function onSettingsChange(Circle $circle, array $oldSettings = []) { |
||
| 743 | |||
| 744 | |||
| 745 | /** |
||
| 746 | * generateEvent() |
||
| 747 | * Create an Activity Event with the basic settings for the app. |
||
| 748 | * |
||
| 749 | * @param $type |
||
| 750 | * |
||
| 751 | * @return \OCP\Activity\IEvent |
||
| 752 | */ |
||
| 753 | private function generateEvent($type) { |
||
| 764 | |||
| 765 | |||
| 766 | /** |
||
| 767 | * Publish the event to the users. |
||
| 768 | * |
||
| 769 | * @param IEvent $event |
||
| 770 | * @param array $users |
||
| 771 | */ |
||
| 772 | private function publishEvent(IEvent $event, array $users) { |
||
| 786 | |||
| 787 | |||
| 788 | /** |
||
| 789 | * @param Circle $circle |
||
| 790 | * @param Member $member |
||
| 791 | */ |
||
| 792 | private function notificationOnInvitation(Circle $circle, Member $member) { |
||
| 822 | |||
| 823 | /** |
||
| 824 | * @param Circle $circle |
||
| 825 | * @param Member $author |
||
| 826 | */ |
||
| 827 | private function notificationOnRequest(Circle $circle, Member $author) { |
||
| 855 | |||
| 856 | |||
| 857 | /** |
||
| 858 | * @param Circle $circle |
||
| 859 | * @param Member $member |
||
| 860 | */ |
||
| 861 | private function notificationOnMemberNew(Circle $circle, Member $member) { |
||
| 885 | |||
| 886 | |||
| 887 | /** |
||
| 888 | * @param string $object |
||
| 889 | * @param string $objectId |
||
| 890 | */ |
||
| 891 | public function deleteNotification(string $object, string $objectId) { |
||
| 902 | |||
| 903 | |||
| 904 | /** |
||
| 905 | * @param Circle $circle |
||
| 906 | * @param Member $author |
||
| 907 | * @param string $userId |
||
| 908 | * @param string $subject |
||
| 909 | * @param string $object |
||
| 910 | * @param string $objectId |
||
| 911 | * |
||
| 912 | * @return INotification |
||
| 913 | */ |
||
| 914 | private function createNotification( |
||
| 929 | |||
| 930 | |||
| 931 | /** |
||
| 932 | * @param string $context |
||
| 933 | * @param array $arguments |
||
| 934 | */ |
||
| 935 | private function dispatch($context, $arguments) { |
||
| 938 | |||
| 939 | } |
||
| 940 |
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.