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 Team 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 Team, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
13 | class Team extends AvatarModel implements TeamInterface, DuplexUrlInterface, EloInterface |
||
14 | { |
||
15 | /** |
||
16 | * The description of the team written in markdown |
||
17 | * |
||
18 | * @var string |
||
19 | */ |
||
20 | protected $description; |
||
21 | |||
22 | /** |
||
23 | * The creation date of the team |
||
24 | * |
||
25 | * @var TimeDate |
||
26 | */ |
||
27 | protected $created; |
||
28 | |||
29 | /** |
||
30 | * The team's current elo |
||
31 | * |
||
32 | * @var int |
||
33 | */ |
||
34 | protected $elo; |
||
35 | |||
36 | /** |
||
37 | * The team's activity |
||
38 | * |
||
39 | * null if we haven't calculated it yet |
||
40 | * |
||
41 | * @var float|null |
||
42 | */ |
||
43 | protected $activity = null; |
||
44 | |||
45 | /** |
||
46 | * The id of the team leader |
||
47 | * |
||
48 | * @var int |
||
49 | */ |
||
50 | protected $leader; |
||
51 | |||
52 | /** |
||
53 | * The number of matches won |
||
54 | * |
||
55 | * @var int |
||
56 | */ |
||
57 | protected $matches_won; |
||
58 | |||
59 | /** |
||
60 | * The number of matches lost |
||
61 | * |
||
62 | * @var int |
||
63 | */ |
||
64 | protected $matches_lost; |
||
65 | |||
66 | /** |
||
67 | * The number of matches tied |
||
68 | * |
||
69 | * @var int |
||
70 | */ |
||
71 | protected $matches_draw; |
||
72 | |||
73 | /** |
||
74 | * The total number of matches |
||
75 | * |
||
76 | * @var int |
||
77 | */ |
||
78 | protected $matches_total; |
||
79 | |||
80 | /** |
||
81 | * The number of members |
||
82 | * |
||
83 | * @var int |
||
84 | */ |
||
85 | protected $members; |
||
86 | |||
87 | /** |
||
88 | * The team's status |
||
89 | * |
||
90 | * @var string |
||
91 | */ |
||
92 | protected $status; |
||
93 | |||
94 | const DEFAULT_STATUS = 'closed'; |
||
95 | |||
96 | /** |
||
97 | * The name of the database table used for queries |
||
98 | */ |
||
99 | const TABLE = "teams"; |
||
100 | |||
101 | /** |
||
102 | * The location where avatars will be stored |
||
103 | */ |
||
104 | const AVATAR_LOCATION = "/web/assets/imgs/avatars/teams/"; |
||
105 | |||
106 | const CREATE_PERMISSION = Permission::CREATE_TEAM; |
||
107 | const EDIT_PERMISSION = Permission::EDIT_TEAM; |
||
108 | const SOFT_DELETE_PERMISSION = Permission::SOFT_DELETE_TEAM; |
||
109 | const HARD_DELETE_PERMISSION = Permission::HARD_DELETE_TEAM; |
||
110 | |||
111 | /** |
||
112 | * {@inheritdoc} |
||
113 | */ |
||
114 | protected function assignResult($team) |
||
133 | 55 | ||
134 | /** |
||
135 | 55 | * Adds a new member to the team |
|
136 | 55 | * |
|
137 | * @param int $id The id of the player to add to the team |
||
138 | * |
||
139 | * @return bool|null True if both the player was added to the team AND the team member count was incremented |
||
140 | */ |
||
141 | View Code Duplication | public function addMember($id) |
|
152 | |||
153 | 55 | /** |
|
154 | 55 | * Increase or decrease the ELO of the team |
|
155 | 55 | * |
|
156 | * @param int $adjust The value to be added to the current ELO (negative to subtract) |
||
157 | * @param Match $match The match where this Elo change took place |
||
158 | */ |
||
159 | public function adjustElo($adjust, Match $match = null) |
||
164 | |||
165 | 33 | /** |
|
166 | 33 | * Change the ELO of the team |
|
167 | 33 | * |
|
168 | * @param int $elo The new team ELO |
||
169 | */ |
||
170 | public function setElo($elo) |
||
174 | 6 | ||
175 | /** |
||
176 | 6 | * Increment the team's match count |
|
177 | 6 | * |
|
178 | * @param int $adjust The number to add to the current matches number (negative to substract) |
||
179 | * @param string $type The match count that should be changed. Can be 'win', 'draw' or 'loss' |
||
180 | */ |
||
181 | public function changeMatchCount($adjust, $type) |
||
202 | |||
203 | 16 | /** |
|
204 | * Decrement the team's match count by one |
||
205 | * |
||
206 | * @param string $type The type of the match. Can be 'win', 'draw' or 'loss' |
||
207 | */ |
||
208 | public function decrementMatchCount($type) |
||
212 | |||
213 | /** |
||
214 | * Get the activity of the team |
||
215 | * |
||
216 | * @return float The team's activity |
||
217 | */ |
||
218 | public function getActivity() |
||
222 | 1 | ||
223 | /** |
||
224 | 1 | * Get the creation date of the team |
|
225 | * |
||
226 | 1 | * @return TimeDate The creation date of the team |
|
227 | */ |
||
228 | public function getCreationDate() |
||
232 | |||
233 | /** |
||
234 | 1 | * Get the description of the team |
|
235 | 1 | * |
|
236 | 1 | * @return string The description of the team |
|
237 | 1 | */ |
|
238 | public function getDescription() |
||
242 | 1 | ||
243 | /** |
||
244 | * Get the current elo of the team |
||
245 | * |
||
246 | * @return int The elo of the team |
||
247 | */ |
||
248 | public function getElo() |
||
252 | 2 | ||
253 | /** |
||
254 | * Get the leader of the team |
||
255 | * |
||
256 | * @return Player The object representing the team leader |
||
257 | */ |
||
258 | public function getLeader() |
||
262 | 1 | ||
263 | /** |
||
264 | * Get the matches this team has participated in |
||
265 | * |
||
266 | * @param string $matchType The filter for match types: "all", "wins", "losses", or "draws" |
||
267 | * @param int $count The amount of matches to be retrieved |
||
268 | * @param int $page The number of the page to return |
||
269 | * |
||
270 | 34 | * @return Match[] The array of match IDs this team has participated in |
|
271 | */ |
||
272 | 34 | public function getMatches($matchType = "all", $count = 5, $page = 1) |
|
281 | |||
282 | 2 | /** |
|
283 | * Get the number of matches that resulted as a draw |
||
284 | * |
||
285 | * @return int The number of matches, respectively |
||
286 | */ |
||
287 | public function getMatchesDraw() |
||
291 | |||
292 | /** |
||
293 | * Get the number of matches that the team has lost |
||
294 | 2 | * |
|
295 | * @return int The number of matches, respectively |
||
296 | 2 | */ |
|
297 | 2 | public function getMatchesLost() |
|
301 | 2 | ||
302 | /** |
||
303 | * Get the URL that points to the team's list of matches |
||
304 | * |
||
305 | * @return string The team's list of matches |
||
306 | */ |
||
307 | public function getMatchesURL() |
||
311 | 1 | ||
312 | /** |
||
313 | * Get the number of matches that the team has won |
||
314 | * |
||
315 | * @return int The number of matches, respectively |
||
316 | */ |
||
317 | public function getMatchesWon() |
||
321 | 1 | ||
322 | /** |
||
323 | * Get the members on the team |
||
324 | * |
||
325 | * @return Player[] The members on the team |
||
326 | */ |
||
327 | public function getMembers() |
||
349 | 2 | ||
350 | /** |
||
351 | 2 | * Get the name of the team |
|
352 | 2 | * |
|
353 | * @return string The name of the team |
||
354 | */ |
||
355 | public function getName() |
||
362 | |||
363 | /** |
||
364 | * Get the name of the team, safe for use in your HTML |
||
365 | * |
||
366 | * @return string The name of the team |
||
367 | 2 | */ |
|
368 | public function getEscapedName() |
||
375 | |||
376 | /** |
||
377 | 55 | * Get the number of members on the team |
|
378 | * |
||
379 | 55 | * @return int The number of members on the team |
|
380 | */ |
||
381 | public function getNumMembers() |
||
385 | |||
386 | /** |
||
387 | * Get the total number of matches this team has played |
||
388 | * |
||
389 | * @return int The total number of matches this team has played |
||
390 | 1 | */ |
|
391 | public function getNumTotalMatches() |
||
395 | 1 | ||
396 | /** |
||
397 | * Get the rank category a team belongs too based on their ELO |
||
398 | * |
||
399 | * This value is always a multiple of 100 and less than or equal to 2000 |
||
400 | * |
||
401 | * @return int The rank category a team belongs to |
||
402 | */ |
||
403 | 2 | public function getRankValue() |
|
407 | |||
408 | /** |
||
409 | * Get the HTML for an image with the rank symbol |
||
410 | * |
||
411 | * @return string The HTML for a rank image |
||
412 | */ |
||
413 | 2 | public function getRankImageLiteral() |
|
417 | |||
418 | /** |
||
419 | * Increment the team's match count by one |
||
420 | * |
||
421 | * @param string $type The type of the match. Can be 'win', 'draw' or 'loss' |
||
422 | */ |
||
423 | public function incrementMatchCount($type) |
||
427 | 1 | ||
428 | /** |
||
429 | * Check if a player is part of this team |
||
430 | * |
||
431 | * @param int $playerID The player to check |
||
432 | * |
||
433 | * @return bool True if the player belongs to this team |
||
434 | */ |
||
435 | 1 | public function isMember($playerID) |
|
441 | |||
442 | /** |
||
443 | * Removes a member from the team |
||
444 | * |
||
445 | * @param int $id The id of the player to remove |
||
446 | * @return void |
||
447 | */ |
||
448 | View Code Duplication | public function removeMember($id) |
|
459 | 2 | ||
460 | /** |
||
461 | 2 | * Update the description of the team |
|
462 | * |
||
463 | * @param string $description The description of the team written as markdown |
||
464 | * @return void |
||
465 | */ |
||
466 | public function setDescription($description) |
||
470 | 2 | ||
471 | /** |
||
472 | 2 | * Change the status of the team |
|
473 | * |
||
474 | * @param string $newStatus The new status of the team (open, closed, disabled or deleted) |
||
475 | * @return self |
||
476 | 2 | */ |
|
477 | public function setStatus($newStatus) |
||
481 | |||
482 | /** |
||
483 | * Change the leader of the team |
||
484 | * |
||
485 | * @param int $leader The ID of the new leader of the team |
||
486 | * @return self |
||
487 | */ |
||
488 | public function setLeader($leader) |
||
492 | |||
493 | /** |
||
494 | * Find if a specific match is the team's last one |
||
495 | * |
||
496 | * @param int|Match $match The match |
||
497 | * @return bool |
||
498 | */ |
||
499 | public function isLastMatch($match) |
||
508 | |||
509 | /** |
||
510 | * {@inheritdoc} |
||
511 | */ |
||
512 | public function delete() |
||
513 | { |
||
514 | parent::delete(); |
||
515 | |||
516 | // Remove all the members of a deleted team |
||
517 | $this->updateProperty($this->members, 'members', 0); |
||
518 | $this->db->execute('UPDATE players SET team = NULL WHERE team = ?', $this->id); |
||
519 | } |
||
520 | |||
521 | 1 | /** |
|
522 | * {@inheritdoc} |
||
523 | */ |
||
524 | 1 | public function supportsMatchCount() |
|
528 | 1 | ||
529 | /** |
||
530 | * Create a new team |
||
531 | * |
||
532 | * @param string $name The name of the team |
||
533 | * @param int $leader The ID of the person creating the team, also the leader |
||
534 | 1 | * @param string $avatar The URL to the team's avatar |
|
535 | * @param string $description The team's description |
||
536 | 1 | * @param string $status The team's status (open, closed, disabled or deleted) |
|
537 | * @param string|\TimeDate $created The date the team was created |
||
538 | * |
||
539 | 1 | * @return Team An object that represents the newly created team |
|
540 | 1 | */ |
|
541 | 1 | public static function createTeam($name, $leader, $avatar, $description, $status = 'closed', $created = "now") |
|
565 | 56 | ||
566 | /** |
||
567 | 56 | * Get all the teams in the database that are not disabled or deleted |
|
568 | 56 | * |
|
569 | 56 | * @return Team[] An array of Team IDs |
|
570 | 56 | */ |
|
571 | 56 | public static function getTeams() |
|
580 | 56 | ||
581 | /** |
||
582 | * Get a single team by its name |
||
583 | 55 | * |
|
584 | 55 | * @param string $name The team name to look for |
|
585 | * @return Team |
||
586 | 55 | */ |
|
587 | public static function getFromName($name) |
||
593 | |||
594 | /** |
||
595 | * Alphabetical order function for use in usort (case-insensitive) |
||
596 | * @return Closure The sort function |
||
597 | */ |
||
598 | public static function getAlphabeticalSort() |
||
604 | |||
605 | /** |
||
606 | * {@inheritdoc} |
||
607 | */ |
||
608 | public static function getActiveStatuses() |
||
612 | 1 | ||
613 | /** |
||
614 | 1 | * {@inheritdoc} |
|
615 | */ |
||
616 | public static function getEagerColumns($prefix = null) |
||
636 | |||
637 | /** |
||
638 | * Get a query builder for teams |
||
639 | * @return TeamQueryBuilder |
||
640 | 1 | */ |
|
641 | public static function getQueryBuilder() |
||
654 | |||
655 | /** |
||
656 | 1 | * {@inheritdoc} |
|
657 | */ |
||
658 | 1 | protected function isEditor($player) |
|
662 | } |
||
663 |
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.