allejo /
bzion
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:
| 1 | <?php |
||
| 11 | abstract class CRUDController extends JSONController |
||
| 12 | { |
||
| 13 | /** |
||
| 14 | * Make sure that the data of a form is valid, only called when creating a |
||
| 15 | * new object |
||
| 16 | * @param Form $form The submitted form |
||
| 17 | * @return void |
||
| 18 | */ |
||
| 19 | protected function validateNew($form) |
||
| 20 | { |
||
| 21 | } |
||
| 22 | |||
| 23 | /** |
||
| 24 | * Make sure that the data of a form is valid |
||
| 25 | * @param Form $form The submitted form |
||
| 26 | * @return void |
||
| 27 | */ |
||
| 28 | protected function validate($form) |
||
| 31 | |||
| 32 | /** |
||
| 33 | * Delete a model |
||
| 34 | * @param PermissionModel $model The model we want to delete |
||
| 35 | * @param Player $me The user who wants to delete the model |
||
| 36 | * @param Closure|null $onSuccess Something to do when the model is deleted |
||
| 37 | * @throws ForbiddenException |
||
| 38 | * @return mixed The response to show to the user |
||
| 39 | */ |
||
| 40 | protected function delete(PermissionModel $model, Player $me, $onSuccess = null) |
||
| 41 | { |
||
| 42 | if ($model->isDeleted()) { |
||
| 43 | // We will have to hard delete the model |
||
| 44 | $hard = true; |
||
| 45 | $message = 'hardDelete'; |
||
| 46 | $action = 'Erase forever'; |
||
| 47 | } else { |
||
| 48 | $hard = false; |
||
| 49 | $message = 'softDelete'; |
||
| 50 | $action = 'Delete'; |
||
| 51 | } |
||
| 52 | |||
| 53 | if (!$this->canDelete($me, $model, $hard)) { |
||
| 54 | throw new ForbiddenException($this->getMessage($model, $message, 'forbidden')); |
||
| 55 | } |
||
| 56 | |||
| 57 | $successMessage = $this->getMessage($model, $message, 'success'); |
||
| 58 | $redirection = $this->redirectToList($model); |
||
| 59 | |||
| 60 | return $this->showConfirmationForm(function () use ($model, $hard, $redirection, $onSuccess) { |
||
| 61 | if ($hard) { |
||
| 62 | $model->wipe(); |
||
| 63 | } else { |
||
| 64 | $model->delete(); |
||
| 65 | } |
||
| 66 | |||
| 67 | if ($onSuccess) { |
||
| 68 | $onSuccess(); |
||
| 69 | } |
||
| 70 | |||
| 71 | return $redirection; |
||
| 72 | }, $this->getMessage($model, $message, 'confirm'), $successMessage, $action); |
||
| 73 | } |
||
| 74 | |||
| 75 | /** |
||
| 76 | * Create a model |
||
| 77 | * |
||
| 78 | * This method requires that you have implemented enter() and a form creator |
||
| 79 | * for the model |
||
| 80 | * |
||
| 81 | * @param Player $me The user who wants to create the model |
||
| 82 | * @throws ForbiddenException |
||
| 83 | * @return mixed The response to show to the user |
||
| 84 | */ |
||
| 85 | 1 | protected function create(Player $me) |
|
| 86 | { |
||
| 87 | 1 | if (!$this->canCreate($me)) { |
|
| 88 | 1 | throw new ForbiddenException($this->getMessage($this->getName(), 'create', 'forbidden')); |
|
| 89 | } |
||
| 90 | |||
| 91 | $creator = $this->getFormCreator(); |
||
| 92 | $form = $creator->create()->handleRequest($this->getRequest()); |
||
| 93 | |||
| 94 | View Code Duplication | if ($form->isSubmitted()) { |
|
|
|
|||
| 95 | $this->validate($form); |
||
| 96 | $this->validateNew($form); |
||
| 97 | if ($form->isValid()) { |
||
| 98 | $model = $creator->enter($form); |
||
| 99 | $this->getFlashBag()->add("success", |
||
| 100 | $this->getMessage($model, 'create', 'success')); |
||
| 101 | |||
| 102 | return $this->redirectTo($model); |
||
| 103 | } |
||
| 104 | } |
||
| 105 | |||
| 106 | return array("form" => $form->createView()); |
||
| 107 | } |
||
| 108 | |||
| 109 | /** |
||
| 110 | * Edit a model |
||
| 111 | * |
||
| 112 | * This method requires that you have implemented update() and a form creator |
||
| 113 | * for the model |
||
| 114 | * |
||
| 115 | * @param PermissionModel $model The model we want to edit |
||
| 116 | * @param Player $me The user who wants to edit the model |
||
| 117 | * @param string $type The name of the variable to pass to the view |
||
| 118 | * @throws ForbiddenException |
||
| 119 | * @return mixed The response to show to the user |
||
| 120 | */ |
||
| 121 | protected function edit(PermissionModel $model, Player $me, $type) |
||
| 143 | |||
| 144 | /** |
||
| 145 | * Find whether a player can delete a model |
||
| 146 | * |
||
| 147 | * @param Player $player The player who wants to delete the model |
||
| 148 | * @param PermissionModel $model The model that will be deleted |
||
| 149 | * @param bool $hard Whether to hard-delete the model instead of soft-deleting it |
||
| 150 | * @return bool |
||
| 151 | */ |
||
| 152 | protected function canDelete($player, $model, $hard = false) |
||
| 153 | { |
||
| 154 | return $player->canDelete($model, $hard); |
||
| 155 | } |
||
| 156 | |||
| 157 | /** |
||
| 158 | * Find whether a player can create a model |
||
| 159 | * |
||
| 160 | * @param Player $player The player who wants to create a model |
||
| 161 | * @return bool |
||
| 162 | */ |
||
| 163 | 1 | protected function canCreate($player) |
|
| 169 | |||
| 170 | /** |
||
| 171 | * Find whether a player can edit a model |
||
| 172 | * |
||
| 173 | * @param Player $player The player who wants to delete the model |
||
| 174 | * @param PermissionModel $model The model which will be edited |
||
| 175 | * @return bool |
||
| 176 | */ |
||
| 177 | protected function canEdit($player, $model) |
||
| 181 | |||
| 182 | /** |
||
| 183 | * Get a redirection response to a model |
||
| 184 | * |
||
| 185 | * Goes to a list of models of the same type if the provided model does not |
||
| 186 | * have a URL |
||
| 187 | * |
||
| 188 | * @param ModelInterface $model The model to redirect to |
||
| 189 | * @return Response |
||
| 190 | */ |
||
| 191 | protected function redirectTo($model) |
||
| 192 | { |
||
| 193 | if ($model instanceof UrlModel) { |
||
| 194 | return new RedirectResponse($model->getUrl()); |
||
| 195 | } else { |
||
| 196 | return $this->redirectToList($model); |
||
| 197 | } |
||
| 198 | } |
||
| 199 | |||
| 200 | /** |
||
| 201 | * Get a redirection response to a list of models |
||
| 202 | * |
||
| 203 | * @param ModelInterface $model The model to whose list we should redirect |
||
| 204 | * @return Response |
||
| 205 | */ |
||
| 206 | protected function redirectToList($model) |
||
| 207 | { |
||
| 208 | $route = $model->getRouteName('list'); |
||
| 209 | $url = Service::getGenerator()->generate($route); |
||
| 210 | |||
| 211 | return new RedirectResponse($url); |
||
| 212 | } |
||
| 213 | |||
| 214 | /** |
||
| 215 | * Dynamically get the form to show to the user |
||
| 216 | * |
||
| 217 | * @param \Model|null $model The model being edited, `null` if we're creating one |
||
| 218 | * @return ModelFormCreator |
||
| 219 | */ |
||
| 220 | private function getFormCreator($model = null) |
||
| 221 | { |
||
| 222 | $type = ($model instanceof Model) ? $model->getType() : $this->getName(); |
||
| 223 | $type = ucfirst($type); |
||
| 224 | |||
| 225 | $creatorClass = "\\BZIon\\Form\\Creator\\{$type}FormCreator"; |
||
| 226 | $creator = new $creatorClass($model, $this->getMe(), $this); |
||
| 227 | |||
| 228 | return $creator; |
||
| 229 | } |
||
| 230 | |||
| 231 | /** |
||
| 232 | * Get a message to show to the user |
||
| 233 | * @param \ModelInterface|string $model The model (or type) to show a message for |
||
| 234 | * @param string $action The action that will be performed (softDelete, hardDelete, create or edit) |
||
| 235 | * @param string $status The message's status (confirm, error or success) |
||
| 236 | * @return string |
||
| 237 | */ |
||
| 238 | 1 | private function getMessage($model, $action, $status, $escape = true) |
|
| 239 | { |
||
| 240 | 1 | if ($model instanceof Model) { |
|
| 241 | $type = strtolower($model->getTypeForHumans()); |
||
| 242 | |||
| 243 | if ($model instanceof NamedModel) { |
||
| 244 | // Twig will not escape the message on confirmation forms |
||
| 245 | $name = $model->getName(); |
||
| 246 | if ($status == 'confirm') { |
||
| 247 | $name = Model::escape($name); |
||
| 248 | } |
||
| 249 | |||
| 250 | $messages = $this->getMessages($type, $name); |
||
| 251 | |||
| 252 | return $messages[$action][$status]['named']; |
||
| 253 | } else { |
||
| 254 | $messages = $this->getMessages($type); |
||
| 255 | |||
| 256 | return $messages[$action][$status]['unnamed']; |
||
| 257 | } |
||
| 258 | } else { |
||
| 259 | 1 | $messages = $this->getMessages(strtolower($model)); |
|
| 260 | |||
| 261 | 1 | return $messages[$action][$status]; |
|
| 262 | } |
||
| 263 | } |
||
| 264 | |||
| 265 | /** |
||
| 266 | * Get a list of messages to show to the user |
||
| 267 | * @param string $type The type of the model that the message refers to |
||
| 268 | * @param string $name The name of the model |
||
| 269 | * @return array |
||
| 270 | */ |
||
| 271 | 1 | protected function getMessages($type, $name = '') |
|
| 330 | } |
||
| 331 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.