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 PMF_Mail 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 PMF_Mail, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
37 | class PMF_Mail |
||
38 | { |
||
39 | /** |
||
40 | * Type of the used MUA. Possible values: |
||
41 | * - built-in. |
||
42 | * |
||
43 | * @var string $agent |
||
44 | */ |
||
45 | public $agent; |
||
46 | |||
47 | /** |
||
48 | * Attached filed. |
||
49 | * |
||
50 | * @var mixed $attachments |
||
51 | */ |
||
52 | public $attachments; |
||
53 | |||
54 | /** |
||
55 | * Body of the e-mail. |
||
56 | * |
||
57 | * @var string $body |
||
58 | */ |
||
59 | public $body = ''; |
||
60 | |||
61 | /** |
||
62 | * Boundary. |
||
63 | * |
||
64 | * @var string $boundary |
||
65 | */ |
||
66 | public $boundary = '----------'; |
||
67 | |||
68 | /** |
||
69 | * Charset. |
||
70 | * |
||
71 | * @var string $charset |
||
72 | */ |
||
73 | public $charset = 'utf-8'; |
||
74 | |||
75 | /** |
||
76 | * Content disposition. |
||
77 | * |
||
78 | * @var string $contentDisposition |
||
79 | */ |
||
80 | public $contentDisposition = 'inline'; |
||
81 | |||
82 | /** |
||
83 | * Content type. |
||
84 | * |
||
85 | * @var string $contentType |
||
86 | */ |
||
87 | public $contentType = 'text/plain'; |
||
88 | |||
89 | /** |
||
90 | * Content transfer encoding. |
||
91 | * |
||
92 | * @var string $contentTransferEncoding |
||
93 | */ |
||
94 | public $contentTransferEncoding = '8bit'; |
||
95 | |||
96 | /** |
||
97 | * The one and only valid End Of Line sequence as per RFC 2822: |
||
98 | * carriage-return followed by line-feed. |
||
99 | * |
||
100 | * @var string $eol |
||
101 | */ |
||
102 | public $eol = "\r\n"; |
||
103 | |||
104 | /** |
||
105 | * Headers of the e-mail. |
||
106 | * |
||
107 | * @var string $headers |
||
108 | */ |
||
109 | public $headers; |
||
110 | |||
111 | /** |
||
112 | * Message of the e-mail: HTML text allowed. |
||
113 | * |
||
114 | * @var string $message |
||
115 | */ |
||
116 | public $message; |
||
117 | |||
118 | /** |
||
119 | * Alternate message of the e-mail: only plain text allowed. |
||
120 | * |
||
121 | * @var string $messageAlt |
||
122 | */ |
||
123 | public $messageAlt; |
||
124 | |||
125 | /** |
||
126 | * Message-ID of the e-mail. |
||
127 | * |
||
128 | * @var string $messageId |
||
129 | */ |
||
130 | public $messageId; |
||
131 | |||
132 | /** |
||
133 | * Priorities: 1 (Highest), 2 (High), 3 (Normal), 4 (Low), 5 (Lowest). |
||
134 | * |
||
135 | * @var mixed $priorities |
||
136 | */ |
||
137 | public $priorities = array( |
||
138 | 1 => 'Highest', |
||
139 | 2 => 'High', |
||
140 | 3 => 'Normal', |
||
141 | 4 => 'Low', |
||
142 | 5 => 'Lowest'); |
||
143 | |||
144 | /** |
||
145 | * Priority of the e-mail: 1 (Highest), 2 (High), 3 (Normal), 4 (Low), 5 (Lowest). |
||
146 | * |
||
147 | * @var int $priority |
||
148 | * @see priorities |
||
149 | */ |
||
150 | public $priority; |
||
151 | |||
152 | /** |
||
153 | * Subject of the e-mail. |
||
154 | * |
||
155 | * @var string $subject |
||
156 | */ |
||
157 | public $subject; |
||
158 | |||
159 | /** |
||
160 | * Recipients of the e-mail as <BCC>. |
||
161 | * |
||
162 | * @var mixed $_bcc |
||
163 | */ |
||
164 | private $_bcc; |
||
165 | |||
166 | /** |
||
167 | * Recipients of the e-mail as <CC>. |
||
168 | * |
||
169 | * @var mixed $_cc |
||
170 | */ |
||
171 | private $_cc; |
||
172 | |||
173 | /** |
||
174 | * Recipients of the e-mail as <From>. |
||
175 | * |
||
176 | * @var mixed $_from |
||
177 | */ |
||
178 | private $_from; |
||
179 | |||
180 | /** |
||
181 | * Mailer string. |
||
182 | * |
||
183 | * @var string $_mailer |
||
184 | */ |
||
185 | private $_mailer; |
||
186 | |||
187 | /** |
||
188 | * Recipient of the optional notification. |
||
189 | * |
||
190 | * @var mixed $_notifyTo |
||
191 | */ |
||
192 | private $_notifyTo; |
||
193 | |||
194 | /** |
||
195 | * Recipient of the e-mail as <Reply-To>. |
||
196 | * |
||
197 | * @var mixed $_replyTo |
||
198 | */ |
||
199 | private $_replyTo; |
||
200 | |||
201 | /** |
||
202 | * Recipient of the e-mail as <Return-Path>. |
||
203 | * |
||
204 | * @var mixed $_returnPath |
||
205 | */ |
||
206 | private $_returnPath; |
||
207 | |||
208 | /** |
||
209 | * Recipient of the e-mail as <Sender>. |
||
210 | * |
||
211 | * @var mixed $_sender |
||
212 | */ |
||
213 | private $_sender; |
||
214 | |||
215 | /** |
||
216 | * Recipients of the e-mail as <TO:>. |
||
217 | * |
||
218 | * @var mixed $_to |
||
219 | */ |
||
220 | private $_to; |
||
221 | |||
222 | /** |
||
223 | * @var PMF_Configuration |
||
224 | */ |
||
225 | private $_config; |
||
226 | |||
227 | /* |
||
228 | * Default constructor. |
||
229 | * Note: any email will be sent from the PMF administrator, use unsetFrom |
||
230 | * before using setFrom. |
||
231 | * |
||
232 | * @param Configuration $config |
||
233 | */ |
||
234 | function __construct(PMF_Configuration $config) |
||
235 | { |
||
236 | // Set default value for public properties |
||
237 | $this->agent = 'built-in'; |
||
238 | $this->attachments = array(); |
||
239 | $this->boundary = self::createBoundary(); |
||
240 | $this->headers = array(); |
||
241 | $this->message = ''; |
||
242 | $this->messageAlt = ''; |
||
243 | $this->messageId = '<'.$_SERVER['REQUEST_TIME'] . '.'. md5(microtime()) . '@' . self::getServerName() . '>'; |
||
244 | $this->priority = 3; // 3 -> Normal |
||
245 | $this->subject = ''; |
||
246 | |||
247 | // Set default value for private properties |
||
248 | $this->_config = $config; |
||
249 | $this->_bcc = array(); |
||
250 | $this->_cc = array(); |
||
251 | $this->_from = array(); |
||
252 | $this->_mailer = 'phpMyFAQ on PHP/' . PHP_VERSION; |
||
253 | $this->_notifyTo = array(); |
||
254 | $this->_replyTo = array(); |
||
255 | $this->_returnPath = array(); |
||
256 | $this->_sender = array(); |
||
257 | $this->_to = array(); |
||
258 | |||
259 | // Set phpMyFAQ related data |
||
260 | $this->_mailer = 'phpMyFAQ/' . $this->_config->get('main.currentVersion'); |
||
261 | $this->setFrom($this->_config->get('main.administrationMail')); |
||
262 | } |
||
263 | |||
264 | /** |
||
265 | * Add an e-mail address to an array. |
||
266 | * |
||
267 | * @param array $target Target array. |
||
268 | * @param string $targetAlias Alias Target alias. |
||
269 | * @param string $address User e-mail address. |
||
270 | * @param string $name User name (optional). |
||
271 | * |
||
272 | * @return bool True if successful, false otherwise. |
||
273 | * |
||
274 | * @todo Enhance error handling using exceptions |
||
275 | */ |
||
276 | private function _addEmailTo(&$target, $targetAlias, $address, $name = null) |
||
319 | |||
320 | /** |
||
321 | * Create the body of the email. |
||
322 | * |
||
323 | * @return void |
||
324 | */ |
||
325 | private function _createBody() |
||
406 | |||
407 | /** |
||
408 | * Create the headers of the email. |
||
409 | * |
||
410 | * @return void |
||
411 | */ |
||
412 | private function _createHeaders() |
||
507 | |||
508 | /** |
||
509 | * Set just one e-mail address into an array. |
||
510 | * |
||
511 | * @param array $target Target array. |
||
512 | * @param string $targetAlias Alias Target alias. |
||
513 | * @param string $address User e-mail address. |
||
514 | * @param string $name User name (optional). |
||
515 | * |
||
516 | * @return bool True if successful, false otherwise. |
||
517 | */ |
||
518 | private function _setEmailTo(&$target, $targetAlias, $address, $name = null) |
||
532 | |||
533 | /** |
||
534 | * Add an attachment. |
||
535 | * |
||
536 | * @param string $path File path. |
||
537 | * @param string $name File name. Defaults to the basename. |
||
538 | * @param string $mimetype File MIME type. Defaults to 'application/octet-stream'. |
||
539 | * @param string $disposition Attachment disposition. Defaults to 'attachment'. |
||
540 | * @param string $cid Content ID, required when disposition is 'inline'. Defaults to ''. |
||
541 | * @return bool True if successful, false otherwise. |
||
542 | */ |
||
543 | public function addAttachment($path, $name = null, $mimetype = 'application/octet-stream', $disposition = 'attachment', $cid = '') |
||
567 | |||
568 | /** |
||
569 | * Add a recipient as <BCC>. |
||
570 | * |
||
571 | * @param string $address User e-mail address. |
||
572 | * @param string $name User name (optional). |
||
573 | * @return bool True if successful, false otherwise. |
||
574 | */ |
||
575 | public function addBcc($address, $name = null) |
||
579 | |||
580 | /** |
||
581 | * Add a recipient as <CC>. |
||
582 | * |
||
583 | * @param string $address User e-mail address. |
||
584 | * @param string $name User name (optional). |
||
585 | * @return bool True if successful, false otherwise. |
||
586 | */ |
||
587 | public function addCc($address, $name = null) |
||
591 | |||
592 | /** |
||
593 | * Add an address to send a notification to. |
||
594 | * |
||
595 | * @param string $address User e-mail address. |
||
596 | * @param string $name User name (optional). |
||
597 | * @return bool True if successful, false otherwise. |
||
598 | */ |
||
599 | public function addNotificationTo($address, $name = null) |
||
603 | |||
604 | /** |
||
605 | * Add a recipient as <TO>. |
||
606 | * |
||
607 | * @param string $address User e-mail address. |
||
608 | * @param string $name User name (optional). |
||
609 | * @return bool True if successful, false otherwise. |
||
610 | */ |
||
611 | public function addTo($address, $name = null) |
||
615 | |||
616 | /** |
||
617 | * Create a string to be used as a valid boundary value. |
||
618 | * |
||
619 | * @static |
||
620 | * @return string The boundary value. |
||
621 | */ |
||
622 | public static function createBoundary() |
||
626 | |||
627 | /** |
||
628 | * Returns the given text being sure that any CR or LF has been fixed |
||
629 | * according with RFC 2822 EOL setting. |
||
630 | * |
||
631 | * @param string $text Text with a mixed usage of CR, LF, CRLF. |
||
632 | * @return string The fixed text. |
||
633 | * @see eol |
||
634 | */ |
||
635 | public function fixEOL($text) |
||
652 | |||
653 | /** |
||
654 | * Returns the date according with RFC 2822. |
||
655 | * |
||
656 | * @static |
||
657 | * @param string $date Unix timestamp. |
||
658 | * @return string The RFC 2822 date if successful, false otherwise. |
||
659 | */ |
||
660 | public static function getDate($date) |
||
666 | |||
667 | /** |
||
668 | * Returns the Unix timestamp with preference to the Page Request time. |
||
669 | * |
||
670 | * @static |
||
671 | * @return int Unix timestamp. |
||
672 | */ |
||
673 | public static function getTime() |
||
681 | |||
682 | /** |
||
683 | * Get the instance of the class implementing the MUA for the given type. |
||
684 | * |
||
685 | * @static |
||
686 | * @param string $mua Type of the MUA. |
||
687 | * |
||
688 | * @return mixed The class instance if successful, false otherwise. |
||
689 | */ |
||
690 | public static function getMUA($mua) |
||
703 | |||
704 | /** |
||
705 | * Returns the server name. |
||
706 | * |
||
707 | * @static |
||
708 | * @return string The server name. |
||
709 | */ |
||
710 | public static function getServerName() |
||
721 | |||
722 | /** |
||
723 | * Send the e-mail according with the current settings. |
||
724 | * |
||
725 | * @return bool True if successful, false otherwise. |
||
726 | * |
||
727 | * @todo Enhance error handling using exceptions |
||
728 | */ |
||
729 | public function send() |
||
798 | |||
799 | /** |
||
800 | * Set the "From" address. |
||
801 | * |
||
802 | * @param string $address User e-mail address. |
||
803 | * @param string $name User name (optional). |
||
804 | * @return bool True if successful, false otherwise. |
||
805 | */ |
||
806 | public function setFrom($address, $name = null) |
||
810 | |||
811 | /** |
||
812 | * Set an HTML message providing also a plain text alternative message, |
||
813 | * if not already set using the $messageAlt property. |
||
814 | * Besides it is possible to put resources as inline attachments |
||
815 | * |
||
816 | * @param string $message HTML message. |
||
817 | * @param bool $sanitize Strip out potentially unsecured HTML tags. Defaults to false. |
||
818 | * @param bool $inline Add images as inline attachments. Defaults to false. |
||
819 | * |
||
820 | * @return void |
||
821 | */ |
||
822 | public function setHTMLMessage($message, $sanitize = false, $inline = false) |
||
864 | |||
865 | /** |
||
866 | * Set the "Reply-to" address. |
||
867 | * |
||
868 | * @param string $address User e-mail address. |
||
869 | * @param string $name User name (optional). |
||
870 | * @return bool True if successful, false otherwise. |
||
871 | */ |
||
872 | public function setReplyTo($address, $name = null) |
||
876 | |||
877 | /** |
||
878 | * Set the "Return-Path" address. |
||
879 | * |
||
880 | * @param string $address User e-mail address. |
||
881 | * @return bool True if successful, false otherwise. |
||
882 | */ |
||
883 | public function setReturnPath($address) |
||
887 | |||
888 | /** |
||
889 | * Set the "Sender" address. |
||
890 | * |
||
891 | * @param string $address User e-mail address. |
||
892 | * @param string $name User name (optional). |
||
893 | * @return bool True if successful, false otherwise. |
||
894 | */ |
||
895 | public function setSender($address, $name = null) |
||
899 | |||
900 | /** |
||
901 | * Remove any previous "From" address. |
||
902 | * |
||
903 | * @return bool True if successful, false otherwise. |
||
904 | */ |
||
905 | public function unsetFrom() |
||
911 | |||
912 | /** |
||
913 | * Validate an address as an e-mail address. |
||
914 | * |
||
915 | * @param string $address E-Mail address |
||
916 | * |
||
917 | * @return bool True if the given address is a valid e-mail address, false otherwise. |
||
918 | */ |
||
919 | public static function validateEmail($address) |
||
940 | |||
941 | /** |
||
942 | * Wraps the lines contained into the given message. |
||
943 | * |
||
944 | * @param string $message Message. |
||
945 | * @param integer $width Column width. Defaults to 72. |
||
946 | * @param boolean $cut Cutting a word is allowed. Defaults to false. |
||
947 | * |
||
948 | * @return string The given message, wrapped as requested. |
||
949 | */ |
||
950 | public function wrapLines($message, $width = 72, $cut = false) |
||
968 | |||
969 | /** |
||
970 | * If the email spam protection has been activated from the general |
||
971 | * phpMyFAQ configuration this method converts an email address e.g. |
||
972 | * from "[email protected]" to "user_AT_example_DOT_org". Otherwise |
||
973 | * it will return the plain email address. |
||
974 | * |
||
975 | * @param string $email E-mail address |
||
976 | * @static |
||
977 | * |
||
978 | * @return string |
||
979 | */ |
||
980 | public function safeEmail($email) |
||
988 | } |
||
989 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.