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 ObjectController 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 ObjectController, and based on these observations, apply Extract Interface, too.
| 1 | <?php namespace XoopsModules\Smartobject; |
||
| 34 | class ObjectController |
||
| 35 | { |
||
| 36 | public $handler; |
||
| 37 | |||
| 38 | /** |
||
| 39 | * SmartObjectController constructor. |
||
| 40 | * @param $handler |
||
| 41 | */ |
||
| 42 | public function __construct($handler) |
||
| 46 | |||
| 47 | /** |
||
| 48 | * @param $smartObj |
||
| 49 | */ |
||
| 50 | public function postDataToObject(&$smartObj) |
||
| 126 | |||
| 127 | /** |
||
| 128 | * @param $smartObj |
||
| 129 | * @param $objectid |
||
| 130 | * @param $created_success_msg |
||
| 131 | * @param $modified_success_msg |
||
| 132 | * @param bool $redirect_page |
||
| 133 | * @param bool $debug |
||
| 134 | * @return mixed |
||
| 135 | */ |
||
| 136 | public function doStoreFromDefaultForm( |
||
| 217 | |||
| 218 | /** |
||
| 219 | * Store the object in the database autmatically from a form sending POST data |
||
| 220 | * |
||
| 221 | * @param string $created_success_msg message to display if new object was created |
||
| 222 | * @param string $modified_success_msg message to display if object was successfully edited |
||
| 223 | * @param bool|string $redirect_page redirect page, if not set, then we backup once |
||
| 224 | * @param bool $debug |
||
| 225 | * @param bool $x_param |
||
| 226 | * @return bool |
||
| 227 | * @internal param string $created_redir_page redirect page after creating the object |
||
| 228 | * @internal param string $modified_redir_page redirect page after editing the object |
||
| 229 | * @internal param bool $exit if set to TRUE then the script ends |
||
| 230 | */ |
||
| 231 | public function storeFromDefaultForm( |
||
| 280 | |||
| 281 | /** |
||
| 282 | * @return bool |
||
| 283 | */ |
||
| 284 | public function storeSmartObjectD() |
||
| 288 | |||
| 289 | /** |
||
| 290 | * @param bool $debug |
||
| 291 | * @param bool $xparam |
||
| 292 | * @return bool |
||
| 293 | */ |
||
| 294 | public function storeSmartObject($debug = false, $xparam = false) |
||
| 300 | |||
| 301 | /** |
||
| 302 | * Handles deletion of an object which keyid is passed as a GET param |
||
| 303 | * |
||
| 304 | * @param bool $confirm_msg |
||
| 305 | * @param string $op |
||
| 306 | * @param bool $userSide |
||
| 307 | * @return void |
||
| 308 | * @internal param string $redir_page redirect page after deleting the object |
||
| 309 | */ |
||
| 310 | public function handleObjectDeletion($confirm_msg = false, $op = 'del', $userSide = false) |
||
| 311 | { |
||
| 312 | global $smart_previous_page; |
||
| 313 | |||
| 314 | $objectid = isset($_REQUEST[$this->handler->keyName]) ? (int)$_REQUEST[$this->handler->keyName] : 0; |
||
| 315 | $smartObj = $this->handler->get($objectid); |
||
| 316 | |||
| 317 | if ($smartObj->isNew()) { |
||
| 318 | redirect_header('javascript:history.go(-1)', 3, _CO_SOBJECT_NOT_SELECTED); |
||
| 319 | } |
||
| 320 | |||
| 321 | $confirm = \Xmf\Request::getInt('confirm', 0, POST); |
||
| 322 | if ($confirm) { |
||
| 323 | View Code Duplication | if (!$this->handler->delete($smartObj)) { |
|
| 324 | redirect_header($_POST['redirect_page'], 3, _CO_SOBJECT_DELETE_ERROR . $smartObj->getHtmlErrors()); |
||
| 325 | } |
||
| 326 | |||
| 327 | redirect_header($_POST['redirect_page'], 3, _CO_SOBJECT_DELETE_SUCCESS); |
||
| 328 | } else { |
||
| 329 | // no confirm: show deletion condition |
||
| 330 | |||
| 331 | xoops_cp_header(); |
||
| 332 | |||
| 333 | if (!$confirm_msg) { |
||
| 334 | $confirm_msg = _CO_SOBJECT_DELETE_CONFIRM; |
||
| 335 | } |
||
| 336 | |||
| 337 | xoops_confirm([ |
||
| 338 | 'op' => $op, |
||
| 339 | $this->handler->keyName => $smartObj->getVar($this->handler->keyName), |
||
| 340 | 'confirm' => 1, |
||
| 341 | 'redirect_page' => $smart_previous_page |
||
| 342 | ], xoops_getenv('PHP_SELF'), sprintf($confirm_msg, $smartObj->getVar($this->handler->identifierName)), _CO_SOBJECT_DELETE); |
||
| 343 | |||
| 344 | xoops_cp_footer(); |
||
| 345 | } |
||
| 346 | exit(); |
||
| 347 | } |
||
| 348 | |||
| 349 | /** |
||
| 350 | * @param bool $confirm_msg |
||
| 351 | * @param string $op |
||
| 352 | */ |
||
| 353 | public function handleObjectDeletionFromUserSide($confirm_msg = false, $op = 'del') |
||
| 354 | { |
||
| 355 | global $smart_previous_page, $xoopsTpl; |
||
| 356 | |||
| 357 | $objectid = isset($_REQUEST[$this->handler->keyName]) ? (int)$_REQUEST[$this->handler->keyName] : 0; |
||
| 358 | $smartObj = $this->handler->get($objectid); |
||
| 359 | |||
| 360 | if ($smartObj->isNew()) { |
||
| 361 | redirect_header('javascript:history.go(-1)', 3, _CO_SOBJECT_NOT_SELECTED); |
||
| 362 | } |
||
| 363 | |||
| 364 | $confirm = \Xmf\Request::getInt('confirm', 0, POST); |
||
| 365 | if ($confirm) { |
||
| 366 | View Code Duplication | if (!$this->handler->delete($smartObj)) { |
|
| 367 | redirect_header($_POST['redirect_page'], 3, _CO_SOBJECT_DELETE_ERROR . $smartObj->getHtmlErrors()); |
||
| 368 | } |
||
| 369 | |||
| 370 | redirect_header($_POST['redirect_page'], 3, _CO_SOBJECT_DELETE_SUCCESS); |
||
| 371 | } else { |
||
| 372 | // no confirm: show deletion condition |
||
| 373 | if (!$confirm_msg) { |
||
| 374 | $confirm_msg = _CO_SOBJECT_DELETE_CONFIRM; |
||
| 375 | } |
||
| 376 | |||
| 377 | ob_start(); |
||
| 378 | xoops_confirm([ |
||
| 379 | 'op' => $op, |
||
| 380 | $this->handler->keyName => $smartObj->getVar($this->handler->keyName), |
||
| 381 | 'confirm' => 1, |
||
| 382 | 'redirect_page' => $smart_previous_page |
||
| 383 | ], xoops_getenv('PHP_SELF'), sprintf($confirm_msg, $smartObj->getVar($this->handler->identifierName)), _CO_SOBJECT_DELETE); |
||
| 384 | $smartobjectDeleteConfirm = ob_get_clean(); |
||
| 385 | $xoopsTpl->assign('smartobject_delete_confirm', $smartobjectDeleteConfirm); |
||
| 386 | } |
||
| 387 | } |
||
| 388 | |||
| 389 | /** |
||
| 390 | * Retreive the object admin side link for a {@link SmartObjectSingleView} page |
||
| 391 | * |
||
| 392 | * @param SmartObject $smartObj reference to the object from which we want the user side link |
||
| 393 | * @param bool $onlyUrl wether or not to return a simple URL or a full <a> link |
||
| 394 | * @param bool $withimage |
||
| 395 | * @return string admin side link to the object |
||
| 396 | */ |
||
| 397 | public function getAdminViewItemLink(SmartObject $smartObj, $onlyUrl = false, $withimage = false) |
||
| 408 | |||
| 409 | /** |
||
| 410 | * Retreive the object user side link |
||
| 411 | * |
||
| 412 | * @param SmartObject $smartObj reference to the object from which we want the user side link |
||
| 413 | * @param bool $onlyUrl wether or not to return a simple URL or a full <a> link |
||
| 414 | * @return string user side link to the object |
||
| 415 | */ |
||
| 416 | public function getItemLink(SmartObject $smartObj, $onlyUrl = false) |
||
| 441 | |||
| 442 | /** |
||
| 443 | * @param $smartObj |
||
| 444 | * @param bool $onlyUrl |
||
| 445 | * @param bool $withimage |
||
| 446 | * @return string |
||
| 447 | */ |
||
| 448 | public function getEditLanguageLink($smartObj, $onlyUrl = false, $withimage = true) |
||
| 459 | |||
| 460 | /** |
||
| 461 | * @param $smartObj |
||
| 462 | * @param bool $onlyUrl |
||
| 463 | * @param bool $withimage |
||
| 464 | * @param bool $userSide |
||
| 465 | * @return string |
||
| 466 | */ |
||
| 467 | View Code Duplication | public function getEditItemLink($smartObj, $onlyUrl = false, $withimage = true, $userSide = false) |
|
| 479 | |||
| 480 | /** |
||
| 481 | * @param $smartObj |
||
| 482 | * @param bool $onlyUrl |
||
| 483 | * @param bool $withimage |
||
| 484 | * @param bool $userSide |
||
| 485 | * @return string |
||
| 486 | */ |
||
| 487 | View Code Duplication | public function getDeleteItemLink($smartObj, $onlyUrl = false, $withimage = true, $userSide = false) |
|
| 499 | |||
| 500 | /** |
||
| 501 | * @param $smartObj |
||
| 502 | * @return string |
||
| 503 | */ |
||
| 504 | public function getPrintAndMailLink($smartObj) |
||
| 533 | |||
| 534 | /** |
||
| 535 | * @return string |
||
| 536 | */ |
||
| 537 | public function getModuleItemString() |
||
| 543 | } |
||
| 544 |
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.