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 |
||
| 30 | class EventStatusLinkResponseListener |
||
| 31 | { |
||
| 32 | |||
| 33 | /** |
||
| 34 | * @var ProducerInterface Producer for publishing messages. |
||
| 35 | */ |
||
| 36 | private $rabbitMqProducer = null; |
||
| 37 | |||
| 38 | /** |
||
| 39 | * @var RouterInterface Router to generate resource URLs |
||
| 40 | */ |
||
| 41 | private $router = null; |
||
| 42 | |||
| 43 | /** |
||
| 44 | * @var Request request |
||
| 45 | */ |
||
| 46 | private $request; |
||
| 47 | |||
| 48 | /** |
||
| 49 | * @var QueueEvent queueevent document |
||
| 50 | */ |
||
| 51 | private $queueEventDocument; |
||
| 52 | |||
| 53 | /** |
||
| 54 | * @var array |
||
| 55 | */ |
||
| 56 | private $eventMap; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * @var ExtReferenceConverter ExtReferenceConverter |
||
| 60 | */ |
||
| 61 | private $extRefConverter; |
||
| 62 | |||
| 63 | /** |
||
| 64 | * @var string classname of the EventWorker document |
||
| 65 | */ |
||
| 66 | private $eventWorkerClassname; |
||
| 67 | |||
| 68 | /** |
||
| 69 | * @var string classname of the EventStatus document |
||
| 70 | */ |
||
| 71 | private $eventStatusClassname; |
||
| 72 | |||
| 73 | /** |
||
| 74 | * @var string classname of the EventStatusStatus document |
||
| 75 | */ |
||
| 76 | private $eventStatusStatusClassname; |
||
| 77 | |||
| 78 | /** |
||
| 79 | * @var string classname of the EventStatusEventResource document |
||
| 80 | */ |
||
| 81 | private $eventStatusEventResourceClassname; |
||
| 82 | |||
| 83 | /** |
||
| 84 | * @var string route name of the /event/status route |
||
| 85 | */ |
||
| 86 | private $eventStatusRouteName; |
||
| 87 | |||
| 88 | /** |
||
| 89 | * @var DocumentManager Document manager |
||
| 90 | */ |
||
| 91 | private $documentManager; |
||
| 92 | |||
| 93 | /** |
||
| 94 | * @var TokenStorage |
||
| 95 | */ |
||
| 96 | protected $tokenStorage; |
||
| 97 | |||
| 98 | /** |
||
| 99 | * @param ProducerInterface $rabbitMqProducer RabbitMQ dependency |
||
| 100 | * @param RouterInterface $router Router dependency |
||
| 101 | * @param RequestStack $requestStack Request stack |
||
| 102 | * @param DocumentManager $documentManager Doctrine document manager |
||
| 103 | * @param ExtReferenceConverter $extRefConverter instance of the ExtReferenceConverter service |
||
| 104 | * @param QueueEvent $queueEventDocument queueevent document |
||
| 105 | * @param array $eventMap eventmap |
||
| 106 | * @param string $eventWorkerClassname classname of the EventWorker document |
||
| 107 | * @param string $eventStatusClassname classname of the EventStatus document |
||
| 108 | * @param string $eventStatusStatusClassname classname of the EventStatusStatus document |
||
| 109 | * @param string $eventStatusEventResourceClassname classname of the E*S*E*Resource document |
||
| 110 | * @param string $eventStatusRouteName name of the route to EventStatus |
||
| 111 | * @param TokenStorage $tokenStorage Security service |
||
| 112 | */ |
||
| 113 | 2 | public function __construct( |
|
| 142 | |||
| 143 | /** |
||
| 144 | * add a rel=eventStatus Link header to the response if necessary |
||
| 145 | * |
||
| 146 | * @param FilterResponseEvent $event response listener event |
||
| 147 | * |
||
| 148 | * @return void |
||
| 149 | */ |
||
| 150 | 2 | public function onKernelResponse(FilterResponseEvent $event) |
|
| 191 | |||
| 192 | /** |
||
| 193 | * we only want to do something if we have a mapped event.. |
||
| 194 | * |
||
| 195 | * @return boolean true if it should not concern us, false otherwise |
||
| 196 | */ |
||
| 197 | 2 | private function isNotConcerningRequest() |
|
| 201 | |||
| 202 | /** |
||
| 203 | * Creates the structured object that will be sent to the queue (eventually..) |
||
| 204 | * |
||
| 205 | * @return QueueEvent event |
||
| 206 | */ |
||
| 207 | 2 | private function createQueueEventObject() |
|
| 217 | |||
| 218 | /** |
||
| 219 | * compose our routingKey. this will have the form of 'document.[bundle].[document].[event]' |
||
| 220 | * rules: |
||
| 221 | * * always 4 parts divided by points. |
||
| 222 | * * in this context (doctrine/odm stuff) we prefix with 'document.' |
||
| 223 | * |
||
| 224 | * @return string routing key |
||
| 225 | */ |
||
| 226 | 2 | private function generateRoutingKey() |
|
| 246 | |||
| 247 | /** |
||
| 248 | * Creates a EventStatus object that gets persisted.. |
||
| 249 | * |
||
| 250 | * @param QueueEvent $queueEvent queueEvent object |
||
| 251 | * |
||
| 252 | * @return string |
||
| 253 | */ |
||
| 254 | 2 | private function getStatusUrl($queueEvent) |
|
| 300 | |||
| 301 | /** |
||
| 302 | * Checks EventWorker for worker that are subscribed to our event and returns |
||
| 303 | * their workerIds as array |
||
| 304 | |||
| 305 | * @param QueueEvent $queueEvent queueEvent object |
||
| 306 | * |
||
| 307 | * @return array array of worker ids |
||
| 308 | */ |
||
| 309 | 2 | private function getSubscribedWorkerIds(QueueEvent $queueEvent) |
|
| 339 | |||
| 340 | /** |
||
| 341 | * Security needs to be enabled to get |
||
| 342 | * |
||
| 343 | * @return String |
||
| 344 | */ |
||
| 345 | 2 | View Code Duplication | private function getSecurityUsername() |
| 355 | } |
||
| 356 |
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 implementation 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 interface: