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 VotesProcess 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 VotesProcess, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
19 | trait VotesProcess |
||
20 | { |
||
21 | |||
22 | /////////// CONSTRUCTOR /////////// |
||
23 | |||
24 | // Data and global options |
||
25 | protected $_Votes; // Votes list |
||
26 | |||
27 | |||
28 | /////////// VOTES LIST /////////// |
||
29 | |||
30 | // How many votes are registered ? |
||
31 | 12 | public function countVotes ($tag = null, bool $with = true) : int |
|
35 | |||
36 | 1 | public function countInvalidVoteWithConstraints () : int |
|
40 | |||
41 | 1 | public function countValidVoteWithConstraints () : int |
|
45 | |||
46 | // Sum votes weight |
||
47 | 2 | public function sumVotesWeight () : int |
|
51 | |||
52 | 6 | public function sumValidVotesWeightWithConstraints () : int |
|
56 | |||
57 | // Get the votes registered list |
||
58 | 11 | public function getVotesList ($tag = null, bool $with = true) : array |
|
62 | |||
63 | 5 | public function getVotesListAsString () : string |
|
67 | |||
68 | 102 | public function getVotesManager () : VotesManager { |
|
71 | |||
72 | 2 | public function getVotesListGenerator ($tag = null, bool $with = true) : \Generator |
|
76 | |||
77 | 9 | public function getVoteKey (Vote $vote) { |
|
80 | |||
81 | |||
82 | /////////// ADD & REMOVE VOTE /////////// |
||
83 | |||
84 | // Add a single vote. Array key is the rank, each candidate in a rank are separate by ',' It is not necessary to register the last rank. |
||
85 | 126 | public function addVote ($vote, $tag = null) : Vote |
|
98 | |||
99 | 7 | public function prepareUpdateVote (Vote $existVote) : void |
|
100 | { |
||
101 | 7 | $this->_Votes->UpdateAndResetComputing($this->getVoteKey($existVote),2); |
|
102 | 7 | } |
|
103 | |||
104 | 7 | public function finishUpdateVote (Vote $existVote) : void |
|
105 | { |
||
106 | 7 | $this->_Votes->UpdateAndResetComputing($this->getVoteKey($existVote),1); |
|
107 | |||
108 | 7 | if ($this->_Votes->isUsingHandler()) : |
|
109 | 1 | $this->_Votes[$this->getVoteKey($existVote)] = $existVote; |
|
110 | endif; |
||
111 | 7 | } |
|
112 | |||
113 | 126 | public function checkVoteCandidate (Vote $vote) : bool |
|
114 | { |
||
115 | 126 | $linkCount = $vote->countLinks(); |
|
116 | 126 | $links = $vote->getLinks(); |
|
117 | |||
118 | 126 | $mirror = $vote->getRanking(); |
|
119 | 126 | $change = false; |
|
120 | 126 | foreach ($vote as $rank => $choice) : |
|
121 | 126 | foreach ($choice as $choiceKey => $candidate) : |
|
122 | 126 | if ( !$this->isRegisteredCandidate($candidate, true) ) : |
|
|
|||
123 | 109 | if ($candidate->getProvisionalState() && $this->isRegisteredCandidate($candidate, false)) : |
|
124 | 108 | if ( $linkCount === 0 || ($linkCount === 1 && reset($links) === $this) ) : |
|
125 | 108 | $mirror[$rank][$choiceKey] = $this->_Candidates[$this->getCandidateKey((string) $candidate)]; |
|
126 | 108 | $change = true; |
|
127 | else : |
||
128 | 126 | return false; |
|
129 | endif; |
||
130 | endif; |
||
131 | endif; |
||
132 | endforeach; |
||
133 | endforeach; |
||
134 | |||
135 | 126 | if ($change) : |
|
136 | 108 | $vote->setRanking( $mirror, |
|
137 | 108 | ( abs($vote->getTimestamp() - microtime(true)) > 0.5 ) ? ($vote->getTimestamp() + 0.001) : null |
|
138 | ); |
||
139 | endif; |
||
140 | |||
141 | 126 | return true; |
|
142 | } |
||
143 | |||
144 | // Write a new vote |
||
145 | 126 | protected function registerVote (Vote $vote, $tag = null) : Vote |
|
146 | { |
||
147 | // Vote identifiant |
||
148 | 126 | $vote->addTags($tag); |
|
149 | |||
150 | // Register |
||
151 | try { |
||
152 | 126 | $vote->registerLink($this); |
|
153 | 126 | $this->_Votes[] = $vote; |
|
154 | 1 | } catch (CondorcetException $e) { |
|
155 | // Security : Check if vote object not already register |
||
156 | 1 | throw new CondorcetException(31); |
|
157 | } |
||
158 | |||
159 | 126 | return $vote; |
|
160 | } |
||
161 | |||
162 | 5 | public function removeVote ($in, bool $with = true) : array |
|
163 | { |
||
164 | 5 | $rem = []; |
|
165 | |||
166 | 5 | if ($in instanceof Vote) : |
|
167 | 4 | $key = $this->getVoteKey($in); |
|
168 | 4 | if ($key !== false) : |
|
169 | 4 | $deletedVote = $this->_Votes[$key]; |
|
170 | 4 | $rem[] = $deletedVote; |
|
171 | |||
172 | 4 | unset($this->_Votes[$key]); |
|
173 | |||
174 | 4 | $deletedVote->destroyLink($this); |
|
175 | endif; |
||
176 | else : |
||
177 | // Prepare Tags |
||
178 | 3 | $tag = VoteUtil::tagsConvert($in); |
|
179 | |||
180 | // Deleting |
||
181 | 3 | foreach ($this->getVotesList($tag, $with) as $key => $value) : |
|
182 | 3 | $deletedVote = $this->_Votes[$key]; |
|
183 | 3 | $rem[] = $deletedVote; |
|
184 | |||
185 | 3 | unset($this->_Votes[$key]); |
|
186 | |||
187 | 3 | $deletedVote->destroyLink($this); |
|
188 | endforeach; |
||
189 | |||
190 | endif; |
||
191 | |||
192 | 5 | return $rem; |
|
193 | } |
||
194 | |||
195 | |||
196 | /////////// PARSE VOTE /////////// |
||
197 | |||
198 | // Return the well formated vote to use. |
||
199 | 126 | protected function prepareVoteInput (&$vote, $tag = null) : void |
|
210 | |||
211 | 2 | public function jsonVotes (string $input) : array |
|
248 | |||
249 | 89 | public function parseVotes (string $input, bool $isFile = false) : array |
|
304 | |||
305 | } |
||
306 |
This check looks for methods that are used by a trait but not required by it.
To illustrate, let’s look at the following code example
The trait
Idable
provides a methodequalsId
that in turn relies on the methodgetId()
. If this method does not exist on a class mixing in this trait, the method will fail.Adding the
getId()
as an abstract method to the trait will make sure it is available.