mambax7 /
smartobject
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 SmartObjectController 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 SmartObjectController, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 32 | class SmartObjectController |
||
| 33 | { |
||
| 34 | public $handler; |
||
| 35 | |||
| 36 | /** |
||
| 37 | * SmartObjectController constructor. |
||
| 38 | * @param $handler |
||
| 39 | */ |
||
| 40 | public function __construct($handler) |
||
| 44 | |||
| 45 | /** |
||
| 46 | * @param $smartObj |
||
| 47 | */ |
||
| 48 | public function postDataToObject(&$smartObj) |
||
| 49 | { |
||
| 50 | foreach (array_keys($smartObj->vars) as $key) { |
||
| 51 | switch ($smartObj->vars[$key]['data_type']) { |
||
| 52 | case XOBJ_DTYPE_IMAGE: |
||
| 53 | View Code Duplication | if (isset($_POST['url_' . $key]) && $_POST['url_' . $key] !== '') { |
|
| 54 | $oldFile = $smartObj->getUploadDir(true) . $smartObj->getVar($key, 'e'); |
||
| 55 | $smartObj->setVar($key, $_POST['url_' . $key]); |
||
| 56 | if (file_exists($oldFile)) { |
||
| 57 | unlink($oldFile); |
||
| 58 | } |
||
| 59 | } |
||
| 60 | View Code Duplication | if (isset($_POST['delete_' . $key]) && $_POST['delete_' . $key] == '1') { |
|
| 61 | $oldFile = $smartObj->getUploadDir(true) . $smartObj->getVar($key, 'e'); |
||
| 62 | $smartObj->setVar($key, ''); |
||
| 63 | if (file_exists($oldFile)) { |
||
| 64 | unlink($oldFile); |
||
| 65 | } |
||
| 66 | } |
||
| 67 | break; |
||
| 68 | |||
| 69 | case XOBJ_DTYPE_URLLINK: |
||
| 70 | $linkObj = $smartObj->getUrlLinkObj($key); |
||
| 71 | $linkObj->setVar('caption', $_POST['caption_' . $key]); |
||
| 72 | $linkObj->setVar('description', $_POST['desc_' . $key]); |
||
| 73 | $linkObj->setVar('target', $_POST['target_' . $key]); |
||
| 74 | $linkObj->setVar('url', $_POST['url_' . $key]); |
||
| 75 | if ($linkObj->getVar('url') !== '') { |
||
| 76 | $smartObj->storeUrlLinkObj($linkObj); |
||
| 77 | } |
||
| 78 | //todo: catch errors |
||
| 79 | $smartObj->setVar($key, $linkObj->getVar('urllinkid')); |
||
| 80 | break; |
||
| 81 | |||
| 82 | case XOBJ_DTYPE_FILE: |
||
| 83 | if (!isset($_FILES['upload_' . $key]['name']) || $_FILES['upload_' . $key]['name'] === '') { |
||
| 84 | $fileObj = $smartObj->getFileObj($key); |
||
| 85 | $fileObj->setVar('caption', $_POST['caption_' . $key]); |
||
| 86 | $fileObj->setVar('description', $_POST['desc_' . $key]); |
||
| 87 | $fileObj->setVar('url', $_POST['url_' . $key]); |
||
| 88 | if (!($fileObj->getVar('url') === '' && $fileObj->getVar('url') === '' && $fileObj->getVar('url') === '')) { |
||
| 89 | $res = $smartObj->storeFileObj($fileObj); |
||
| 90 | if ($res) { |
||
| 91 | $smartObj->setVar($key, $fileObj->getVar('fileid')); |
||
| 92 | } else { |
||
| 93 | //error setted, but no error message (to be improved) |
||
| 94 | $smartObj->setErrors($fileObj->getErrors()); |
||
| 95 | } |
||
| 96 | } |
||
| 97 | } |
||
| 98 | break; |
||
| 99 | |||
| 100 | case XOBJ_DTYPE_STIME: |
||
| 101 | case XOBJ_DTYPE_MTIME: |
||
| 102 | case XOBJ_DTYPE_LTIME: |
||
| 103 | // check if this field's value is available in the POST array |
||
| 104 | if (is_array($_POST[$key]) && isset($_POST[$key]['date'])) { |
||
| 105 | $value = strtotime($_POST[$key]['date']) + $_POST[$key]['time']; |
||
| 106 | } else { |
||
| 107 | $value = strtotime($_POST[$key]); |
||
| 108 | //if strtotime returns false, the value is already a time stamp |
||
| 109 | if (!$value) { |
||
| 110 | $value = (int)$_POST[$key]; |
||
| 111 | } |
||
| 112 | } |
||
| 113 | $smartObj->setVar($key, $value); |
||
| 114 | |||
| 115 | break; |
||
| 116 | |||
| 117 | default: |
||
| 118 | $smartObj->setVar($key, $_POST[$key]); |
||
| 119 | break; |
||
| 120 | } |
||
| 121 | } |
||
| 122 | } |
||
| 123 | |||
| 124 | /** |
||
| 125 | * @param $smartObj |
||
| 126 | * @param $objectid |
||
| 127 | * @param $created_success_msg |
||
| 128 | * @param $modified_success_msg |
||
| 129 | * @param bool $redirect_page |
||
| 130 | * @param bool $debug |
||
| 131 | * @return mixed |
||
| 132 | */ |
||
| 133 | public function doStoreFromDefaultForm(&$smartObj, $objectid, $created_success_msg, $modified_success_msg, $redirect_page = false, $debug = false) |
||
| 207 | |||
| 208 | /** |
||
| 209 | * Store the object in the database autmatically from a form sending POST data |
||
| 210 | * |
||
| 211 | * @param string $created_success_msg message to display if new object was created |
||
| 212 | * @param string $modified_success_msg message to display if object was successfully edited |
||
| 213 | * @param bool|string $redirect_page redirect page, if not set, then we backup once |
||
| 214 | * @param bool $debug |
||
| 215 | * @param bool $x_param |
||
| 216 | * @return bool |
||
| 217 | * @internal param string $created_redir_page redirect page after creating the object |
||
| 218 | * @internal param string $modified_redir_page redirect page after editing the object |
||
| 219 | * @internal param bool $exit if set to TRUE then the script ends |
||
| 220 | */ |
||
| 221 | public function storeFromDefaultForm($created_success_msg, $modified_success_msg, $redirect_page = false, $debug = false, $x_param = false) |
||
| 265 | |||
| 266 | /** |
||
| 267 | * @return bool |
||
| 268 | */ |
||
| 269 | public function storeSmartObjectD() |
||
| 273 | |||
| 274 | /** |
||
| 275 | * @param bool $debug |
||
| 276 | * @param bool $xparam |
||
| 277 | * @return bool |
||
| 278 | */ |
||
| 279 | public function storeSmartObject($debug = false, $xparam = false) |
||
| 285 | |||
| 286 | /** |
||
| 287 | * Handles deletion of an object which keyid is passed as a GET param |
||
| 288 | * |
||
| 289 | * @param bool $confirm_msg |
||
| 290 | * @param string $op |
||
| 291 | * @param bool $userSide |
||
| 292 | * @return bool |
||
| 293 | * @internal param string $redir_page redirect page after deleting the object |
||
| 294 | */ |
||
| 295 | public function handleObjectDeletion($confirm_msg = false, $op = 'del', $userSide = false) |
||
| 330 | |||
| 331 | /** |
||
| 332 | * @param bool $confirm_msg |
||
| 333 | * @param string $op |
||
| 334 | */ |
||
| 335 | public function handleObjectDeletionFromUserSide($confirm_msg = false, $op = 'del') |
||
| 367 | |||
| 368 | /** |
||
| 369 | * Retreive the object admin side link for a {@link SmartObjectSingleView} page |
||
| 370 | * |
||
| 371 | * @param SmartObject $smartObj reference to the object from which we want the user side link |
||
| 372 | * @param bool $onlyUrl wether or not to return a simple URL or a full <a> link |
||
| 373 | * @param bool $withimage |
||
| 374 | * @return string admin side link to the object |
||
| 375 | */ |
||
| 376 | public function getAdminViewItemLink(SmartObject $smartObj, $onlyUrl = false, $withimage = false) |
||
| 395 | |||
| 396 | /** |
||
| 397 | * Retreive the object user side link |
||
| 398 | * |
||
| 399 | * @param SmartObject $smartObj reference to the object from which we want the user side link |
||
| 400 | * @param bool $onlyUrl wether or not to return a simple URL or a full <a> link |
||
| 401 | * @return string user side link to the object |
||
| 402 | */ |
||
| 403 | public function getItemLink(SmartObject $smartObj, $onlyUrl = false) |
||
| 444 | |||
| 445 | /** |
||
| 446 | * @param $smartObj |
||
| 447 | * @param bool $onlyUrl |
||
| 448 | * @param bool $withimage |
||
| 449 | * @return string |
||
| 450 | */ |
||
| 451 | public function getEditLanguageLink($smartObj, $onlyUrl = false, $withimage = true) |
||
| 478 | |||
| 479 | /** |
||
| 480 | * @param $smartObj |
||
| 481 | * @param bool $onlyUrl |
||
| 482 | * @param bool $withimage |
||
| 483 | * @param bool $userSide |
||
| 484 | * @return string |
||
| 485 | */ |
||
| 486 | View Code Duplication | public function getEditItemLink($smartObj, $onlyUrl = false, $withimage = true, $userSide = false) |
|
| 506 | |||
| 507 | /** |
||
| 508 | * @param $smartObj |
||
| 509 | * @param bool $onlyUrl |
||
| 510 | * @param bool $withimage |
||
| 511 | * @param bool $userSide |
||
| 512 | * @return string |
||
| 513 | */ |
||
| 514 | View Code Duplication | public function getDeleteItemLink($smartObj, $onlyUrl = false, $withimage = true, $userSide = false) |
|
| 534 | |||
| 535 | /** |
||
| 536 | * @param $smartObj |
||
| 537 | * @return string |
||
| 538 | */ |
||
| 539 | public function getPrintAndMailLink($smartObj) |
||
| 568 | |||
| 569 | /** |
||
| 570 | * @return string |
||
| 571 | */ |
||
| 572 | public function getModuleItemString() |
||
| 578 | } |
||
| 579 |
Loading history...
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.