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 Provider 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 Provider, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 23 | class Provider implements IProvider { |
||
| 24 | |||
| 25 | /** @var MiscService */ |
||
| 26 | protected $miscService; |
||
| 27 | |||
| 28 | /** @var IL10N */ |
||
| 29 | protected $l10n; |
||
| 30 | |||
| 31 | /** @var IURLGenerator */ |
||
| 32 | protected $url; |
||
| 33 | |||
| 34 | /** @var IManager */ |
||
| 35 | protected $activityManager; |
||
| 36 | |||
| 37 | public function __construct( |
||
| 45 | |||
| 46 | |||
| 47 | /** |
||
| 48 | * @param string $lang |
||
| 49 | * @param IEvent $event |
||
| 50 | * @param IEvent|null $previousEvent |
||
| 51 | * |
||
| 52 | * @return IEvent |
||
| 53 | */ |
||
| 54 | public function parse($lang, IEvent $event, IEvent $previousEvent = null) { |
||
| 55 | |||
| 56 | if ($event->getApp() !== 'circles') { |
||
| 57 | throw new \InvalidArgumentException(); |
||
| 58 | } |
||
| 59 | |||
| 60 | try { |
||
| 61 | |||
| 62 | $params = $event->getSubjectParameters(); |
||
| 63 | $circle = Circle::fromJSON($this->l10n, $params['circle']); |
||
| 64 | |||
| 65 | $this->setIcon($event, $circle); |
||
| 66 | $this->parseAsMember($event, $circle, $params); |
||
| 67 | $this->parseAsModerator($event, $circle, $params); |
||
| 68 | $this->generateParsedSubject($event); |
||
| 69 | |||
| 70 | return $event; |
||
| 71 | } catch (\Exception $e) { |
||
| 72 | throw new \InvalidArgumentException(); |
||
| 73 | } |
||
| 74 | } |
||
| 75 | |||
| 76 | |||
| 77 | private function setIcon(IEvent &$event, Circle $circle) { |
||
| 78 | $event->setIcon( |
||
| 79 | CirclesService::getCircleIcon( |
||
| 80 | $circle->getType(), |
||
| 81 | (method_exists($this->activityManager, 'getRequirePNG') |
||
| 82 | && $this->activityManager->getRequirePNG()) |
||
| 83 | ) |
||
| 84 | ); |
||
| 85 | } |
||
| 86 | |||
| 87 | /** |
||
| 88 | * @param Circle $circle |
||
| 89 | * @param IEvent $event |
||
| 90 | * @param array $params |
||
| 91 | * |
||
| 92 | * @return IEvent |
||
| 93 | */ |
||
| 94 | private function parseAsMember(IEvent &$event, Circle $circle, $params) { |
||
| 121 | |||
| 122 | |||
| 123 | /** |
||
| 124 | * @param Circle $circle |
||
| 125 | * @param IEvent $event |
||
| 126 | * |
||
| 127 | * @return IEvent |
||
| 128 | */ |
||
| 129 | private function parseMemberAsMember(IEvent &$event, Circle $circle) { |
||
| 149 | |||
| 150 | |||
| 151 | /** |
||
| 152 | * Parse on Subject 'member_join'. |
||
| 153 | * If circle is private, we say that user accepted his invitation. |
||
| 154 | * |
||
| 155 | * @param IEvent $event |
||
| 156 | * @param Circle $circle |
||
| 157 | * @param Member $member |
||
| 158 | * |
||
| 159 | * @return IEvent |
||
| 160 | */ |
||
| 161 | View Code Duplication | private function parseSubjectMemberJoin(IEvent &$event, Circle $circle, Member $member) { |
|
| 176 | |||
| 177 | |||
| 178 | /** |
||
| 179 | * Parse on Subject 'member_add'. |
||
| 180 | * If circle is private, we say that user's invitation was accepted. |
||
| 181 | * |
||
| 182 | * @param IEvent $event |
||
| 183 | * @param Circle $circle |
||
| 184 | * @param Member $member |
||
| 185 | * |
||
| 186 | * @return IEvent |
||
| 187 | */ |
||
| 188 | View Code Duplication | private function parseSubjectMemberAdd(IEvent &$event, Circle $circle, Member $member) { |
|
| 205 | |||
| 206 | |||
| 207 | /** |
||
| 208 | * Parse on Subject 'member_left'. |
||
| 209 | * If circle is private and member was not a real member, we say that member rejected his |
||
| 210 | * invitation. |
||
| 211 | * |
||
| 212 | * @param IEvent $event |
||
| 213 | * @param Circle $circle |
||
| 214 | * @param Member $member |
||
| 215 | * |
||
| 216 | * @return IEvent |
||
| 217 | */ |
||
| 218 | View Code Duplication | private function parseSubjectMemberLeft(IEvent &$event, Circle $circle, Member $member) { |
|
| 234 | |||
| 235 | |||
| 236 | /** |
||
| 237 | * Parse on Subject 'member_remove'. |
||
| 238 | * If circle is private and member was not a real member, we say that his invitation was |
||
| 239 | * rejected. |
||
| 240 | * |
||
| 241 | * @param IEvent $event |
||
| 242 | * @param Circle $circle |
||
| 243 | * @param Member $member |
||
| 244 | * |
||
| 245 | * @return IEvent |
||
| 246 | */ |
||
| 247 | View Code Duplication | private function parseSubjectMemberRemove(IEvent &$event, Circle $circle, Member $member) { |
|
| 265 | |||
| 266 | |||
| 267 | /** |
||
| 268 | * @param Circle $circle |
||
| 269 | * @param IEvent $event |
||
| 270 | * @param array $params |
||
| 271 | * |
||
| 272 | * @return IEvent |
||
| 273 | * @throws Exception |
||
| 274 | */ |
||
| 275 | private function parseAsModerator(IEvent &$event, Circle $circle, $params) { |
||
| 298 | |||
| 299 | |||
| 300 | /** |
||
| 301 | * @param Circle $circle |
||
| 302 | * @param IEvent $event |
||
| 303 | * |
||
| 304 | * @return IEvent |
||
| 305 | */ |
||
| 306 | private function parseGroupAsModerator(IEvent &$event, Circle $circle) { |
||
| 345 | |||
| 346 | |||
| 347 | /** |
||
| 348 | * @param Circle $circle |
||
| 349 | * @param IEvent $event |
||
| 350 | * |
||
| 351 | * @return IEvent |
||
| 352 | */ |
||
| 353 | private function parseMemberAsModerator(IEvent &$event, Circle $circle) { |
||
| 394 | |||
| 395 | |||
| 396 | /** |
||
| 397 | * @param Circle $circle |
||
| 398 | * @param IEvent $event |
||
| 399 | * |
||
| 400 | * @return IEvent |
||
| 401 | */ |
||
| 402 | private function parseLinkAsModerator(IEvent &$event, Circle $circle) { |
||
| 487 | |||
| 488 | |||
| 489 | /** |
||
| 490 | * general function to generate Circle event. |
||
| 491 | * |
||
| 492 | * @param IEvent $event |
||
| 493 | * @param Circle $circle |
||
| 494 | * @param FederatedLink $remote |
||
| 495 | * @param string $ownEvent |
||
| 496 | * @param string $othersEvent |
||
| 497 | * |
||
| 498 | * @return IEvent |
||
| 499 | */ |
||
| 500 | View Code Duplication | private function parseCircleEvent( |
|
| 515 | |||
| 516 | |||
| 517 | /** |
||
| 518 | * general function to generate Member event. |
||
| 519 | * |
||
| 520 | * @param Circle $circle |
||
| 521 | * @param $member |
||
| 522 | * @param IEvent $event |
||
| 523 | * @param $ownEvent |
||
| 524 | * @param $othersEvent |
||
| 525 | * |
||
| 526 | * @return IEvent |
||
| 527 | */ |
||
| 528 | private function parseMemberEvent( |
||
| 543 | |||
| 544 | |||
| 545 | /** |
||
| 546 | * general function to generate Link event. |
||
| 547 | * |
||
| 548 | * @param Circle $circle |
||
| 549 | * @param FederatedLink $remote |
||
| 550 | * @param IEvent $event |
||
| 551 | * @param string $line |
||
| 552 | * |
||
| 553 | * @return IEvent |
||
| 554 | */ |
||
| 555 | private function parseLinkEvent(IEvent &$event, Circle $circle, FederatedLink $remote, $line) { |
||
| 563 | |||
| 564 | |||
| 565 | /** |
||
| 566 | * general function to generate Circle+Member event. |
||
| 567 | * |
||
| 568 | * @param Circle $circle |
||
| 569 | * @param Member $member |
||
| 570 | * @param IEvent $event |
||
| 571 | * @param string $ownEvent |
||
| 572 | * @param string $othersEvent |
||
| 573 | * |
||
| 574 | * @return IEvent |
||
| 575 | */ |
||
| 576 | View Code Duplication | private function parseCircleMemberEvent( |
|
| 592 | |||
| 593 | |||
| 594 | /** |
||
| 595 | * general function to generate Circle+Member advanced event. |
||
| 596 | * |
||
| 597 | * @param Circle $circle |
||
| 598 | * @param Member $member |
||
| 599 | * @param IEvent $event |
||
| 600 | *\ |
||
| 601 | * @param $ownEvent |
||
| 602 | * @param $targetEvent |
||
| 603 | * @param $othersEvent |
||
| 604 | * |
||
| 605 | * @return IEvent |
||
| 606 | */ |
||
| 607 | private function parseCircleMemberAdvancedEvent( |
||
| 627 | |||
| 628 | |||
| 629 | /** |
||
| 630 | * @param IEvent $event |
||
| 631 | */ |
||
| 632 | private function generateParsedSubject(IEvent &$event) { |
||
| 644 | |||
| 645 | |||
| 646 | /** |
||
| 647 | * @param Circle $circle |
||
| 648 | * @param string $userId |
||
| 649 | * |
||
| 650 | * @return bool |
||
| 651 | */ |
||
| 652 | private function isViewerTheAuthor(Circle $circle, $userId) { |
||
| 664 | |||
| 665 | |||
| 666 | /** |
||
| 667 | * @param Member $member |
||
| 668 | * |
||
| 669 | * @return array<string,string|integer> |
||
| 670 | */ |
||
| 671 | private function generateMemberParameter(Member $member) { |
||
| 678 | |||
| 679 | /** |
||
| 680 | * @param Circle $circle |
||
| 681 | * |
||
| 682 | * @return string|array <string,string|integer> |
||
| 683 | */ |
||
| 684 | private function generateViewerParameter(Circle $circle) { |
||
| 694 | |||
| 695 | |||
| 696 | /** |
||
| 697 | * @param Circle $circle |
||
| 698 | * |
||
| 699 | * @return array<string,string|integer> |
||
| 700 | */ |
||
| 701 | private function generateCircleParameter(Circle $circle) { |
||
| 710 | |||
| 711 | |||
| 712 | /** |
||
| 713 | * @param FederatedLink $link |
||
| 714 | * |
||
| 715 | * @return array<string,string|integer> |
||
| 716 | */ |
||
| 717 | private function generateRemoteCircleParameter(FederatedLink $link) { |
||
| 725 | |||
| 726 | |||
| 727 | /** |
||
| 728 | * @param $userId |
||
| 729 | * |
||
| 730 | * @return array<string,string|integer> |
||
| 731 | */ |
||
| 732 | private function generateUserParameter($userId) { |
||
| 740 | |||
| 741 | |||
| 742 | /** |
||
| 743 | * @param $groupId |
||
| 744 | * |
||
| 745 | * @return array<string,string|integer> |
||
| 746 | */ |
||
| 747 | private function generateGroupParameter($groupId) { |
||
| 755 | } |
||
| 756 |
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.