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 MailService 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 MailService, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
28 | class MailService implements MailServiceInterface, EventManagerAwareInterface, MailListenerAwareInterface |
||
29 | { |
||
30 | /** |
||
31 | * @var \Zend\Mail\Message |
||
32 | */ |
||
33 | private $message; |
||
34 | /** |
||
35 | * @var \Zend\Mail\Transport\TransportInterface |
||
36 | */ |
||
37 | private $transport; |
||
38 | /** |
||
39 | * @var RendererInterface |
||
40 | */ |
||
41 | private $renderer; |
||
42 | /** |
||
43 | * @var EventManagerInterface |
||
44 | */ |
||
45 | private $events; |
||
46 | /** |
||
47 | * @var array |
||
48 | */ |
||
49 | private $attachments = []; |
||
50 | /** |
||
51 | * @var DefaultLayoutInterface |
||
52 | */ |
||
53 | private $defaultLayout; |
||
54 | |||
55 | /** |
||
56 | * Creates a new MailService |
||
57 | * @param Message $message |
||
58 | * @param TransportInterface $transport |
||
59 | * @param RendererInterface $renderer Renderer used to render templates, typically a PhpRenderer |
||
60 | */ |
||
61 | 33 | public function __construct(Message $message, TransportInterface $transport, RendererInterface $renderer) |
|
68 | |||
69 | /** |
||
70 | * Returns this service's message |
||
71 | * @return \Zend\Mail\Message |
||
72 | * @see \AcMailer\Service\MailServiceInterface::getMessage() |
||
73 | */ |
||
74 | 13 | public function getMessage() |
|
78 | |||
79 | /** |
||
80 | * Sends the mail |
||
81 | * @return ResultInterface |
||
82 | * @throws MailException |
||
83 | */ |
||
84 | 9 | public function send() |
|
112 | |||
113 | /** |
||
114 | * Creates a new MailEvent object |
||
115 | * @param ResultInterface $result |
||
116 | * @param string $name |
||
117 | * @return MailEvent |
||
118 | */ |
||
119 | 9 | protected function createMailEvent($name = MailEvent::EVENT_MAIL_PRE_SEND, ResultInterface $result = null) |
|
127 | |||
128 | /** |
||
129 | * Creates a error MailResult from an exception |
||
130 | * @param \Exception $e |
||
131 | * @return MailResult |
||
132 | */ |
||
133 | 4 | protected function createMailResultFromException(\Exception $e) |
|
137 | |||
138 | /** |
||
139 | * Sets the message body |
||
140 | * @param \Zend\Mime\Part|\Zend\Mime\Message|string $body Email body |
||
141 | * @param string $charset |
||
142 | * @return $this Returns this MailService for chaining purposes |
||
143 | * @throws InvalidArgumentException |
||
144 | * @see \AcMailer\Service\MailServiceInterface::setBody() |
||
145 | */ |
||
146 | 21 | public function setBody($body, $charset = null) |
|
182 | |||
183 | /** |
||
184 | * Sets the body of this message from a template |
||
185 | * @param string|\Zend\View\Model\ViewModel $template |
||
186 | * @param array $params |
||
187 | * @see \AcMailer\Service\MailServiceInterface::setTemplate() |
||
188 | */ |
||
189 | 5 | public function setTemplate($template, array $params = []) |
|
211 | |||
212 | /** |
||
213 | * Sets the default layout to be used with all the templates set when calling setTemplate. |
||
214 | * |
||
215 | * @param DefaultLayoutInterface $layout |
||
216 | * @return mixed |
||
217 | */ |
||
218 | 33 | public function setDefaultLayout(DefaultLayoutInterface $layout = null) |
|
222 | |||
223 | /** |
||
224 | * Renders template childrens. |
||
225 | * Inspired on Zend\View\View implementation to recursively render child models |
||
226 | * @param ViewModel $model |
||
227 | * @see Zend\View\View::renderChildren |
||
228 | */ |
||
229 | 5 | protected function renderChildren(ViewModel $model) |
|
252 | |||
253 | /** |
||
254 | * Attaches files to the message if any |
||
255 | */ |
||
256 | 9 | protected function attachFiles() |
|
300 | |||
301 | /** |
||
302 | * Sets the message subject |
||
303 | * @param string $subject The subject of the message |
||
304 | * @return $this Returns this MailService for chaining purposes |
||
305 | * @deprecated Use $mailService->getMessage()->setSubject() instead |
||
306 | */ |
||
307 | public function setSubject($subject) |
||
312 | |||
313 | /** |
||
314 | * @param string $path |
||
315 | * @param string|null $filename |
||
316 | * @return $this |
||
317 | */ |
||
318 | 2 | View Code Duplication | public function addAttachment($path, $filename = null) |
327 | |||
328 | /** |
||
329 | * @param array $paths |
||
330 | * @return $this |
||
331 | */ |
||
332 | 12 | public function addAttachments(array $paths) |
|
336 | |||
337 | /** |
||
338 | * @param array $paths |
||
339 | * @return $this |
||
340 | */ |
||
341 | 14 | public function setAttachments(array $paths) |
|
346 | |||
347 | /** |
||
348 | * Returns the list of attachments |
||
349 | * @return array |
||
350 | */ |
||
351 | 2 | public function getAttachments() |
|
355 | |||
356 | /** |
||
357 | * Inject an EventManager instance |
||
358 | * @param EventManagerInterface $events |
||
359 | * @return $this|void |
||
360 | */ |
||
361 | 10 | public function setEventManager(EventManagerInterface $events) |
|
370 | /** |
||
371 | * Retrieve the event manager |
||
372 | * Lazy-loads an EventManager instance if none registered. |
||
373 | * @return EventManagerInterface |
||
374 | */ |
||
375 | 10 | public function getEventManager() |
|
383 | |||
384 | /** |
||
385 | * Attaches a new MailListenerInterface |
||
386 | * @param MailListenerInterface $mailListener |
||
387 | * @param int $priority |
||
388 | * @return mixed|void |
||
389 | */ |
||
390 | 4 | public function attachMailListener(MailListenerInterface $mailListener, $priority = 1) |
|
395 | |||
396 | /** |
||
397 | * Detaches provided MailListener |
||
398 | * @param MailListenerInterface $mailListener |
||
399 | * @return $this |
||
400 | */ |
||
401 | 1 | public function detachMailListener(MailListenerInterface $mailListener) |
|
406 | |||
407 | /** |
||
408 | * @param TransportInterface $transport |
||
409 | * @return $this |
||
410 | */ |
||
411 | 1 | public function setTransport(TransportInterface $transport) |
|
416 | |||
417 | /** |
||
418 | * Returns the transport object that will be used to send the wrapped message |
||
419 | * @return TransportInterface |
||
420 | */ |
||
421 | 5 | public function getTransport() |
|
425 | |||
426 | /** |
||
427 | * @param RendererInterface $renderer |
||
428 | * |
||
429 | * @return $this |
||
430 | */ |
||
431 | 1 | public function setRenderer(RendererInterface $renderer) |
|
436 | |||
437 | /** |
||
438 | * Returns the renderer object that will be used to render templates |
||
439 | * @return RendererInterface |
||
440 | */ |
||
441 | 4 | public function getRenderer() |
|
445 | } |
||
446 |
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.