Conditions | 11 |
Paths | 12 |
Total Lines | 83 |
Code Lines | 55 |
Lines | 0 |
Ratio | 0 % |
Changes | 5 | ||
Bugs | 1 | Features | 4 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
1 | <?php |
||
155 | public function matchReportAction(Logger $log, Request $request) |
||
156 | { |
||
157 | $log->addNotice("Match data received from " . $request->getClientIp()); |
||
158 | $log->addDebug("Debug match data query: " . http_build_query($this->params->all())); |
||
159 | |||
160 | $matchType = $this->params->get('matchType', Match::OFFICIAL); |
||
161 | |||
162 | $teamOneBZIDs = $this->params->get('teamOnePlayers'); |
||
163 | $teamTwoBZIDs = $this->params->get('teamTwoPlayers'); |
||
164 | |||
165 | $teamOnePlayers = $this->bzidsToIdArray($teamOneBZIDs); |
||
166 | $teamTwoPlayers = $this->bzidsToIdArray($teamTwoBZIDs); |
||
167 | |||
168 | if (Match::OFFICIAL === $matchType) { |
||
169 | $teamOne = $this->getTeam($teamOnePlayers); |
||
170 | $teamTwo = $this->getTeam($teamTwoPlayers); |
||
171 | |||
172 | // If we fail to get the the team ID for either the teams or both reported teams are the same team, we cannot |
||
173 | // report the match due to it being illegal. |
||
174 | |||
175 | // An invalid team could be found in either or both teams, so we need to check both teams and log the match |
||
176 | // failure respectively. |
||
177 | $error = true; |
||
178 | if (!$teamOne->isValid()) { |
||
179 | $log->addNotice("The BZIDs ($teamOneBZIDs) were not found on the same team. Match invalidated."); |
||
180 | } elseif (!$teamTwo->isValid()) { |
||
181 | $log->addNotice("The BZIDs ($teamTwoBZIDs) were not found on the same team. Match invalidated."); |
||
182 | } else { |
||
183 | $error = false; |
||
184 | } |
||
185 | |||
186 | if ($error) { |
||
187 | throw new ForbiddenException("An invalid player was found during the match. Please message a referee to manually report the match."); |
||
188 | } |
||
189 | |||
190 | if ($teamOne->isSameAs($teamTwo)) { |
||
191 | $log->addNotice("The '" . $teamOne->getName() . "' team played against each other in an official match. Match invalidated."); |
||
192 | throw new ForbiddenException("Holy sanity check, Batman! The same team can't play against each other in an official match."); |
||
193 | } |
||
194 | } elseif (Match::FUN === $matchType) { |
||
195 | if (count($teamOnePlayers) < 2 || count($teamTwoPlayers) < 2) { |
||
196 | throw new ForbiddenException("You are not allowed to report a match with less than 2 players per team."); |
||
197 | } |
||
198 | } |
||
199 | |||
200 | $map = Map::fetchFromAlias($this->params->get('mapPlayed')); |
||
201 | |||
202 | $match = Match::enterMatch( |
||
203 | (isset($teamOne)) ? $teamOne->getId() : null, |
||
204 | (isset($teamTwo)) ? $teamTwo->getId() : null, |
||
205 | $this->params->get('teamOneWins'), |
||
206 | $this->params->get('teamTwoWins'), |
||
207 | $this->params->get('duration'), |
||
208 | null, |
||
209 | $this->params->get('matchTime'), |
||
210 | $teamOnePlayers, |
||
211 | $teamTwoPlayers, |
||
212 | $this->params->get('server'), |
||
213 | $this->params->get('port'), |
||
214 | $this->params->get('replayFile'), |
||
215 | $map->getId(), |
||
216 | $this->params->get('matchType'), |
||
217 | $this->params->get('teamOneColor'), |
||
218 | $this->params->get('teamTwoColor') |
||
219 | ); |
||
220 | |||
221 | $log->addNotice("Match reported automatically", array( |
||
222 | 'winner' => array( |
||
223 | 'name' => $match->getWinner()->getName(), |
||
224 | 'score' => $match->getScore($match->getWinner()), |
||
225 | ), |
||
226 | 'loser' => array( |
||
227 | 'name' => $match->getLoser()->getName(), |
||
228 | 'score' => $match->getScore($match->getLoser()) |
||
229 | ), |
||
230 | 'eloDiff' => $match->getEloDiff(), |
||
231 | 'type' => $matchType, |
||
232 | 'map' => $map->getName() |
||
233 | )); |
||
234 | |||
235 | // Output the match stats that will be sent back to BZFS |
||
236 | return $match->getName(); |
||
237 | } |
||
238 | |||
291 |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the parent class: