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 |
||
| 38 | class EventsService { |
||
| 39 | |||
| 40 | |||
| 41 | /** @var string */ |
||
| 42 | private $userId; |
||
| 43 | |||
| 44 | /** @var IManager */ |
||
| 45 | private $activityManager; |
||
| 46 | |||
| 47 | /** @var IUserManager */ |
||
| 48 | private $userManager; |
||
| 49 | |||
| 50 | /** @var CirclesRequest */ |
||
| 51 | private $circlesRequest; |
||
| 52 | |||
| 53 | /** @var MiscService */ |
||
| 54 | private $miscService; |
||
| 55 | |||
| 56 | |||
| 57 | /** |
||
| 58 | * Events constructor. |
||
| 59 | * |
||
| 60 | * @param string $userId |
||
| 61 | * @param IManager $activityManager |
||
| 62 | * @param IUserManager $userManager |
||
| 63 | * @param CirclesRequest $circlesRequest |
||
| 64 | * @param MiscService $miscService |
||
| 65 | */ |
||
| 66 | public function __construct( |
||
| 67 | $userId, IManager $activityManager, IUserManager $userManager, |
||
| 68 | CirclesRequest $circlesRequest, MiscService $miscService |
||
| 69 | ) { |
||
| 70 | $this->userId = $userId; |
||
| 71 | $this->activityManager = $activityManager; |
||
| 72 | $this->userManager = $userManager; |
||
| 73 | $this->circlesRequest = $circlesRequest; |
||
| 74 | $this->miscService = $miscService; |
||
| 75 | } |
||
| 76 | |||
| 77 | |||
| 78 | /** |
||
| 79 | * onCircleCreation() |
||
| 80 | * |
||
| 81 | * Called when a circle is created. |
||
| 82 | * Broadcast an activity to the cloud |
||
| 83 | * We won't do anything if the circle is not PUBLIC or PRIVATE |
||
| 84 | * |
||
| 85 | * @param Circle $circle |
||
| 86 | */ |
||
| 87 | public function onCircleCreation(Circle $circle) { |
||
| 88 | if ($circle->getType() !== Circle::CIRCLES_PUBLIC |
||
| 89 | && $circle->getType() !== Circle::CIRCLES_PRIVATE |
||
| 90 | ) { |
||
| 91 | return; |
||
| 92 | } |
||
| 93 | |||
| 94 | $event = $this->generateEvent('circles_as_member'); |
||
| 95 | $event->setSubject('circle_create', ['circle' => json_encode($circle)]); |
||
| 96 | |||
| 97 | $this->userManager->callForSeenUsers( |
||
| 98 | function($user) use ($event) { |
||
| 99 | /** @var IUser $user */ |
||
| 100 | $this->publishEvent($event, [$user]); |
||
| 101 | } |
||
| 102 | ); |
||
| 103 | } |
||
| 104 | |||
| 105 | |||
| 106 | /** |
||
| 107 | * onCircleDestruction() |
||
| 108 | * |
||
| 109 | * Called when a circle is destroyed. |
||
| 110 | * Broadcast an activity on its members. |
||
| 111 | * We won't do anything if the circle is PERSONAL |
||
| 112 | * |
||
| 113 | * @param Circle $circle |
||
| 114 | */ |
||
| 115 | View Code Duplication | public function onCircleDestruction(Circle $circle) { |
|
|
|
|||
| 116 | if ($circle->getType() === Circle::CIRCLES_PERSONAL) { |
||
| 117 | return; |
||
| 118 | } |
||
| 119 | |||
| 120 | $event = $this->generateEvent('circles_as_member'); |
||
| 121 | $event->setSubject('circle_delete', ['circle' => json_encode($circle)]); |
||
| 122 | $this->publishEvent( |
||
| 123 | $event, $this->circlesRequest->getMembers($circle->getId(), Member::LEVEL_MEMBER) |
||
| 124 | ); |
||
| 125 | } |
||
| 126 | |||
| 127 | |||
| 128 | /** |
||
| 129 | * onMemberNew() |
||
| 130 | * |
||
| 131 | * Called when a member is added to a circle. |
||
| 132 | * Broadcast an activity to the new member and to the moderators of the circle. |
||
| 133 | * We won't do anything if the circle is PERSONAL |
||
| 134 | * If the level is still 0, we will redirect to onMemberAlmost and manage the |
||
| 135 | * invitation/request from there |
||
| 136 | * If the level is Owner, we ignore the event. |
||
| 137 | * |
||
| 138 | * @param Circle $circle |
||
| 139 | * @param Member $member |
||
| 140 | */ |
||
| 141 | public function onMemberNew(Circle $circle, Member $member) { |
||
| 164 | |||
| 165 | |||
| 166 | /** |
||
| 167 | * onMemberAlmost() |
||
| 168 | * |
||
| 169 | * Called when a member is added to a circle with level=0 |
||
| 170 | * Trigger onMemberInvitation() or onMemberInvitationRequest() based on Member Status |
||
| 171 | * |
||
| 172 | * @param Circle $circle |
||
| 173 | * @param Member $member |
||
| 174 | */ |
||
| 175 | private function onMemberAlmost(Circle $circle, Member $member) { |
||
| 189 | |||
| 190 | |||
| 191 | /** |
||
| 192 | * onMemberInvitation() |
||
| 193 | * |
||
| 194 | * Called when a member is invited to a circle. |
||
| 195 | * Broadcast an activity to the invited member and to the moderators of the circle. |
||
| 196 | * |
||
| 197 | * @param Circle $circle |
||
| 198 | * @param Member $member |
||
| 199 | */ |
||
| 200 | View Code Duplication | public function onMemberInvitation(Circle $circle, Member $member) { |
|
| 217 | |||
| 218 | |||
| 219 | /** |
||
| 220 | * onMemberInvitationRequest() |
||
| 221 | * |
||
| 222 | * Called when a member request an invitation to a private circle. |
||
| 223 | * Broadcast an activity to the requester and to the moderators of the circle. |
||
| 224 | * |
||
| 225 | * @param Circle $circle |
||
| 226 | * @param Member $member |
||
| 227 | */ |
||
| 228 | View Code Duplication | public function onMemberInvitationRequest(Circle $circle, Member $member) { |
|
| 247 | |||
| 248 | |||
| 249 | /** |
||
| 250 | * onCircleMemberLeaving() |
||
| 251 | * |
||
| 252 | * Called when a member is removed from a circle. |
||
| 253 | * Broadcast an activity to the new member and to the moderators of the circle. |
||
| 254 | * We won't do anything if the circle is PERSONAL |
||
| 255 | * |
||
| 256 | * @param Circle $circle |
||
| 257 | * @param Member $member |
||
| 258 | */ |
||
| 259 | public function onMemberLeaving(Circle $circle, Member $member) { |
||
| 278 | |||
| 279 | |||
| 280 | /** |
||
| 281 | * onMemberNew() |
||
| 282 | * |
||
| 283 | * Called when a member have his level changed. |
||
| 284 | * Broadcast an activity to all moderator of the circle. |
||
| 285 | * We won't do anything if the circle is PERSONAL |
||
| 286 | * If the level is Owner, we identify the event as a Coup d'Etat and we broadcast all members. |
||
| 287 | * |
||
| 288 | * @param Circle $circle |
||
| 289 | * @param Member $member |
||
| 290 | */ |
||
| 291 | public function onMemberLevel(Circle $circle, Member $member) { |
||
| 315 | |||
| 316 | |||
| 317 | /** |
||
| 318 | * onMemberOwner() |
||
| 319 | * |
||
| 320 | * Called when the owner rights of a circle have be given to another member. |
||
| 321 | * |
||
| 322 | * @param Circle $circle |
||
| 323 | * @param Member $member |
||
| 324 | */ |
||
| 325 | View Code Duplication | private function onMemberOwner(Circle $circle, Member $member) { |
|
| 338 | |||
| 339 | |||
| 340 | /** |
||
| 341 | * onLinkRequestSent() |
||
| 342 | * |
||
| 343 | * Called when a request to generate a link with a remote circle is sent. |
||
| 344 | * |
||
| 345 | * @param Circle $circle |
||
| 346 | * @param FederatedLink $link |
||
| 347 | */ |
||
| 348 | View Code Duplication | public function onLinkRequestSent(Circle $circle, FederatedLink $link) { |
|
| 361 | |||
| 362 | |||
| 363 | /** |
||
| 364 | * onLinkRequestReceived() |
||
| 365 | * |
||
| 366 | * Called when a request to generate a link from a remote host is received. |
||
| 367 | * |
||
| 368 | * @param Circle $circle |
||
| 369 | * @param FederatedLink $link |
||
| 370 | */ |
||
| 371 | View Code Duplication | public function onLinkRequestReceived(Circle $circle, FederatedLink $link) { |
|
| 384 | |||
| 385 | |||
| 386 | /** |
||
| 387 | * onLinkRequestRejected() |
||
| 388 | * |
||
| 389 | * Called when a request to generate a link from a remote host is dismissed. |
||
| 390 | * |
||
| 391 | * @param Circle $circle |
||
| 392 | * @param FederatedLink $link |
||
| 393 | */ |
||
| 394 | View Code Duplication | public function onLinkRequestRejected(Circle $circle, FederatedLink $link) { |
|
| 407 | |||
| 408 | |||
| 409 | /** |
||
| 410 | * onLinkRequestRejected() |
||
| 411 | * |
||
| 412 | * Called when a request to generate a link from a remote host is dismissed. |
||
| 413 | * |
||
| 414 | * @param Circle $circle |
||
| 415 | * @param FederatedLink $link |
||
| 416 | */ |
||
| 417 | View Code Duplication | public function onLinkRequestCanceled(Circle $circle, FederatedLink $link) { |
|
| 430 | |||
| 431 | |||
| 432 | /** |
||
| 433 | * onLinkRequestAccepted() |
||
| 434 | * |
||
| 435 | * Called when a request to generate a link from a remote host is accepted. |
||
| 436 | * |
||
| 437 | * @param Circle $circle |
||
| 438 | * @param FederatedLink $link |
||
| 439 | */ |
||
| 440 | View Code Duplication | public function onLinkRequestAccepted(Circle $circle, FederatedLink $link) { |
|
| 453 | |||
| 454 | |||
| 455 | /** |
||
| 456 | * onLinkUp() |
||
| 457 | * |
||
| 458 | * Called when a link is Up and Running. |
||
| 459 | * |
||
| 460 | * @param Circle $circle |
||
| 461 | * @param FederatedLink $link |
||
| 462 | */ |
||
| 463 | View Code Duplication | public function onLinkRequestAccepting(Circle $circle, FederatedLink $link) { |
|
| 476 | |||
| 477 | |||
| 478 | /** |
||
| 479 | * onLinkUp() |
||
| 480 | * |
||
| 481 | * Called when a link is Up and Running. |
||
| 482 | * |
||
| 483 | * @param Circle $circle |
||
| 484 | * @param FederatedLink $link |
||
| 485 | */ |
||
| 486 | View Code Duplication | public function onLinkUp(Circle $circle, FederatedLink $link) { |
|
| 499 | |||
| 500 | |||
| 501 | /** |
||
| 502 | * onLinkDown() |
||
| 503 | * |
||
| 504 | * Called when a link is closed (usually by remote). |
||
| 505 | * |
||
| 506 | * @param Circle $circle |
||
| 507 | * @param FederatedLink $link |
||
| 508 | */ |
||
| 509 | View Code Duplication | public function onLinkDown(Circle $circle, FederatedLink $link) { |
|
| 522 | |||
| 523 | |||
| 524 | /** |
||
| 525 | * onLinkRemove() |
||
| 526 | * |
||
| 527 | * Called when a link is removed. |
||
| 528 | * |
||
| 529 | * @param Circle $circle |
||
| 530 | * @param FederatedLink $link |
||
| 531 | */ |
||
| 532 | public function onLinkRemove(Circle $circle, FederatedLink $link) { |
||
| 550 | |||
| 551 | |||
| 552 | /** |
||
| 553 | * generateEvent() |
||
| 554 | * Create an Activity Event with the basic settings for the app. |
||
| 555 | * |
||
| 556 | * @param $type |
||
| 557 | * |
||
| 558 | * @return \OCP\Activity\IEvent |
||
| 559 | */ |
||
| 560 | private function generateEvent($type) { |
||
| 568 | |||
| 569 | |||
| 570 | private function publishEvent(IEvent $event, array $users) { |
||
| 584 | |||
| 585 | |||
| 586 | } |
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.