libgraviton /
graviton
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 RestController 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 RestController, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 46 | class RestController |
||
| 47 | { |
||
| 48 | /** |
||
| 49 | * @var DocumentModel |
||
| 50 | */ |
||
| 51 | private $model; |
||
| 52 | |||
| 53 | /** |
||
| 54 | * @var ContainerInterface service_container |
||
| 55 | */ |
||
| 56 | private $container; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * @var Response |
||
| 60 | */ |
||
| 61 | private $response; |
||
| 62 | |||
| 63 | /** |
||
| 64 | * @var RestUtilsInterface |
||
| 65 | */ |
||
| 66 | private $restUtils; |
||
| 67 | |||
| 68 | /** |
||
| 69 | * @var SchemaUtils |
||
| 70 | */ |
||
| 71 | private $schemaUtils; |
||
| 72 | |||
| 73 | /** |
||
| 74 | * @var Router |
||
| 75 | */ |
||
| 76 | private $router; |
||
| 77 | |||
| 78 | /** |
||
| 79 | * @var EngineInterface |
||
| 80 | */ |
||
| 81 | private $templating; |
||
| 82 | |||
| 83 | /** |
||
| 84 | * @var JsonPatchValidator |
||
| 85 | */ |
||
| 86 | private $jsonPatchValidator; |
||
| 87 | |||
| 88 | /** |
||
| 89 | * @var SecurityUtils |
||
| 90 | */ |
||
| 91 | protected $securityUtils; |
||
| 92 | |||
| 93 | /** |
||
| 94 | * @param Response $response Response |
||
| 95 | * @param RestUtilsInterface $restUtils Rest utils |
||
| 96 | * @param Router $router Router |
||
| 97 | * @param EngineInterface $templating Templating |
||
| 98 | * @param ContainerInterface $container Container |
||
| 99 | * @param SchemaUtils $schemaUtils Schema utils |
||
| 100 | */ |
||
| 101 | public function __construct( |
||
| 102 | Response $response, |
||
| 103 | RestUtilsInterface $restUtils, |
||
| 104 | Router $router, |
||
| 105 | EngineInterface $templating, |
||
| 106 | ContainerInterface $container, |
||
| 107 | SchemaUtils $schemaUtils |
||
| 108 | ) { |
||
| 109 | $this->response = $response; |
||
| 110 | $this->restUtils = $restUtils; |
||
| 111 | $this->router = $router; |
||
| 112 | $this->templating = $templating; |
||
| 113 | $this->container = $container; |
||
| 114 | $this->schemaUtils = $schemaUtils; |
||
| 115 | } |
||
| 116 | |||
| 117 | /** |
||
| 118 | * Setter for the SecurityUtils |
||
| 119 | * |
||
| 120 | * @param SecurityUtils $securityUtils The securityUtils service |
||
| 121 | * @return void |
||
| 122 | */ |
||
| 123 | public function setSecurityUtils(SecurityUtils $securityUtils) |
||
| 124 | { |
||
| 125 | $this->securityUtils = $securityUtils; |
||
| 126 | } |
||
| 127 | |||
| 128 | /** |
||
| 129 | * @param JsonPatchValidator $jsonPatchValidator Service for validation json patch |
||
| 130 | * @return void |
||
| 131 | */ |
||
| 132 | public function setJsonPatchValidator(JsonPatchValidator $jsonPatchValidator) |
||
| 133 | { |
||
| 134 | $this->jsonPatchValidator = $jsonPatchValidator; |
||
| 135 | } |
||
| 136 | |||
| 137 | /** |
||
| 138 | * Get the container object |
||
| 139 | * |
||
| 140 | * @return \Symfony\Component\DependencyInjection\ContainerInterface |
||
| 141 | * |
||
| 142 | * @obsolete |
||
| 143 | */ |
||
| 144 | public function getContainer() |
||
| 145 | { |
||
| 146 | return $this->container; |
||
| 147 | } |
||
| 148 | |||
| 149 | /** |
||
| 150 | * Returns a single record |
||
| 151 | * |
||
| 152 | * @param Request $request Current http request |
||
| 153 | * @param string $id ID of record |
||
| 154 | * |
||
| 155 | * @return \Symfony\Component\HttpFoundation\Response $response Response with result or error |
||
| 156 | */ |
||
| 157 | View Code Duplication | public function getAction(Request $request, $id) |
|
|
|
|||
| 158 | { |
||
| 159 | $response = $this->getResponse() |
||
| 160 | ->setStatusCode(Response::HTTP_OK) |
||
| 161 | ->setContent($this->serialize($this->findRecord($id, $request))); |
||
| 162 | |||
| 163 | return $response; |
||
| 164 | } |
||
| 165 | |||
| 166 | /** |
||
| 167 | * Get the response object |
||
| 168 | * |
||
| 169 | * @return \Symfony\Component\HttpFoundation\Response $response Response object |
||
| 170 | */ |
||
| 171 | public function getResponse() |
||
| 172 | { |
||
| 173 | return $this->response; |
||
| 174 | } |
||
| 175 | |||
| 176 | /** |
||
| 177 | * Get a single record from database or throw an exception if it doesn't exist |
||
| 178 | * |
||
| 179 | * @param mixed $id Record id |
||
| 180 | * @param Request $request request |
||
| 181 | * |
||
| 182 | * @throws \Graviton\ExceptionBundle\Exception\NotFoundException |
||
| 183 | * |
||
| 184 | * @return object $record Document object |
||
| 185 | */ |
||
| 186 | protected function findRecord($id, Request $request = null) |
||
| 187 | { |
||
| 188 | $response = $this->getResponse(); |
||
| 189 | |||
| 190 | if (!($this->getModel()->recordExists($id))) { |
||
| 191 | $e = new NotFoundException("Entry with id " . $id . " not found!"); |
||
| 192 | $e->setResponse($response); |
||
| 193 | throw $e; |
||
| 194 | } |
||
| 195 | |||
| 196 | return $this->getModel()->find($id, $request); |
||
| 197 | } |
||
| 198 | |||
| 199 | /** |
||
| 200 | * Return the model |
||
| 201 | * |
||
| 202 | * @throws \Exception in case no model was defined. |
||
| 203 | * |
||
| 204 | * @return DocumentModel $model Model |
||
| 205 | */ |
||
| 206 | public function getModel() |
||
| 214 | |||
| 215 | /** |
||
| 216 | * Set the model class |
||
| 217 | * |
||
| 218 | * @param DocumentModel $model Model class |
||
| 219 | * |
||
| 220 | * @return self |
||
| 221 | */ |
||
| 222 | public function setModel(DocumentModel $model) |
||
| 228 | |||
| 229 | /** |
||
| 230 | * Serialize the given record and throw an exception if something went wrong |
||
| 231 | * |
||
| 232 | * @param object|object[] $result Record(s) |
||
| 233 | * |
||
| 234 | * @throws \Graviton\ExceptionBundle\Exception\SerializationException |
||
| 235 | * |
||
| 236 | * @return string $content Json content |
||
| 237 | */ |
||
| 238 | protected function serialize($result) |
||
| 239 | { |
||
| 240 | $response = $this->getResponse(); |
||
| 241 | |||
| 242 | try { |
||
| 243 | // array is serialized as an object {"0":{...},"1":{...},...} when data contains an empty objects |
||
| 244 | // we serialize each item because we can assume this bug affects only root array element |
||
| 245 | if (is_array($result) && array_keys($result) === range(0, count($result) - 1)) { |
||
| 246 | $result = array_map( |
||
| 247 | function ($item) { |
||
| 248 | return $this->getRestUtils()->serializeContent($item); |
||
| 249 | }, |
||
| 250 | $result |
||
| 251 | ); |
||
| 252 | |||
| 253 | return '['.implode(',', array_filter($result)).']'; |
||
| 254 | } |
||
| 255 | |||
| 256 | return $this->getRestUtils()->serializeContent($result); |
||
| 257 | } catch (\Exception $e) { |
||
| 258 | $exception = new SerializationException($e); |
||
| 259 | $exception->setResponse($response); |
||
| 260 | throw $exception; |
||
| 261 | } |
||
| 262 | } |
||
| 263 | |||
| 264 | /** |
||
| 265 | * Get RestUtils service |
||
| 266 | * |
||
| 267 | * @return \Graviton\RestBundle\Service\RestUtils |
||
| 268 | */ |
||
| 269 | public function getRestUtils() |
||
| 273 | |||
| 274 | /** |
||
| 275 | * Returns all records |
||
| 276 | * |
||
| 277 | * @param Request $request Current http request |
||
| 278 | * |
||
| 279 | * @return \Symfony\Component\HttpFoundation\Response $response Response with result or error |
||
| 280 | */ |
||
| 281 | View Code Duplication | public function allAction(Request $request) |
|
| 291 | |||
| 292 | /** |
||
| 293 | * Writes a new Entry to the database |
||
| 294 | * |
||
| 295 | * @param Request $request Current http request |
||
| 296 | * |
||
| 297 | * @return \Symfony\Component\HttpFoundation\Response $response Result of action with data (if successful) |
||
| 298 | */ |
||
| 299 | public function postAction(Request $request) |
||
| 325 | |||
| 326 | /** |
||
| 327 | * Validates the current request on schema violations. If there are errors, |
||
| 328 | * the exception is thrown. If not, the deserialized record is returned. |
||
| 329 | * |
||
| 330 | * @param object|string $content \stdClass of the request content |
||
| 331 | * @param DocumentModel $model the model to check the schema for |
||
| 332 | * |
||
| 333 | * @return \Graviton\JsonSchemaBundle\Exception\ValidationExceptionError[] |
||
| 334 | * @throws \Exception |
||
| 335 | */ |
||
| 336 | protected function validateRequest($content, DocumentModel $model) |
||
| 344 | |||
| 345 | /** |
||
| 346 | * Deserialize the given content throw an exception if something went wrong |
||
| 347 | * |
||
| 348 | * @param string $content Request content |
||
| 349 | * @param string $documentClass Document class |
||
| 350 | * |
||
| 351 | * @throws DeserializationException |
||
| 352 | * |
||
| 353 | * @return object $record Document |
||
| 354 | */ |
||
| 355 | protected function deserialize($content, $documentClass) |
||
| 377 | |||
| 378 | /** |
||
| 379 | * Get the router from the dic |
||
| 380 | * |
||
| 381 | * @return Router |
||
| 382 | */ |
||
| 383 | public function getRouter() |
||
| 387 | |||
| 388 | /** |
||
| 389 | * Update a record |
||
| 390 | * |
||
| 391 | * @param Number $id ID of record |
||
| 392 | * @param Request $request Current http request |
||
| 393 | * |
||
| 394 | * @throws MalformedInputException |
||
| 395 | * |
||
| 396 | * @return Response $response Result of action with data (if successful) |
||
| 397 | */ |
||
| 398 | public function putAction($id, Request $request) |
||
| 433 | |||
| 434 | /** |
||
| 435 | * Patch a record |
||
| 436 | * |
||
| 437 | * @param Number $id ID of record |
||
| 438 | * @param Request $request Current http request |
||
| 439 | * |
||
| 440 | * @throws MalformedInputException |
||
| 441 | * |
||
| 442 | * @return Response $response Result of action with data (if successful) |
||
| 443 | */ |
||
| 444 | public function patchAction($id, Request $request) |
||
| 493 | |||
| 494 | /** |
||
| 495 | * Deletes a record |
||
| 496 | * |
||
| 497 | * @param Number $id ID of record |
||
| 498 | * |
||
| 499 | * @return Response $response Result of the action |
||
| 500 | */ |
||
| 501 | public function deleteAction($id) |
||
| 513 | |||
| 514 | /** |
||
| 515 | * Return OPTIONS results. |
||
| 516 | * |
||
| 517 | * @param Request $request Current http request |
||
| 518 | * |
||
| 519 | * @throws SerializationException |
||
| 520 | * @return \Symfony\Component\HttpFoundation\Response $response Result of the action |
||
| 521 | */ |
||
| 522 | public function optionsAction(Request $request) |
||
| 543 | |||
| 544 | |||
| 545 | /** |
||
| 546 | * Return schema GET results. |
||
| 547 | * |
||
| 548 | * @param Request $request Current http request |
||
| 549 | * @param string $id ID of record |
||
| 550 | * |
||
| 551 | * @throws SerializationException |
||
| 552 | * @return \Symfony\Component\HttpFoundation\Response $response Result of the action |
||
| 553 | */ |
||
| 554 | public function schemaAction(Request $request, $id = null) |
||
| 589 | |||
| 590 | /** |
||
| 591 | * Renders a view. |
||
| 592 | * |
||
| 593 | * @param string $view The view name |
||
| 594 | * @param array $parameters An array of parameters to pass to the view |
||
| 595 | * @param Response $response A response instance |
||
| 596 | * |
||
| 597 | * @return Response A Response instance |
||
| 598 | */ |
||
| 599 | public function render($view, array $parameters = array(), Response $response = null) |
||
| 603 | |||
| 604 | /** |
||
| 605 | * @param Request $request request |
||
| 606 | * @return string |
||
| 607 | */ |
||
| 608 | private function getRouteName(Request $request) |
||
| 620 | |||
| 621 | /** |
||
| 622 | * Security needs to be enabled to get Object. |
||
| 623 | * |
||
| 624 | * @return SecurityUser |
||
| 625 | * @throws UsernameNotFoundException |
||
| 626 | */ |
||
| 627 | public function getSecurityUser() |
||
| 631 | } |
||
| 632 |
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.