XOOPS /
XoopsCore25
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 XoopsMailer 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 XoopsMailer, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 39 | class XoopsMailer |
||
| 40 | { |
||
| 41 | /** |
||
| 42 | * reference to a {@link XoopsMultiMailer} |
||
| 43 | * |
||
| 44 | * @var XoopsMultiMailer |
||
| 45 | * @access private |
||
| 46 | * @since 21.02.2003 14:14:13 |
||
| 47 | */ |
||
| 48 | public $multimailer; |
||
| 49 | // sender email address |
||
| 50 | // private |
||
| 51 | public $fromEmail; |
||
| 52 | // sender name |
||
| 53 | // private |
||
| 54 | public $fromName; |
||
| 55 | // RMV-NOTIFY |
||
| 56 | // sender UID |
||
| 57 | // private |
||
| 58 | public $fromUser; |
||
| 59 | // array of user class objects |
||
| 60 | // private |
||
| 61 | public $toUsers; |
||
| 62 | // array of email addresses |
||
| 63 | // private |
||
| 64 | public $toEmails; |
||
| 65 | // custom headers |
||
| 66 | // private |
||
| 67 | public $headers; |
||
| 68 | // subject of mail |
||
| 69 | // private |
||
| 70 | public $subject; |
||
| 71 | // body of mail |
||
| 72 | // private |
||
| 73 | public $body; |
||
| 74 | // error messages |
||
| 75 | // private |
||
| 76 | public $errors; |
||
| 77 | // messages upon success |
||
| 78 | // private |
||
| 79 | public $success; |
||
| 80 | // private |
||
| 81 | public $isMail; |
||
| 82 | // private |
||
| 83 | public $isPM; |
||
| 84 | // private |
||
| 85 | public $assignedTags; |
||
| 86 | // private |
||
| 87 | public $template; |
||
| 88 | // private |
||
| 89 | public $templatedir; |
||
| 90 | // protected |
||
| 91 | public $charSet = 'iso-8859-1'; |
||
| 92 | // protected |
||
| 93 | public $encoding = '8bit'; |
||
| 94 | |||
| 95 | /** |
||
| 96 | * Constructor |
||
| 97 | * |
||
| 98 | * @return XoopsMailer |
||
| 99 | */ |
||
| 100 | public function __construct() |
||
| 105 | |||
| 106 | /** |
||
| 107 | * PHP 4 style constructor compatibility shim |
||
| 108 | * |
||
| 109 | * @deprecated all callers should be using parent::__construct() |
||
| 110 | */ |
||
| 111 | public function XoopsMailer() |
||
| 117 | |||
| 118 | /** |
||
| 119 | * @param bool $value |
||
| 120 | */ |
||
| 121 | public function setHTML($value = true) |
||
| 125 | |||
| 126 | // public |
||
| 127 | // reset all properties to default |
||
| 128 | public function reset() |
||
| 149 | |||
| 150 | // public |
||
| 151 | /** |
||
| 152 | * @param null $value |
||
| 153 | */ |
||
| 154 | public function setTemplateDir($value = null) |
||
| 163 | |||
| 164 | // private |
||
| 165 | /** |
||
| 166 | * @return bool|string |
||
| 167 | */ |
||
| 168 | public function getTemplatePath() |
||
| 187 | |||
| 188 | // public |
||
| 189 | /** |
||
| 190 | * @param $value |
||
| 191 | */ |
||
| 192 | public function setTemplate($value) |
||
| 196 | |||
| 197 | // pupblic |
||
| 198 | /** |
||
| 199 | * @param $value |
||
| 200 | */ |
||
| 201 | public function setFromEmail($value) |
||
| 205 | |||
| 206 | // public |
||
| 207 | /** |
||
| 208 | * @param $value |
||
| 209 | */ |
||
| 210 | public function setFromName($value) |
||
| 214 | |||
| 215 | // RMV-NOTIFY |
||
| 216 | // public |
||
| 217 | /** |
||
| 218 | * @param $user |
||
| 219 | */ |
||
| 220 | public function setFromUser($user) |
||
| 226 | |||
| 227 | // public |
||
| 228 | /** |
||
| 229 | * @param $value |
||
| 230 | */ |
||
| 231 | public function setPriority($value) |
||
| 235 | |||
| 236 | // public |
||
| 237 | /** |
||
| 238 | * @param $value |
||
| 239 | */ |
||
| 240 | public function setSubject($value) |
||
| 244 | |||
| 245 | // public |
||
| 246 | /** |
||
| 247 | * @param $value |
||
| 248 | */ |
||
| 249 | public function setBody($value) |
||
| 253 | |||
| 254 | // public |
||
| 255 | public function useMail() |
||
| 259 | |||
| 260 | // public |
||
| 261 | public function usePM() |
||
| 265 | |||
| 266 | // public |
||
| 267 | /** |
||
| 268 | * @param bool $debug |
||
| 269 | * |
||
| 270 | * @return bool |
||
| 271 | */ |
||
| 272 | public function send($debug = false) |
||
| 273 | { |
||
| 274 | global $xoopsConfig; |
||
| 275 | if ($this->body == '' && $this->template == '') { |
||
| 276 | if ($debug) { |
||
| 277 | $this->errors[] = _MAIL_MSGBODY; |
||
| 278 | } |
||
| 279 | |||
| 280 | return false; |
||
| 281 | } elseif ($this->template != '') { |
||
| 282 | $path = $this->getTemplatePath(); |
||
| 283 | if (!($fd = @fopen($path, 'r'))) { |
||
| 284 | if ($debug) { |
||
| 285 | $this->errors[] = _MAIL_FAILOPTPL; |
||
| 286 | } |
||
| 287 | |||
| 288 | return false; |
||
| 289 | } |
||
| 290 | $this->setBody(fread($fd, filesize($path))); |
||
| 291 | } |
||
| 292 | // for sending mail only |
||
| 293 | if ($this->isMail || !empty($this->toEmails)) { |
||
| 294 | if (!empty($this->priority)) { |
||
| 295 | $this->headers[] = 'X-Priority: ' . $this->priority; |
||
| 296 | } |
||
| 297 | // $this->headers[] = "X-Mailer: PHP/".phpversion(); |
||
| 298 | // $this->headers[] = "Return-Path: ".$this->fromEmail; |
||
| 299 | $headers = implode($this->LE, $this->headers); |
||
| 300 | } |
||
| 301 | // TODO: we should have an option of no-reply for private messages and emails |
||
| 302 | // to which we do not accept replies. e.g. the site admin doesn't want a |
||
| 303 | // a lot of message from people trying to unsubscribe. Just make sure to |
||
| 304 | // give good instructions in the message. |
||
| 305 | // add some standard tags (user-dependent tags are included later) |
||
| 306 | global $xoopsConfig; |
||
| 307 | |||
| 308 | $this->assign('X_ADMINMAIL', $xoopsConfig['adminmail']); |
||
| 309 | $this->assign('X_SITENAME', $xoopsConfig['sitename']); |
||
| 310 | $this->assign('X_SITEURL', XOOPS_URL . '/'); |
||
| 311 | // TODO: also X_ADMINNAME?? |
||
| 312 | // TODO: X_SIGNATURE, X_DISCLAIMER ?? - these are probably best |
||
| 313 | // done as includes if mail templates ever get this sophisticated |
||
| 314 | // replace tags with actual values |
||
| 315 | foreach ($this->assignedTags as $k => $v) { |
||
| 316 | $this->body = str_replace('{' . $k . '}', $v, $this->body); |
||
| 317 | $this->subject = str_replace('{' . $k . '}', $v, $this->subject); |
||
| 318 | } |
||
| 319 | $this->body = str_replace("\r\n", "\n", $this->body); |
||
| 320 | $this->body = str_replace("\r", "\n", $this->body); |
||
| 321 | $this->body = str_replace("\n", $this->LE, $this->body); |
||
| 322 | // send mail to specified mail addresses, if any |
||
| 323 | foreach ($this->toEmails as $mailaddr) { |
||
| 324 | if (!$this->sendMail($mailaddr, $this->subject, $this->body, $headers)) { |
||
| 325 | if ($debug) { |
||
| 326 | $this->errors[] = sprintf(_MAIL_SENDMAILNG, $mailaddr); |
||
| 327 | } |
||
| 328 | } else { |
||
| 329 | if ($debug) { |
||
| 330 | $this->success[] = sprintf(_MAIL_MAILGOOD, $mailaddr); |
||
| 331 | } |
||
| 332 | } |
||
| 333 | } |
||
| 334 | // send message to specified users, if any |
||
| 335 | // NOTE: we don't send to LIST of recipients, because the tags |
||
| 336 | // below are dependent on the user identity; i.e. each user |
||
| 337 | // receives (potentially) a different message |
||
| 338 | foreach ($this->toUsers as $user) { |
||
| 339 | // set some user specific variables |
||
| 340 | $subject = str_replace('{X_UNAME}', $user->getVar('uname'), $this->subject); |
||
| 341 | $text = str_replace('{X_UID}', $user->getVar('uid'), $this->body); |
||
| 342 | $text = str_replace('{X_UEMAIL}', $user->getVar('email'), $text); |
||
| 343 | $text = str_replace('{X_UNAME}', $user->getVar('uname'), $text); |
||
| 344 | $text = str_replace('{X_UACTLINK}', XOOPS_URL . '/register.php?op=actv&id=' . $user->getVar('uid') . '&actkey=' . $user->getVar('actkey'), $text); |
||
| 345 | // send mail |
||
| 346 | View Code Duplication | if ($this->isMail) { |
|
| 347 | if (!$this->sendMail($user->getVar('email'), $subject, $text, $headers)) { |
||
| 348 | if ($debug) { |
||
| 349 | $this->errors[] = sprintf(_MAIL_SENDMAILNG, $user->getVar('uname')); |
||
| 350 | } |
||
| 351 | } else { |
||
| 352 | if ($debug) { |
||
| 353 | $this->success[] = sprintf(_MAIL_MAILGOOD, $user->getVar('uname')); |
||
| 354 | } |
||
| 355 | } |
||
| 356 | } |
||
| 357 | // send private message |
||
| 358 | View Code Duplication | if ($this->isPM) { |
|
| 359 | if (!$this->sendPM($user->getVar('uid'), $subject, $text)) { |
||
| 360 | if ($debug) { |
||
| 361 | $this->errors[] = sprintf(_MAIL_SENDPMNG, $user->getVar('uname')); |
||
| 362 | } |
||
| 363 | } else { |
||
| 364 | if ($debug) { |
||
| 365 | $this->success[] = sprintf(_MAIL_PMGOOD, $user->getVar('uname')); |
||
| 366 | } |
||
| 367 | } |
||
| 368 | } |
||
| 369 | flush(); |
||
| 370 | } |
||
| 371 | return !(count($this->errors) > 0); |
||
| 372 | } |
||
| 373 | |||
| 374 | // private |
||
| 375 | /** |
||
| 376 | * @param $uid |
||
| 377 | * @param $subject |
||
| 378 | * @param $body |
||
| 379 | * |
||
| 380 | * @return bool |
||
| 381 | */ |
||
| 382 | public function sendPM($uid, $subject, $body) |
||
| 398 | |||
| 399 | /** |
||
| 400 | * Send email |
||
| 401 | * |
||
| 402 | * Uses the new XoopsMultiMailer |
||
| 403 | * |
||
| 404 | * @param $email |
||
| 405 | * @param $subject |
||
| 406 | * @param $body |
||
| 407 | * @param $headers |
||
| 408 | * |
||
| 409 | * @return bool |
||
| 410 | */ |
||
| 411 | public function sendMail($email, $subject, $body, $headers) |
||
| 440 | |||
| 441 | // public |
||
| 442 | /** |
||
| 443 | * @param bool $ashtml |
||
| 444 | * |
||
| 445 | * @return string |
||
| 446 | */ |
||
| 447 | public function getErrors($ashtml = true) |
||
| 464 | |||
| 465 | // public |
||
| 466 | /** |
||
| 467 | * @param bool $ashtml |
||
| 468 | * |
||
| 469 | * @return string |
||
| 470 | */ |
||
| 471 | public function getSuccess($ashtml = true) |
||
| 486 | |||
| 487 | // public |
||
| 488 | /** |
||
| 489 | * @param $tag |
||
| 490 | * @param null $value |
||
| 491 | */ |
||
| 492 | public function assign($tag, $value = null) |
||
| 509 | |||
| 510 | // public |
||
| 511 | /** |
||
| 512 | * @param $value |
||
| 513 | */ |
||
| 514 | public function addHeaders($value) |
||
| 518 | |||
| 519 | // public |
||
| 520 | /** |
||
| 521 | * @param $email |
||
| 522 | */ |
||
| 523 | public function setToEmails($email) |
||
| 535 | |||
| 536 | // public |
||
| 537 | /** |
||
| 538 | * @param $user |
||
| 539 | */ |
||
| 540 | public function setToUsers(&$user) |
||
| 552 | |||
| 553 | // public |
||
| 554 | /** |
||
| 555 | * @param $group |
||
| 556 | */ |
||
| 557 | public function setToGroups($group) |
||
| 570 | |||
| 571 | // abstract |
||
| 572 | // to be overridden by lang specific mail class, if needed |
||
| 573 | /** |
||
| 574 | * @param $text |
||
| 575 | * |
||
| 576 | * @return mixed |
||
| 577 | */ |
||
| 578 | public function encodeFromName($text) |
||
| 582 | |||
| 583 | // abstract |
||
| 584 | // to be overridden by lang specific mail class, if needed |
||
| 585 | /** |
||
| 586 | * @param $text |
||
| 587 | * |
||
| 588 | * @return mixed |
||
| 589 | */ |
||
| 590 | public function encodeSubject($text) |
||
| 594 | |||
| 595 | // abstract |
||
| 596 | // to be overridden by lang specific mail class, if needed |
||
| 597 | /** |
||
| 598 | * @param $text |
||
| 599 | */ |
||
| 600 | public function encodeBody(&$text) |
||
| 603 | } |
||
| 604 |
Loading history...
The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.
The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.
To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.