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 ElggUser 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 ElggUser, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class ElggUser extends \ElggEntity |
||
17 | implements Friendable { |
||
18 | |||
19 | /** |
||
20 | * {@inheritdoc} |
||
21 | */ |
||
22 | 504 | public function getType() { |
|
25 | |||
26 | /** |
||
27 | * Get user language or default to site language |
||
28 | * |
||
29 | * @param string $fallback If this is provided, it will be returned if the user doesn't have a language set. |
||
30 | * If null, the site language will be returned. |
||
31 | * |
||
32 | * @return string |
||
33 | */ |
||
34 | public function getLanguage($fallback = null) { |
||
43 | |||
44 | /** |
||
45 | * {@inheritdoc} |
||
46 | */ |
||
47 | 286 | public function __set($name, $value) { |
|
48 | switch ($name) { |
||
49 | 286 | case 'salt': |
|
50 | 286 | case 'password': |
|
51 | _elgg_services()->logger->error("User entities no longer contain {$name}"); |
||
52 | return; |
||
53 | 286 | case 'password_hash': |
|
54 | _elgg_services()->logger->error("password_hash is a readonly attribute."); |
||
55 | return; |
||
56 | 286 | case 'email': |
|
57 | 275 | if (!validate_email_address($value)) { |
|
58 | throw new \InvalidParameterException("Email is not a valid email address"); |
||
59 | } |
||
60 | 275 | break; |
|
61 | 286 | case 'username': |
|
62 | 286 | if (!validate_username($value)) { |
|
63 | throw new \InvalidParameterException("Username is not a valid username"); |
||
64 | } |
||
65 | 286 | $existing_user = get_user_by_username($value); |
|
66 | 286 | if ($existing_user && ($existing_user->guid !== $this->guid)) { |
|
67 | throw new \InvalidParameterException("{$name} is supposed to be unique for ElggUser"); |
||
68 | } |
||
69 | 286 | break; |
|
70 | } |
||
71 | |||
72 | 286 | parent::__set($name, $value); |
|
73 | 286 | } |
|
74 | |||
75 | /** |
||
76 | * {@inheritdoc} |
||
77 | */ |
||
78 | 3 | public function getURL() { |
|
79 | |||
80 | 3 | $result = parent::getURL(); |
|
81 | 3 | if ($result !== '') { |
|
82 | return $result; |
||
83 | } |
||
84 | |||
85 | 3 | return elgg_normalize_url("user/view/{$this->guid}"); |
|
86 | } |
||
87 | |||
88 | /** |
||
89 | * Ban this user. |
||
90 | * |
||
91 | * @param string $reason Optional reason |
||
92 | * |
||
93 | * @return bool |
||
94 | */ |
||
95 | 62 | View Code Duplication | public function ban($reason = '') { |
|
|||
96 | |||
97 | 62 | if (!$this->canEdit()) { |
|
98 | return false; |
||
99 | } |
||
100 | |||
101 | 62 | if (!elgg_trigger_event('ban', 'user', $this)) { |
|
102 | return false; |
||
103 | } |
||
104 | |||
105 | 62 | $this->ban_reason = $reason; |
|
1 ignored issue
–
show
|
|||
106 | 62 | $this->banned = 'yes'; |
|
107 | |||
108 | 62 | _elgg_invalidate_cache_for_entity($this->guid); |
|
109 | 62 | _elgg_invalidate_memcache_for_entity($this->guid); |
|
110 | |||
111 | 62 | return true; |
|
112 | } |
||
113 | |||
114 | /** |
||
115 | * Unban this user. |
||
116 | * |
||
117 | * @return bool |
||
118 | */ |
||
119 | 1 | View Code Duplication | public function unban() { |
120 | |||
121 | 1 | if (!$this->canEdit()) { |
|
122 | return false; |
||
123 | } |
||
124 | |||
125 | 1 | if (!elgg_trigger_event('unban', 'user', $this)) { |
|
126 | return false; |
||
127 | } |
||
128 | |||
129 | 1 | unset($this->ban_reason); |
|
130 | 1 | $this->banned = 'yes'; |
|
131 | |||
132 | 1 | _elgg_invalidate_cache_for_entity($this->guid); |
|
133 | 1 | _elgg_invalidate_memcache_for_entity($this->guid); |
|
134 | |||
135 | 1 | return true; |
|
136 | } |
||
137 | |||
138 | /** |
||
139 | * Is this user banned or not? |
||
140 | * |
||
141 | * @return bool |
||
142 | */ |
||
143 | 20 | public function isBanned() { |
|
146 | |||
147 | /** |
||
148 | * Is this user admin? |
||
149 | * |
||
150 | * @return bool |
||
151 | */ |
||
152 | 450 | public function isAdmin() { |
|
159 | |||
160 | /** |
||
161 | * Make the user an admin |
||
162 | * |
||
163 | * @return bool |
||
164 | */ |
||
165 | 4 | View Code Duplication | public function makeAdmin() { |
182 | |||
183 | /** |
||
184 | * Remove the admin flag for user |
||
185 | * |
||
186 | * @return bool |
||
187 | */ |
||
188 | 2 | View Code Duplication | public function removeAdmin() { |
205 | |||
206 | /** |
||
207 | * Sets the last logon time of the user to right now. |
||
208 | * |
||
209 | * @return void |
||
210 | */ |
||
211 | public function setLastLogin() { |
||
224 | |||
225 | /** |
||
226 | * Sets the last action time of the given user to right now. |
||
227 | * |
||
228 | * @see _elgg_session_boot The session boot calls this at the beginning of every request |
||
229 | * |
||
230 | * @return void |
||
231 | */ |
||
232 | public function setLastAction() { |
||
245 | |||
246 | /** |
||
247 | * Gets the validation status of a user. |
||
248 | * |
||
249 | * @return bool|null Null means status was not set for this user. |
||
250 | */ |
||
251 | public function isValidated() { |
||
257 | |||
258 | /** |
||
259 | * Set the validation status for a user. |
||
260 | * |
||
261 | * @param bool $status Validated (true) or unvalidated (false) |
||
262 | * @param string $method Optional method to say how a user was validated |
||
263 | * @return void |
||
264 | */ |
||
265 | 63 | public function setValidationStatus($status, $method = '') { |
|
276 | |||
277 | /** |
||
278 | * Adds a user as a friend |
||
279 | * |
||
280 | * @param int $friend_guid The GUID of the user to add |
||
281 | * @param bool $create_river_item Create the river item announcing this friendship |
||
282 | * |
||
283 | * @return bool |
||
284 | */ |
||
285 | 1 | public function addFriend($friend_guid, $create_river_item = false) { |
|
305 | |||
306 | /** |
||
307 | * Removes a user as a friend |
||
308 | * |
||
309 | * @param int $friend_guid The GUID of the user to remove |
||
310 | * @return bool |
||
311 | */ |
||
312 | public function removeFriend($friend_guid) { |
||
315 | |||
316 | /** |
||
317 | * Determines whether or not this user is a friend of the currently logged in user |
||
318 | * |
||
319 | * @return bool |
||
320 | */ |
||
321 | public function isFriend() { |
||
324 | |||
325 | /** |
||
326 | * Determines whether this user is friends with another user |
||
327 | * |
||
328 | * @param int $user_guid The GUID of the user to check against |
||
329 | * |
||
330 | * @return bool |
||
331 | */ |
||
332 | public function isFriendsWith($user_guid) { |
||
335 | |||
336 | /** |
||
337 | * Determines whether or not this user is another user's friend |
||
338 | * |
||
339 | * @param int $user_guid The GUID of the user to check against |
||
340 | * |
||
341 | * @return bool |
||
342 | */ |
||
343 | public function isFriendOf($user_guid) { |
||
346 | |||
347 | /** |
||
348 | * {@inheritdoc} |
||
349 | */ |
||
350 | View Code Duplication | public function getFriends(array $options = []) { |
|
357 | |||
358 | /** |
||
359 | * {@inheritdoc} |
||
360 | */ |
||
361 | View Code Duplication | public function getFriendsOf(array $options = []) { |
|
369 | |||
370 | /** |
||
371 | * Gets the user's groups |
||
372 | * |
||
373 | * @param array $options Options array. |
||
374 | * |
||
375 | * @return array|false Array of \ElggGroup, or false, depending on success |
||
376 | */ |
||
377 | public function getGroups(array $options = []) { |
||
384 | |||
385 | /** |
||
386 | * {@inheritdoc} |
||
387 | */ |
||
388 | public function getObjects(array $options = []) { |
||
394 | |||
395 | /** |
||
396 | * {@inheritdoc} |
||
397 | */ |
||
398 | View Code Duplication | public function getFriendsObjects(array $options = []) { |
|
406 | |||
407 | /** |
||
408 | * Get a user's owner GUID |
||
409 | * |
||
410 | * Returns it's own GUID if the user is not owned. |
||
411 | * |
||
412 | * @return int |
||
413 | */ |
||
414 | public function getOwnerGUID() { |
||
421 | |||
422 | /** |
||
423 | * {@inheritdoc} |
||
424 | */ |
||
425 | protected function prepareObject($object) { |
||
433 | |||
434 | /** |
||
435 | * Can a user comment on this user? |
||
436 | * |
||
437 | * @see \ElggEntity::canComment() |
||
438 | * |
||
439 | * @param int $user_guid User guid (default is logged in user) |
||
440 | * @param bool $default Default permission |
||
441 | * @return bool |
||
442 | * @since 1.8.0 |
||
443 | */ |
||
444 | 1 | public function canComment($user_guid = 0, $default = null) { |
|
451 | |||
452 | /** |
||
453 | * Set the necessary metadata to store a hash of the user's password. |
||
454 | * |
||
455 | * @param string $password The password to be hashed |
||
456 | * @return void |
||
457 | * @since 1.10.0 |
||
458 | */ |
||
459 | 63 | public function setPassword($password) { |
|
462 | |||
463 | /** |
||
464 | * Enable or disable a notification delivery method |
||
465 | * |
||
466 | * @param string $method Method name |
||
467 | * @param bool $enabled Enabled or disabled |
||
468 | * @return bool |
||
469 | */ |
||
470 | 64 | public function setNotificationSetting($method, $enabled = true) { |
|
474 | |||
475 | /** |
||
476 | * Returns users's notification settings |
||
477 | * <code> |
||
478 | * [ |
||
479 | * 'email' => true, // enabled |
||
480 | * 'ajax' => false, // disabled |
||
481 | * ] |
||
482 | * </code> |
||
483 | * |
||
484 | * @return array |
||
485 | */ |
||
486 | 19 | public function getNotificationSettings() { |
|
498 | } |
||
499 |
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.