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 Challenge 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 Challenge, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
21 | class Challenge extends CModel |
||
22 | { |
||
23 | const TIME_LIMIT_HOURS = 8; |
||
|
|||
24 | const TIME_LIMIT_LASTCALL_HOURS = 4; |
||
25 | |||
26 | private $id; |
||
27 | private $active = false; |
||
28 | private $caller; |
||
29 | private $opponent; |
||
30 | private $loot_caller; |
||
31 | private $loot_opponent; |
||
32 | private $cnt_won_caller; |
||
33 | private $cnt_won_opponent; |
||
34 | private $point_caller; |
||
35 | private $point_opponent; |
||
36 | private $name_caller; |
||
37 | private $name_opponent; |
||
38 | private $winner; |
||
39 | private $created; |
||
40 | private $listDuels = []; |
||
41 | |||
42 | public function attributeNames() |
||
46 | |||
47 | public function getId() |
||
51 | |||
52 | public function getActive() |
||
56 | |||
57 | public function getOpponentLink($clubID) |
||
64 | |||
65 | public function getCaller() |
||
69 | |||
70 | public function getOpponent() |
||
74 | |||
75 | public function getName_caller() |
||
79 | |||
80 | public function getName_opponent() |
||
84 | |||
85 | public function getCnt_won_caller() |
||
89 | |||
90 | public function getCnt_won_opponent() |
||
94 | |||
95 | public function getPoint_caller() |
||
99 | |||
100 | public function getPoint_opponent() |
||
104 | |||
105 | public function getLoot_caller() |
||
109 | |||
110 | public function getLoot_opponent() |
||
114 | |||
115 | public function getStartTime() |
||
119 | |||
120 | public function getEndTime() |
||
124 | |||
125 | public function getListDuels() |
||
129 | |||
130 | public function getWinner() |
||
134 | |||
135 | public function setId($id) |
||
139 | |||
140 | public function setCaller($clubID) |
||
144 | |||
145 | public function setOpponent($clubID) |
||
149 | |||
150 | View Code Duplication | public function fetch() |
|
166 | |||
167 | View Code Duplication | public function fetchActiveChallenge() |
|
185 | |||
186 | public function hasActiveChallenge($clubID) |
||
197 | |||
198 | /** |
||
199 | * @param integer $opponentID |
||
200 | */ |
||
201 | View Code Duplication | public function underCallTimeLimit($clubID, $opponentID) |
|
212 | |||
213 | /** |
||
214 | * @param integer $clubID |
||
215 | */ |
||
216 | View Code Duplication | public function underLastCallTimeLimit($clubID) |
|
227 | |||
228 | /** |
||
229 | * @param Club $opponent |
||
230 | */ |
||
231 | public function callToChallenge($opponent) |
||
285 | |||
286 | public function fetchListDuels() |
||
307 | |||
308 | private function getAwards($id, $role) |
||
317 | |||
318 | private function addCommandToStack($params) |
||
327 | |||
328 | private function addReminder() |
||
335 | } |
||
336 |
This check looks for multiple assignments in successive lines of code. It will report an issue if the operators are not in a straight line.
To visualize
will produce issues in the first and second line, while this second example
will produce no issues.