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 GameController 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 GameController, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
14 | class GameController extends AbstractActionController |
||
15 | { |
||
16 | /** |
||
17 | * @var \PlaygroundGame\Service\GameService |
||
18 | */ |
||
19 | protected $gameService; |
||
20 | |||
21 | protected $prizeService; |
||
22 | |||
23 | protected $options; |
||
24 | |||
25 | protected $game; |
||
26 | |||
27 | protected $user; |
||
28 | |||
29 | protected $withGame = array( |
||
30 | 'home', |
||
31 | 'index', |
||
32 | 'terms', |
||
33 | 'conditions', |
||
34 | 'leaderboard', |
||
35 | 'register', |
||
36 | 'bounce', |
||
37 | 'prizes', |
||
38 | 'prize', |
||
39 | 'fangate', |
||
40 | 'share', |
||
41 | 'optin', |
||
42 | 'login', |
||
43 | 'play', |
||
44 | 'result', |
||
45 | 'preview', |
||
46 | 'list' |
||
47 | ); |
||
48 | |||
49 | protected $withOnlineGame = array( |
||
50 | 'leaderboard', |
||
51 | 'register', |
||
52 | 'bounce', |
||
53 | 'play', |
||
54 | 'result' |
||
55 | ); |
||
56 | |||
57 | protected $withAnyUser = array( |
||
58 | 'share', |
||
59 | 'result', |
||
60 | 'play' |
||
61 | ); |
||
62 | |||
63 | public function setEventManager(\Zend\EventManager\EventManagerInterface $events) |
||
125 | |||
126 | /** |
||
127 | * Action called if matched action does not exist |
||
128 | * For this view not to be catched by Zend\Mvc\View\RouteNotFoundStrategy |
||
129 | * it has to be rendered in the controller. Hence the code below. |
||
130 | * |
||
131 | * This action is injected as a catchall action for each custom_games definition |
||
132 | * This way, when a custom_game is created, the 404 is it's responsability and the |
||
133 | * view can be defined in design/frontend/default/custom/$slug/playground_game/$gametype/404.phtml |
||
134 | * |
||
135 | * |
||
136 | * @return \Zend\Stdlib\ResponseInterface |
||
137 | */ |
||
138 | public function notFoundAction() |
||
155 | |||
156 | /** |
||
157 | * This action acts as a hub : Depending on the first step of the game, it will forward the action to this step |
||
158 | */ |
||
159 | public function homeAction() |
||
191 | |||
192 | /** |
||
193 | * Homepage of the game |
||
194 | */ |
||
195 | public function indexAction() |
||
211 | |||
212 | /** |
||
213 | * leaderboardAction |
||
214 | * |
||
215 | * @return ViewModel $viewModel |
||
216 | */ |
||
217 | public function leaderboardAction() |
||
218 | { |
||
219 | $filter = $this->getEvent()->getRouteMatch()->getParam('filter'); |
||
220 | $p = $this->getEvent()->getRouteMatch()->getParam('p'); |
||
221 | |||
222 | $beforeLayout = $this->layout()->getTemplate(); |
||
223 | $subViewModel = $this->forward()->dispatch( |
||
224 | 'playgroundreward', |
||
225 | array('action' => 'leaderboard', 'filter' => $filter, 'p' => $p) |
||
226 | ); |
||
227 | |||
228 | // suite au forward, le template de layout a changé, je dois le rétablir... |
||
229 | $this->layout()->setTemplate($beforeLayout); |
||
230 | |||
231 | // give the ability to the game to have its customized look and feel. |
||
232 | $templatePathResolver = $this->getServiceLocator()->get('Zend\View\Resolver\TemplatePathStack'); |
||
233 | $l = $templatePathResolver->getPaths(); |
||
234 | |||
235 | $templatePathResolver->addPath($l[0].'custom/'.$this->game->getIdentifier()); |
||
236 | |||
237 | return $subViewModel; |
||
238 | } |
||
239 | |||
240 | /** |
||
241 | * This action has been designed to be called by other controllers |
||
242 | * It gives the ability to display an information form and persist it in the game entry |
||
243 | * |
||
244 | * @return \Zend\View\Model\ViewModel |
||
245 | */ |
||
246 | public function registerAction() |
||
247 | { |
||
248 | $form = $this->getGameService()->createFormFromJson($this->game->getPlayerForm()->getForm(), 'playerForm'); |
||
249 | |||
250 | if ($this->getRequest()->isPost()) { |
||
251 | // POST Request: Process form |
||
252 | $data = array_merge_recursive( |
||
253 | $this->getRequest()->getPost()->toArray(), |
||
254 | $this->getRequest()->getFiles()->toArray() |
||
255 | ); |
||
256 | |||
257 | $form->setData($data); |
||
258 | |||
259 | if ($form->isValid()) { |
||
260 | // steps of the game |
||
261 | $steps = $this->game->getStepsArray(); |
||
262 | // sub steps of the game |
||
263 | $viewSteps = $this->game->getStepsViewsArray(); |
||
264 | |||
265 | // register position |
||
266 | $key = array_search($this->params('action'), $viewSteps); |
||
267 | View Code Duplication | if (!$key) { |
|
268 | // register is not a substep of the game so it's a step |
||
269 | $key = array_search($this->params('action'), $steps); |
||
270 | $keyStep = true; |
||
271 | } else { |
||
272 | // register was a substep, i search the index of its parent |
||
273 | $key = array_search($key, $steps); |
||
274 | $keyStep = false; |
||
275 | } |
||
276 | |||
277 | // play position |
||
278 | $keyplay = array_search('play', $viewSteps); |
||
279 | |||
280 | View Code Duplication | if (!$keyplay) { |
|
281 | // play is not a substep, so it's a step |
||
282 | $keyplay = array_search('play', $steps); |
||
283 | $keyplayStep = true; |
||
284 | } else { |
||
285 | // play is a substep so I search the index of its parent |
||
286 | $keyplay = array_search($keyplay, $steps); |
||
287 | $keyplayStep = false; |
||
288 | } |
||
289 | |||
290 | // If register step before play, I don't have no entry yet. I have to create one |
||
291 | // If register after play step, I search for the last entry created by play step. |
||
292 | |||
293 | if ($key < $keyplay || ($keyStep && !$keyplayStep && $key <= $keyplay)) { |
||
294 | $entry = $this->getGameService()->play($this->game, $this->user); |
||
295 | if (!$entry) { |
||
296 | // the user has already taken part of this game and the participation limit has been reached |
||
297 | $this->flashMessenger()->addMessage('Vous avez déjà participé'); |
||
298 | |||
299 | return $this->redirect()->toUrl( |
||
300 | $this->frontendUrl()->fromRoute( |
||
301 | $this->game->getClassType().'/result', |
||
302 | array( |
||
303 | 'id' => $this->game->getIdentifier(), |
||
304 | |||
305 | ) |
||
306 | ) |
||
307 | ); |
||
308 | } |
||
309 | } else { |
||
310 | // I'm looking for an entry without anonymousIdentifier (the active entry in fact). |
||
311 | $entry = $this->getGameService()->findLastEntry($this->game, $this->user); |
||
312 | if ($this->getGameService()->hasReachedPlayLimit($this->game, $this->user)) { |
||
313 | // the user has already taken part of this game and the participation limit has been reached |
||
314 | $this->flashMessenger()->addMessage('Vous avez déjà participé'); |
||
315 | |||
316 | return $this->redirect()->toUrl( |
||
317 | $this->frontendUrl()->fromRoute( |
||
318 | $this->game->getClassType().'/result', |
||
319 | array( |
||
320 | 'id' => $this->game->getIdentifier(), |
||
321 | |||
322 | ) |
||
323 | ) |
||
324 | ); |
||
325 | } |
||
326 | } |
||
327 | |||
328 | $this->getGameService()->updateEntryPlayerForm($form->getData(), $this->game, $this->user, $entry); |
||
329 | |||
330 | if (!empty($this->game->nextStep($this->params('action')))) { |
||
331 | return $this->redirect()->toUrl( |
||
332 | $this->frontendUrl()->fromRoute( |
||
333 | $this->game->getClassType() .'/' . $this->game->nextStep($this->params('action')), |
||
334 | array('id' => $this->game->getIdentifier()), |
||
335 | array('force_canonical' => true) |
||
336 | ) |
||
337 | ); |
||
338 | } |
||
339 | } |
||
340 | } |
||
341 | |||
342 | $viewModel = $this->buildView($this->game); |
||
343 | $viewModel->setVariables(array( |
||
344 | 'form' => $form |
||
345 | )); |
||
346 | |||
347 | return $viewModel; |
||
348 | } |
||
349 | |||
350 | /** |
||
351 | * This action takes care of the terms of the game |
||
352 | */ |
||
353 | public function termsAction() |
||
354 | { |
||
355 | $viewModel = $this->buildView($this->game); |
||
356 | |||
357 | return $viewModel; |
||
358 | } |
||
359 | |||
360 | /** |
||
361 | * This action takes care of the conditions of the game |
||
362 | */ |
||
363 | public function conditionsAction() |
||
369 | |||
370 | /** |
||
371 | * This action takes care of bounce page of the game |
||
372 | */ |
||
373 | public function bounceAction() |
||
392 | |||
393 | |||
394 | /** |
||
395 | * This action displays the Prizes page associated to the game |
||
396 | */ |
||
397 | public function prizesAction() |
||
407 | |||
408 | /** |
||
409 | * This action displays a specific Prize page among those associated to the game |
||
410 | */ |
||
411 | public function prizeAction() |
||
425 | |||
426 | public function gameslistAction() |
||
473 | |||
474 | public function fangateAction() |
||
480 | |||
481 | public function shareAction() |
||
482 | { |
||
483 | $statusMail = null; |
||
484 | $lastEntry = null; |
||
485 | |||
486 | // steps of the game |
||
487 | $steps = $this->game->getStepsArray(); |
||
488 | // sub steps of the game |
||
489 | $viewSteps = $this->game->getStepsViewsArray(); |
||
490 | |||
491 | // share position |
||
492 | $key = array_search($this->params('action'), $viewSteps); |
||
493 | View Code Duplication | if (!$key) { |
|
494 | // share is not a substep of the game so it's a step |
||
495 | $key = array_search($this->params('action'), $steps); |
||
496 | $keyStep = true; |
||
497 | } else { |
||
498 | // share was a substep, I search the index of its parent |
||
499 | $key = array_search($key, $steps); |
||
500 | $keyStep = false; |
||
501 | } |
||
502 | |||
503 | // play position |
||
504 | $keyplay = array_search('play', $viewSteps); |
||
505 | |||
506 | View Code Duplication | if (!$keyplay) { |
|
507 | // play is not a substep, so it's a step |
||
508 | $keyplay = array_search('play', $steps); |
||
509 | $keyplayStep = true; |
||
510 | } else { |
||
511 | // play is a substep so I search the index of its parent |
||
512 | $keyplay = array_search($keyplay, $steps); |
||
513 | $keyplayStep = false; |
||
514 | } |
||
515 | |||
516 | if ($key && $keyplay && $keyplay <= $key) { |
||
517 | // Has the user finished the game ? |
||
518 | $lastEntry = $this->getGameService()->findLastInactiveEntry($this->game, $this->user); |
||
519 | |||
520 | View Code Duplication | if ($lastEntry === null) { |
|
521 | return $this->redirect()->toUrl( |
||
522 | $this->frontendUrl()->fromRoute( |
||
523 | $this->game->getClassType(), |
||
524 | array('id' => $this->game->getIdentifier()) |
||
525 | ) |
||
526 | ); |
||
527 | } |
||
528 | } |
||
529 | |||
530 | $form = $this->getServiceLocator()->get('playgroundgame_sharemail_form'); |
||
531 | $form->setAttribute('method', 'post'); |
||
532 | |||
533 | // buildView must be before sendMail because it adds the game template path to the templateStack |
||
534 | $viewModel = $this->buildView($this->game); |
||
535 | |||
536 | if ($this->getRequest()->isPost()) { |
||
537 | $data = $this->getRequest()->getPost()->toArray(); |
||
538 | $form->setData($data); |
||
539 | View Code Duplication | if ($form->isValid()) { |
|
540 | $result = $this->getGameService()->sendShareMail($data, $this->game, $this->user, $lastEntry); |
||
541 | if ($result) { |
||
542 | $statusMail = true; |
||
543 | } |
||
544 | } |
||
545 | } |
||
546 | |||
547 | $viewModel->setVariables(array( |
||
548 | 'statusMail' => $statusMail, |
||
549 | 'form' => $form, |
||
550 | )); |
||
551 | |||
552 | return $viewModel; |
||
553 | } |
||
554 | |||
555 | View Code Duplication | public function fbshareAction() |
|
575 | |||
576 | public function fbrequestAction() |
||
598 | |||
599 | public function tweetAction() |
||
618 | |||
619 | View Code Duplication | public function googleAction() |
|
640 | |||
641 | public function optinAction() |
||
642 | { |
||
643 | $userService = $this->getServiceLocator()->get('zfcuser_user_service'); |
||
644 | |||
659 | |||
660 | public function loginAction() |
||
723 | |||
724 | public function userregisterAction() |
||
952 | |||
953 | View Code Duplication | public function cmsPageAction() |
|
972 | |||
973 | View Code Duplication | public function cmsListAction() |
|
993 | |||
994 | /** |
||
995 | * |
||
996 | * @param \PlaygroundGame\Entity\Game $game |
||
997 | * @param \PlaygroundUser\Entity\User $user |
||
998 | */ |
||
999 | public function checkFbRegistration($user, $game) |
||
1048 | |||
1049 | /** |
||
1050 | * This method create the basic Game view |
||
1051 | * @param \PlaygroundGame\Entity\Game $game |
||
1052 | */ |
||
1053 | public function buildView($game) |
||
1054 | { |
||
1055 | if ($this->getRequest()->isXmlHttpRequest()) { |
||
1056 | $viewModel = new JsonModel(); |
||
1057 | if ($game) { |
||
1058 | $view = $this->addAdditionalView($game); |
||
1059 | if ($view && $view instanceof \Zend\View\Model\ViewModel) { |
||
1060 | $viewModel->setVariables($view->getVariables()); |
||
1061 | } |
||
1062 | } |
||
1063 | } else { |
||
1064 | $viewModel = new ViewModel(); |
||
1065 | |||
1066 | if ($game) { |
||
1067 | $this->addMetaTitle($game); |
||
1068 | $this->addMetaBitly(); |
||
1069 | $this->addGaEvent($game); |
||
1070 | |||
1071 | $this->customizeGameDesign($game); |
||
1072 | |||
1073 | $view = $this->addAdditionalView($game); |
||
1074 | if ($view && $view instanceof \Zend\View\Model\ViewModel) { |
||
1075 | $viewModel->addChild($view, 'additional'); |
||
1076 | } elseif ($view && $view instanceof \Zend\Http\PhpEnvironment\Response) { |
||
1077 | return $view; |
||
1078 | } |
||
1079 | |||
1080 | $this->layout()->setVariables( |
||
1081 | array( |
||
1082 | 'action' => $this->params('action'), |
||
1083 | 'game' => $game, |
||
1084 | 'flashMessages' => $this->flashMessenger()->getMessages(), |
||
1085 | |||
1086 | ) |
||
1087 | ); |
||
1088 | } |
||
1089 | } |
||
1090 | |||
1091 | if ($game) { |
||
1092 | $viewModel->setVariables($this->getShareData($game)); |
||
1093 | $viewModel->setVariables(array('game' => $game)); |
||
1094 | } |
||
1095 | |||
1096 | return $viewModel; |
||
1097 | } |
||
1098 | |||
1099 | /** |
||
1100 | * @param \PlaygroundGame\Entity\Game $game |
||
1101 | */ |
||
1102 | public function addAdditionalView($game) |
||
1103 | { |
||
1104 | $view = false; |
||
1105 | |||
1106 | $actionName = $this->getEvent()->getRouteMatch()->getParam('action', 'not-found'); |
||
1107 | $stepsViews = json_decode($game->getStepsViews(), true); |
||
1108 | if ($stepsViews && isset($stepsViews[$actionName])) { |
||
1109 | $beforeLayout = $this->layout()->getTemplate(); |
||
1110 | $actionData = $stepsViews[$actionName]; |
||
1111 | if (is_string($actionData)) { |
||
1112 | $action = $actionData; |
||
1113 | $controller = $this->getEvent()->getRouteMatch()->getParam('controller', 'playgroundgame_game'); |
||
1114 | $view = $this->forward()->dispatch( |
||
1115 | $controller, |
||
1116 | array( |
||
1117 | 'action' => $action, |
||
1118 | 'id' => $game->getIdentifier() |
||
1119 | ) |
||
1120 | ); |
||
1121 | } elseif (is_array($actionData) && count($actionData)>0) { |
||
1122 | $action = key($actionData); |
||
1123 | $controller = $actionData[$action]; |
||
1124 | $view = $this->forward()->dispatch( |
||
1125 | $controller, |
||
1126 | array( |
||
1127 | 'action' => $action, |
||
1128 | 'id' => $game->getIdentifier() |
||
1129 | ) |
||
1130 | ); |
||
1131 | } |
||
1132 | // suite au forward, le template de layout a changé, je dois le rétablir... |
||
1133 | $this->layout()->setTemplate($beforeLayout); |
||
1134 | } |
||
1135 | |||
1136 | return $view; |
||
1137 | } |
||
1138 | |||
1139 | public function addMetaBitly() |
||
1149 | |||
1150 | /** |
||
1151 | * @param \PlaygroundGame\Entity\Game $game |
||
1152 | */ |
||
1153 | public function addGaEvent($game) |
||
1161 | |||
1162 | /** |
||
1163 | * @param \PlaygroundGame\Entity\Game $game |
||
1164 | */ |
||
1165 | public function addMetaTitle($game) |
||
1185 | |||
1186 | /** |
||
1187 | * @param \PlaygroundGame\Entity\Game $game |
||
1188 | */ |
||
1189 | public function customizeGameDesign($game) |
||
1204 | |||
1205 | /** |
||
1206 | * @param \PlaygroundGame\Entity\Game $game |
||
1207 | */ |
||
1208 | public function getShareData($game) |
||
1294 | |||
1295 | /** |
||
1296 | * return ajax response in json format |
||
1297 | * |
||
1298 | * @param array $data |
||
1299 | * @return \Zend\View\Model\JsonModel |
||
1300 | */ |
||
1301 | View Code Duplication | protected function successJson($data = null) |
|
1309 | |||
1310 | /** |
||
1311 | * return ajax response in json format |
||
1312 | * |
||
1313 | * @param string $message |
||
1314 | * @return \Zend\View\Model\JsonModel |
||
1315 | */ |
||
1316 | View Code Duplication | protected function errorJson($message = null) |
|
1324 | |||
1325 | /** |
||
1326 | * @param string $helperName |
||
1327 | */ |
||
1328 | protected function getViewHelper($helperName) |
||
1332 | |||
1333 | public function getGameService() |
||
1341 | |||
1342 | public function setGameService(GameService $gameService) |
||
1348 | |||
1349 | public function getPrizeService() |
||
1357 | |||
1358 | public function setPrizeService(PrizeService $prizeService) |
||
1364 | |||
1365 | public function getOptions() |
||
1373 | |||
1374 | public function setOptions($options) |
||
1380 | } |
||
1381 |
If you implement
__call
and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.This is often the case, when
__call
is implemented by a parent class and only the child class knows which methods exist: