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:
1 | <?php |
||
43 | class MembersService { |
||
44 | |||
45 | /** @var string */ |
||
46 | private $userId; |
||
47 | |||
48 | /** @var IL10N */ |
||
49 | private $l10n; |
||
50 | |||
51 | /** @var IUserManager */ |
||
52 | private $userManager; |
||
53 | |||
54 | /** @var ConfigService */ |
||
55 | private $configService; |
||
56 | |||
57 | /** @var CirclesMapper */ |
||
58 | private $dbCircles; |
||
59 | |||
60 | /** @var MembersMapper */ |
||
61 | private $dbMembers; |
||
62 | |||
63 | /** @var MiscService */ |
||
64 | private $miscService; |
||
65 | |||
66 | View Code Duplication | public function __construct( |
|
67 | $userId, |
||
68 | IL10N $l10n, |
||
69 | IUserManager $userManager, |
||
70 | ConfigService $configService, |
||
71 | DatabaseService $databaseService, |
||
72 | MiscService $miscService |
||
73 | ) { |
||
74 | $this->userId = $userId; |
||
75 | $this->l10n = $l10n; |
||
76 | $this->userManager = $userManager; |
||
77 | $this->configService = $configService; |
||
78 | $this->miscService = $miscService; |
||
79 | |||
80 | $this->dbCircles = $databaseService->getCirclesMapper(); |
||
81 | $this->dbMembers = $databaseService->getMembersMapper(); |
||
82 | } |
||
83 | |||
84 | |||
85 | /** |
||
86 | * @param $circleId |
||
87 | * @param $name |
||
88 | * |
||
89 | * @return array |
||
90 | * @throws CircleDoesNotExistException |
||
91 | * @throws MemberAlreadyExistsException |
||
92 | * @throws MemberDoesNotExistException |
||
93 | * @throws MemberIsNotModeratorException |
||
94 | * @throws NoUserException |
||
95 | */ |
||
96 | public function addMember($circleId, $name) { |
||
97 | |||
98 | if (!$this->userManager->userExists($name)) { |
||
99 | throw new NoUserException("The selected user does not exist"); |
||
100 | } |
||
101 | |||
102 | // we check that this->userId is moderator |
||
103 | try { |
||
104 | $this->dbMembers->getMemberFromCircle($circleId, $this->userId) |
||
105 | ->hasToBeModerator(); |
||
106 | } catch (MemberDoesNotExistException $e) { |
||
107 | throw $e; |
||
108 | } catch (MemberIsNotModeratorException $e) { |
||
109 | throw new MemberIsNotModeratorException("You are not moderator of this circle"); |
||
110 | } |
||
111 | |||
112 | try { |
||
113 | $member = $this->dbMembers->getMemberFromCircle($circleId, $name); |
||
114 | |||
115 | } catch (MemberDoesNotExistException $e) { |
||
116 | $member = new Member(); |
||
117 | $member->setCircleId($circleId); |
||
118 | $member->setUserId($name); |
||
119 | $member->setLevel(Member::LEVEL_NONE); |
||
120 | $member->setStatus(Member::STATUS_NONMEMBER); |
||
121 | |||
122 | $this->dbMembers->add($member); |
||
123 | } |
||
124 | |||
125 | try { |
||
126 | $circle = $this->dbCircles->getDetailsFromCircle($this->userId, $circleId); |
||
127 | } catch (CircleDoesNotExistException $e) { |
||
128 | throw $e; |
||
129 | } |
||
130 | |||
131 | if ($this->memberAlreadyExist($member)) { |
||
132 | throw new MemberAlreadyExistsException(); |
||
133 | } |
||
134 | |||
135 | $member->setCircleId($circleId); |
||
136 | $member->setUserId($name); |
||
137 | |||
138 | switch ($circle->getType()) { |
||
139 | case Circle::CIRCLES_PRIVATE: |
||
140 | $this->inviteMemberToPrivateCircle($member); |
||
141 | break; |
||
142 | |||
143 | default: |
||
144 | $this->addMemberToCircle($member); |
||
145 | break; |
||
146 | } |
||
147 | |||
148 | $this->dbMembers->editMember($member); |
||
149 | |||
150 | return $this->dbMembers->getMembersFromCircle($circleId, $circle->getUser()); |
||
151 | } |
||
152 | |||
153 | |||
154 | /** |
||
155 | * return if member already exists |
||
156 | * |
||
157 | * @param $member |
||
158 | * |
||
159 | * @return bool |
||
160 | */ |
||
161 | private function memberAlreadyExist($member) { |
||
162 | return ($member->getLevel() > Member::LEVEL_NONE |
||
163 | || ($member->getStatus() !== Member::STATUS_NONMEMBER |
||
164 | && $member->getStatus() !== Member::STATUS_REQUEST) |
||
165 | ); |
||
166 | } |
||
167 | |||
168 | /** |
||
169 | * Invite a Member to a private Circle, or accept his request. |
||
170 | * |
||
171 | * @param $member |
||
172 | */ |
||
173 | private function inviteMemberToPrivateCircle(&$member) { |
||
181 | |||
182 | /** |
||
183 | * add a member to a circle. |
||
184 | * |
||
185 | * @param $member |
||
186 | */ |
||
187 | private function addMemberToCircle(&$member) { |
||
191 | |||
192 | |||
193 | /** |
||
194 | * @param $circleId |
||
195 | * @param $name |
||
196 | * |
||
197 | * @return array |
||
198 | * @throws \Exception |
||
199 | */ |
||
200 | View Code Duplication | public function removeMember($circleId, $name) { |
|
221 | |||
222 | } |
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.