| 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: