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 Match 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 Match, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
14 | class Match extends UrlModel implements NamedModel |
||
15 | { |
||
16 | const OFFICIAL = "official"; |
||
17 | const SPECIAL = "special"; |
||
18 | const FUN = "fm"; |
||
19 | |||
20 | use Timestamp; |
||
21 | |||
22 | /** |
||
23 | * The ID of the first team of the match |
||
24 | * @var int |
||
25 | */ |
||
26 | protected $team_a; |
||
27 | |||
28 | /** |
||
29 | * The ID of the second team of the match |
||
30 | * @var int |
||
31 | */ |
||
32 | protected $team_b; |
||
33 | |||
34 | /** |
||
35 | * The match points (usually the number of flag captures) Team A scored |
||
36 | * @var int |
||
37 | */ |
||
38 | protected $team_a_points; |
||
39 | |||
40 | /** |
||
41 | * The match points Team B scored |
||
42 | * @var int |
||
43 | */ |
||
44 | protected $team_b_points; |
||
45 | |||
46 | /** |
||
47 | * The BZIDs of players part of Team A who participated in the match, separated by commas |
||
48 | * @var string |
||
49 | */ |
||
50 | protected $team_a_players; |
||
51 | |||
52 | /** |
||
53 | * The BZIDs of players part of Team B who participated in the match, separated by commas |
||
54 | * @var string |
||
55 | */ |
||
56 | protected $team_b_players; |
||
57 | |||
58 | /** |
||
59 | * The ELO score of Team A after the match |
||
60 | * @var int |
||
61 | */ |
||
62 | protected $team_a_elo_new; |
||
63 | |||
64 | /** |
||
65 | * The ELO score of Team B after the match |
||
66 | * @var int |
||
67 | */ |
||
68 | protected $team_b_elo_new; |
||
69 | |||
70 | /** |
||
71 | * The map ID used in the match if the league supports more than one map |
||
72 | * @var int |
||
73 | */ |
||
74 | protected $map; |
||
75 | |||
76 | /** |
||
77 | * A JSON string of events that happened during a match, such as captures and substitutions |
||
78 | * @var string |
||
79 | */ |
||
80 | protected $match_details; |
||
81 | |||
82 | /** |
||
83 | * The port of the server where the match took place |
||
84 | * @var int |
||
85 | */ |
||
86 | protected $port; |
||
87 | |||
88 | /** |
||
89 | * The server location of there the match took place |
||
90 | * @var string |
||
91 | */ |
||
92 | protected $server; |
||
93 | |||
94 | /** |
||
95 | * The file name of the replay file of the match |
||
96 | * @var string |
||
97 | */ |
||
98 | protected $replay_file; |
||
99 | |||
100 | /** |
||
101 | * The absolute value of the ELO score difference |
||
102 | * @var int |
||
103 | */ |
||
104 | protected $elo_diff; |
||
105 | |||
106 | /** |
||
107 | * The timestamp representing when the match information was last updated |
||
108 | * @var TimeDate |
||
109 | */ |
||
110 | protected $updated; |
||
111 | |||
112 | /** |
||
113 | * The duration of the match in minutes |
||
114 | * @var int |
||
115 | */ |
||
116 | protected $duration; |
||
117 | |||
118 | /** |
||
119 | * The ID of the person (i.e. referee) who last updated the match information |
||
120 | * @var string |
||
121 | */ |
||
122 | protected $entered_by; |
||
123 | |||
124 | /** |
||
125 | * The status of the match. Can be 'entered', 'disabled', 'deleted' or 'reported' |
||
126 | * @var string |
||
127 | */ |
||
128 | protected $status; |
||
129 | |||
130 | /** |
||
131 | * The name of the database table used for queries |
||
132 | */ |
||
133 | const TABLE = "matches"; |
||
134 | |||
135 | const CREATE_PERMISSION = Permission::ENTER_MATCH; |
||
136 | const EDIT_PERMISSION = Permission::EDIT_MATCH; |
||
137 | const SOFT_DELETE_PERMISSION = Permission::SOFT_DELETE_MATCH; |
||
138 | const HARD_DELETE_PERMISSION = Permission::HARD_DELETE_MATCH; |
||
139 | |||
140 | /** |
||
141 | * {@inheritdoc} |
||
142 | */ |
||
143 | 9 | protected function assignResult($match) |
|
165 | |||
166 | /** |
||
167 | * Get the name of the route that shows the object |
||
168 | * @param string $action The route's suffix |
||
169 | * @return string |
||
170 | */ |
||
171 | 1 | public static function getRouteName($action = 'show') |
|
175 | |||
176 | /** |
||
177 | * Get a one word description of a match relative to a team (i.e. win, loss, or draw) |
||
178 | * |
||
179 | * @param int $teamID The team ID we want the noun for |
||
180 | * |
||
181 | * @return string Either "win", "loss", or "draw" relative to the team |
||
182 | */ |
||
183 | 1 | public function getMatchDescription($teamID) |
|
193 | |||
194 | /** |
||
195 | * Get a one letter description of a match relative to a team (i.e. W, L, or T) |
||
196 | * |
||
197 | * @param int $teamID The team ID we want the noun for |
||
198 | * |
||
199 | * @return string Either "W", "L", or "T" relative to the team |
||
200 | */ |
||
201 | 1 | public function getMatchLetter($teamID) |
|
205 | |||
206 | /** |
||
207 | * Get the score of a specific team |
||
208 | * |
||
209 | * @param int|Team $teamID The team we want the score for |
||
210 | * |
||
211 | * @return int The score that team received |
||
212 | */ |
||
213 | 2 | View Code Duplication | public function getScore($teamID) |
226 | |||
227 | /** |
||
228 | * Get the score of the opponent relative to a team |
||
229 | * |
||
230 | * @param int $teamID The opponent of the team we want the score for |
||
231 | * |
||
232 | * @return int The score of the opponent |
||
233 | */ |
||
234 | 2 | View Code Duplication | public function getOpponentScore($teamID) |
246 | |||
247 | /** |
||
248 | * Get the opponent of a match relative to a team ID |
||
249 | * |
||
250 | * @param int $teamID The team who is known in a match |
||
251 | * |
||
252 | * @return Team The opponent team |
||
253 | */ |
||
254 | 8 | public function getOpponent($teamID) |
|
262 | |||
263 | /** |
||
264 | * Get the timestamp of the last update of the match |
||
265 | * |
||
266 | * @return TimeDate The match's update timestamp |
||
267 | */ |
||
268 | 1 | public function getUpdated() |
|
272 | |||
273 | /** |
||
274 | * Set the timestamp of the match |
||
275 | * |
||
276 | * @param mixed The match's new timestamp |
||
277 | * @return $this |
||
278 | */ |
||
279 | public function setTimestamp($timestamp) |
||
286 | |||
287 | /** |
||
288 | * Get the first team involved in the match |
||
289 | * @return Team Team A's id |
||
290 | */ |
||
291 | 9 | public function getTeamA() |
|
295 | |||
296 | /** |
||
297 | * Get the second team involved in the match |
||
298 | * @return Team Team B's id |
||
299 | */ |
||
300 | 9 | public function getTeamB() |
|
304 | |||
305 | /** |
||
306 | * Get the list of players on Team A who participated in this match |
||
307 | * @return Player[]|null Returns null if there were no players recorded for this match |
||
308 | */ |
||
309 | 1 | public function getTeamAPlayers() |
|
313 | |||
314 | /** |
||
315 | * Get the list of players on Team B who participated in this match |
||
316 | * @return Player[]|null Returns null if there were no players recorded for this match |
||
317 | */ |
||
318 | 1 | public function getTeamBPlayers() |
|
322 | |||
323 | /** |
||
324 | * Get the list of players for a team in a match |
||
325 | * @param Team|int The team or team ID |
||
326 | * @return Player[]|null Returns null if there were no players recorded for this match |
||
327 | */ |
||
328 | 1 | public function getPlayers($team) |
|
342 | |||
343 | /** |
||
344 | * Set the players of the match's teams |
||
345 | * |
||
346 | * @param int[] $teamAPlayers An array of player IDs |
||
347 | * @param int[] $teamBPlayers An array of player IDs |
||
348 | * @return self |
||
349 | */ |
||
350 | public function setTeamPlayers($teamAPlayers = array(), $teamBPlayers = array()) |
||
357 | |||
358 | /** |
||
359 | * Get an array of players based on a string representation |
||
360 | * @param string $playerString |
||
361 | * @return Player[]|null Returns null if there were no players recorded for this match |
||
362 | */ |
||
363 | 1 | private static function parsePlayers($playerString) |
|
371 | |||
372 | /** |
||
373 | * Get the first team's points |
||
374 | * @return int Team A's points |
||
375 | */ |
||
376 | 4 | public function getTeamAPoints() |
|
380 | |||
381 | /** |
||
382 | * Get the second team's points |
||
383 | * @return int Team B's points |
||
384 | */ |
||
385 | 4 | public function getTeamBPoints() |
|
389 | |||
390 | /** |
||
391 | * Set the match team points |
||
392 | * |
||
393 | * @param int $teamAPoints Team A's points |
||
394 | * @param int $teamBPoints Team B's points |
||
395 | * @return self |
||
396 | */ |
||
397 | public function setTeamPoints($teamAPoints, $teamBPoints) |
||
404 | |||
405 | /** |
||
406 | * Get the ELO difference applied to each team's old ELO |
||
407 | * @return int The ELO difference |
||
408 | */ |
||
409 | 7 | public function getEloDiff() |
|
413 | |||
414 | /** |
||
415 | * Get the first team's new ELO |
||
416 | * @return int Team A's new ELO |
||
417 | */ |
||
418 | 7 | public function getTeamAEloNew() |
|
422 | |||
423 | /** |
||
424 | * Get the second team's new ELO |
||
425 | * @return int Team B's new ELO |
||
426 | */ |
||
427 | 7 | public function getTeamBEloNew() |
|
431 | |||
432 | /** |
||
433 | * Get the first team's old ELO |
||
434 | * @return int |
||
435 | */ |
||
436 | 6 | public function getTeamAEloOld() |
|
440 | |||
441 | /** |
||
442 | * Get the second team's old ELO |
||
443 | * @return int |
||
444 | */ |
||
445 | 6 | public function getTeamBEloOld() |
|
449 | |||
450 | /** |
||
451 | * Get the team's new ELO |
||
452 | * @param Team $team The team whose new ELO to return |
||
453 | * @return int|null The new ELO, or null if the team provided has not |
||
454 | * participated in the match |
||
455 | */ |
||
456 | 1 | public function getTeamEloNew(Team $team) |
|
466 | |||
467 | /** |
||
468 | * Get the team's old ELO |
||
469 | * @param Team $team The team whose old ELO to return |
||
470 | * @return int|null The old ELO, or null if the team provided has not |
||
471 | * participated in the match |
||
472 | */ |
||
473 | 1 | public function getTeamEloOld(Team $team) |
|
483 | |||
484 | /** |
||
485 | * Get the map where the match was played on |
||
486 | * @return Map Returns an invalid map if no map was found |
||
487 | */ |
||
488 | 1 | public function getMap() |
|
492 | |||
493 | /** |
||
494 | * Set the map where the match was played |
||
495 | * @param int $map The ID of the map |
||
496 | * @return self |
||
497 | */ |
||
498 | public function setMap($map) |
||
502 | |||
503 | /** |
||
504 | * Get a JSON decoded array of events that occurred during the match |
||
505 | * @return mixed|null Returns null if there were no events recorded for the match |
||
506 | */ |
||
507 | public function getMatchDetails() |
||
511 | |||
512 | /** |
||
513 | * Get the server address of the server where this match took place |
||
514 | * @return string|null Returns null if there was no server address recorded |
||
515 | */ |
||
516 | public function getServerAddress() |
||
524 | |||
525 | /** |
||
526 | * Set the server address of the server where this match took place |
||
527 | * |
||
528 | * @param string|null $server The server hostname |
||
529 | * @param int|null $port The server port |
||
530 | * @return self |
||
531 | */ |
||
532 | public function setServerAddress($server = null, $port = 5154) |
||
539 | |||
540 | /** |
||
541 | * Get the name of the replay file for this specific map |
||
542 | * @param int $length The length of the replay file name; it will be truncated |
||
543 | * @return string Returns null if there was no replay file name recorded |
||
544 | */ |
||
545 | public function getReplayFileName($length = 0) |
||
553 | |||
554 | /** |
||
555 | * Get the match duration |
||
556 | * @return int The duration of the match in minutes |
||
557 | */ |
||
558 | 3 | public function getDuration() |
|
562 | |||
563 | /** |
||
564 | * Set the match duration |
||
565 | * |
||
566 | * @param int $duration The new duration of the match in minutes |
||
567 | * @return self |
||
568 | */ |
||
569 | public function setDuration($duration) |
||
573 | |||
574 | /** |
||
575 | * Get the user who entered the match |
||
576 | * @return Player |
||
577 | */ |
||
578 | 2 | public function getEnteredBy() |
|
582 | |||
583 | /** |
||
584 | * Get the loser of the match |
||
585 | * |
||
586 | * @return Team The team that was the loser or the team with the lower elo if the match was a draw |
||
587 | */ |
||
588 | 8 | public function getLoser() |
|
596 | |||
597 | /** |
||
598 | * Get the winner of a match |
||
599 | * |
||
600 | * @return Team The team that was the victor or the team with the lower elo if the match was a draw |
||
601 | */ |
||
602 | 8 | public function getWinner() |
|
614 | |||
615 | /** |
||
616 | * Determine whether the match was a draw |
||
617 | * @return bool True if the match ended without any winning teams |
||
618 | */ |
||
619 | 9 | public function isDraw() |
|
623 | |||
624 | /** |
||
625 | * Find out whether the match involves a team |
||
626 | * |
||
627 | * @param Team $team The team to check |
||
628 | * @return bool |
||
629 | */ |
||
630 | 1 | public function involvesTeam($team) |
|
634 | |||
635 | /** |
||
636 | * Reset the ELOs of the teams participating in the match |
||
637 | * |
||
638 | * @return self |
||
639 | */ |
||
640 | public function resetELOs() |
||
645 | |||
646 | /** |
||
647 | * Calculate the match's contribution to the team activity |
||
648 | * |
||
649 | * @return float |
||
650 | */ |
||
651 | 1 | public function getActivity() |
|
664 | |||
665 | /** |
||
666 | * Enter a new match to the database |
||
667 | * @param int $a Team A's ID |
||
668 | * @param int $b Team B's ID |
||
669 | * @param int $a_points Team A's match points |
||
670 | * @param int $b_points Team B's match points |
||
671 | * @param int $duration The match duration in minutes |
||
672 | * @param int|null $entered_by The ID of the player reporting the match |
||
673 | * @param string|DateTime $timestamp When the match was played |
||
674 | * @param int[] $a_players The IDs of the first team's players |
||
675 | * @param int[] $b_players The IDs of the second team's players |
||
676 | * @param string|null $server The address of the server where the match was played |
||
677 | * @param int|null $port The port of the server where the match was played |
||
678 | * @param string $replayFile The name of the replay file of the match |
||
679 | * @param int $map The ID of the map where the match was played, only for rotational leagues |
||
680 | * @param string $matchType The type of match (e.g. official, fm, special) |
||
681 | * @return Match An object representing the match that was just entered |
||
682 | */ |
||
683 | 9 | public static function enterMatch( |
|
735 | |||
736 | /** |
||
737 | * Calculate the ELO score difference |
||
738 | * |
||
739 | * Computes the ELO score difference on each team after a match, based on |
||
740 | * GU League's rules. |
||
741 | * |
||
742 | * @param int $a_elo Team A's current ELO score |
||
743 | * @param int $b_elo Team B's current ELO score |
||
744 | * @param int $a_points Team A's match points |
||
745 | * @param int $b_points Team B's match points |
||
746 | * @param int $duration The match duration in minutes |
||
747 | * @return int The ELO score difference |
||
748 | */ |
||
749 | 9 | public static function calculateEloDiff($a_elo, $b_elo, $a_points, $b_points, $duration) |
|
772 | |||
773 | /** |
||
774 | * Find if a match's stored ELO is correct |
||
775 | */ |
||
776 | public function isEloCorrect() |
||
786 | |||
787 | /** |
||
788 | * Recalculate the match's elo and adjust the team ELO values |
||
789 | */ |
||
790 | public function recalculateElo() |
||
811 | |||
812 | /** |
||
813 | * Get all the matches in the database |
||
814 | */ |
||
815 | 1 | public static function getMatches() |
|
819 | |||
820 | /** |
||
821 | * Get a query builder for matches |
||
822 | * @return MatchQueryBuilder |
||
823 | */ |
||
824 | 3 | public static function getQueryBuilder() |
|
837 | |||
838 | /** |
||
839 | * {@inheritdoc} |
||
840 | */ |
||
841 | public function delete() |
||
847 | |||
848 | /** |
||
849 | * {@inheritdoc} |
||
850 | */ |
||
851 | 3 | public static function getActiveStatuses() |
|
855 | |||
856 | /** |
||
857 | * {@inheritdoc} |
||
858 | */ |
||
859 | 1 | public function getName() |
|
869 | |||
870 | /** |
||
871 | * Update the match count of the teams participating in the match |
||
872 | * |
||
873 | * @param bool $decrement Whether to decrement instead of incrementing the match count |
||
874 | */ |
||
875 | 9 | private function updateMatchCount($decrement = false) |
|
887 | } |
||
888 |
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.