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 VCard 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 VCard, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
19 | class VCard |
||
20 | { |
||
21 | /** |
||
22 | * definedElements |
||
23 | * |
||
24 | * @var array |
||
25 | */ |
||
26 | private $definedElements; |
||
27 | |||
28 | /** |
||
29 | * Filename |
||
30 | * |
||
31 | * @var string |
||
32 | */ |
||
33 | private $filename; |
||
34 | |||
35 | /** |
||
36 | * Multiple properties for element allowed |
||
37 | * |
||
38 | * @var array |
||
39 | */ |
||
40 | private $multiplePropertiesForElementAllowed = array( |
||
41 | 'email', |
||
42 | 'address', |
||
43 | 'phoneNumber', |
||
44 | 'url', |
||
45 | 'label' |
||
46 | ); |
||
47 | |||
48 | /** |
||
49 | * Properties |
||
50 | * |
||
51 | * @var array |
||
52 | */ |
||
53 | private $properties; |
||
54 | |||
55 | /** |
||
56 | * Default Charset |
||
57 | * |
||
58 | * @var string |
||
59 | */ |
||
60 | public $charset = 'utf-8'; |
||
61 | |||
62 | /** |
||
63 | * Add address |
||
64 | * |
||
65 | * @param string [optional] $name |
||
66 | * @param string [optional] $extended |
||
67 | * @param string [optional] $street |
||
68 | * @param string [optional] $city |
||
69 | * @param string [optional] $region |
||
70 | * @param string [optional] $zip |
||
71 | * @param string [optional] $country |
||
72 | * @param string [optional] $type |
||
73 | * $type may be DOM | INTL | POSTAL | PARCEL | HOME | WORK |
||
74 | * or any combination of these: e.g. "WORK;PARCEL;POSTAL" |
||
75 | * @return $this |
||
76 | */ |
||
77 | public function addAddress( |
||
99 | |||
100 | /** |
||
101 | * Add birthday |
||
102 | * |
||
103 | * @param string $date Format is YYYY-MM-DD |
||
104 | * @return $this |
||
105 | */ |
||
106 | public function addBirthday($date) |
||
116 | |||
117 | /** |
||
118 | * Add company |
||
119 | * |
||
120 | * @param string $company |
||
121 | * @return $this |
||
122 | */ |
||
123 | public function addCompany($company) |
||
138 | |||
139 | /** |
||
140 | * Add email |
||
141 | * |
||
142 | * @param string $address The e-mail address |
||
143 | * @param string [optional] $type The type of the email address |
||
144 | * $type may be PREF | WORK | HOME |
||
145 | * or any combination of these: e.g. "PREF;WORK" |
||
146 | * @return $this |
||
147 | */ |
||
148 | public function addEmail($address, $type = '') |
||
158 | |||
159 | /** |
||
160 | * Add jobtitle |
||
161 | * |
||
162 | * @param string $jobtitle The jobtitle for the person. |
||
163 | * @return $this |
||
164 | */ |
||
165 | public function addJobtitle($jobtitle) |
||
175 | |||
176 | /** |
||
177 | * Add a label |
||
178 | * |
||
179 | * @param string $label |
||
180 | * @param string $type |
||
181 | * |
||
182 | * @return $this |
||
183 | */ |
||
184 | View Code Duplication | public function addLabel($label, $type = '') |
|
194 | |||
195 | /** |
||
196 | * Add a photo or logo (depending on property name) |
||
197 | * |
||
198 | * @param string $property LOGO|PHOTO |
||
199 | * @param string $url image url or filename |
||
200 | * @param bool $include Do we include the image in our vcard or not? |
||
201 | * @throws VCardMediaException if file is empty or not an image file |
||
202 | */ |
||
203 | private function addMedia($property, $url, $include = true, $element) |
||
235 | |||
236 | /** |
||
237 | * Add name |
||
238 | * |
||
239 | * @param string [optional] $lastName |
||
240 | * @param string [optional] $firstName |
||
241 | * @param string [optional] $additional |
||
242 | * @param string [optional] $prefix |
||
243 | * @param string [optional] $suffix |
||
244 | * @return $this |
||
245 | */ |
||
246 | public function addName( |
||
285 | |||
286 | /** |
||
287 | * Add note |
||
288 | * |
||
289 | * @param string $note |
||
290 | * @return $this |
||
291 | */ |
||
292 | public function addNote($note) |
||
302 | |||
303 | /** |
||
304 | * Add phone number |
||
305 | * |
||
306 | * @param string $number |
||
307 | * @param string [optional] $type |
||
308 | * Type may be PREF | WORK | HOME | VOICE | FAX | MSG | |
||
309 | * CELL | PAGER | BBS | CAR | MODEM | ISDN | VIDEO |
||
310 | * or any senseful combination, e.g. "PREF;WORK;VOICE" |
||
311 | * @return $this |
||
312 | */ |
||
313 | public function addPhoneNumber($number, $type = '') |
||
323 | |||
324 | /** |
||
325 | * Add Photo |
||
326 | * |
||
327 | * @param string $url image url or filename |
||
328 | * @param bool $include Include the image in our vcard? |
||
329 | * @return $this |
||
330 | */ |
||
331 | public function addPhoto($url, $include = true) |
||
342 | |||
343 | /** |
||
344 | * Add URL |
||
345 | * |
||
346 | * @param string $url |
||
347 | * @param string [optional] $type Type may be WORK | HOME |
||
348 | * @return $this |
||
349 | */ |
||
350 | View Code Duplication | public function addURL($url, $type = '') |
|
360 | |||
361 | /** |
||
362 | * Build VCard (.vcf) |
||
363 | * |
||
364 | * @return string |
||
365 | */ |
||
366 | public function buildVCard() |
||
386 | |||
387 | /** |
||
388 | * Build VCalender (.ics) - Safari (< iOS 8) can not open .vcf files, so we have build a workaround. |
||
389 | * |
||
390 | * @return string |
||
391 | */ |
||
392 | public function buildVCalendar() |
||
427 | |||
428 | /** |
||
429 | * Returns the browser user agent string. |
||
430 | * |
||
431 | * @return string |
||
432 | */ |
||
433 | protected function getUserAgent() |
||
443 | |||
444 | /** |
||
445 | * Decode |
||
446 | * |
||
447 | * @param string $value The value to decode |
||
448 | * @return string decoded |
||
449 | */ |
||
450 | private function decode($value) |
||
455 | |||
456 | /** |
||
457 | * Download a vcard or vcal file to the browser. |
||
458 | */ |
||
459 | public function download() |
||
471 | |||
472 | /** |
||
473 | * Fold a line according to RFC2425 section 5.8.1. |
||
474 | * |
||
475 | * @link http://tools.ietf.org/html/rfc2425#section-5.8.1 |
||
476 | * @param string $text |
||
477 | * @return mixed |
||
478 | */ |
||
479 | protected function fold($text) |
||
488 | |||
489 | /** |
||
490 | * Get output as string |
||
491 | * @deprecated in the future |
||
492 | * |
||
493 | * @return string |
||
494 | */ |
||
495 | public function get() |
||
499 | |||
500 | /** |
||
501 | * Get charset |
||
502 | * |
||
503 | * @return string |
||
504 | */ |
||
505 | public function getCharset() |
||
509 | |||
510 | /** |
||
511 | * Get content type |
||
512 | * |
||
513 | * @return string |
||
514 | */ |
||
515 | public function getContentType() |
||
520 | |||
521 | /** |
||
522 | * Get filename |
||
523 | * |
||
524 | * @return string |
||
525 | */ |
||
526 | public function getFilename() |
||
530 | |||
531 | /** |
||
532 | * Get file extension |
||
533 | * |
||
534 | * @return string |
||
535 | */ |
||
536 | public function getFileExtension() |
||
541 | |||
542 | /** |
||
543 | * Get headers |
||
544 | * |
||
545 | * @param bool $asAssociative |
||
546 | * @return array |
||
547 | */ |
||
548 | public function getHeaders($asAssociative) |
||
571 | |||
572 | /** |
||
573 | * Get output as string |
||
574 | * iOS devices (and safari < iOS 8 in particular) can not read .vcf (= vcard) files. |
||
575 | * So I build a workaround to build a .ics (= vcalender) file. |
||
576 | * |
||
577 | * @return string |
||
578 | */ |
||
579 | public function getOutput() |
||
591 | |||
592 | /** |
||
593 | * Get properties |
||
594 | * |
||
595 | * @return array |
||
596 | */ |
||
597 | public function getProperties() |
||
601 | |||
602 | /** |
||
603 | * Has property |
||
604 | * |
||
605 | * @param string $key |
||
606 | * @return bool |
||
607 | */ |
||
608 | public function hasProperty($key) |
||
620 | |||
621 | /** |
||
622 | * Is iOS - Check if the user is using an iOS-device |
||
623 | * |
||
624 | * @return bool |
||
625 | */ |
||
626 | public function isIOS() |
||
633 | |||
634 | /** |
||
635 | * Is iOS less than 7 (should cal wrapper be returned) |
||
636 | * |
||
637 | * @return bool |
||
638 | */ |
||
639 | public function isIOS7() |
||
643 | |||
644 | /** |
||
645 | * Save to a file |
||
646 | * |
||
647 | * @return void |
||
648 | */ |
||
649 | public function save() |
||
658 | |||
659 | /** |
||
660 | * Set charset |
||
661 | * |
||
662 | * @param mixed $charset |
||
663 | * @return void |
||
664 | */ |
||
665 | public function setCharset($charset) |
||
669 | |||
670 | /** |
||
671 | * Set filename |
||
672 | * |
||
673 | * @param mixed $value |
||
674 | * @param bool $overwrite [optional] Default overwrite is true |
||
675 | * @param string $separator [optional] Default separator is an underscore '_' |
||
676 | * @return void |
||
677 | */ |
||
678 | public function setFilename($value, $overwrite = true, $separator = '_') |
||
706 | |||
707 | /** |
||
708 | * Set property |
||
709 | * |
||
710 | * @param string $element The element name you want to set, f.e.: name, email, phoneNumber, ... |
||
711 | * @param string $key |
||
712 | * @param string $value |
||
713 | * @return void |
||
714 | */ |
||
715 | private function setProperty($element, $key, $value) |
||
732 | |||
733 | /** |
||
734 | * Checks if we should return vcard in cal wrapper |
||
735 | * |
||
736 | * @return bool |
||
737 | */ |
||
738 | protected function shouldAttachmentBeCal() |
||
748 | } |
||
749 |
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.