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 Administration 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 Administration, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 13 | class Administration extends Symphony |
||
| 14 | { |
||
| 15 | /** |
||
| 16 | * The class representation of the current Symphony backend page, |
||
| 17 | * which is a subclass of the `HTMLPage` class. Symphony uses a convention |
||
| 18 | * of prefixing backend page classes with 'content'. ie. 'contentBlueprintsSections' |
||
| 19 | * |
||
| 20 | * @var HTMLPage |
||
| 21 | */ |
||
| 22 | public $Page; |
||
| 23 | /** |
||
| 24 | * The path of the current page, ie. '/blueprints/sections/' |
||
| 25 | * |
||
| 26 | * @var string |
||
| 27 | */ |
||
| 28 | private $_currentPage = null; |
||
| 29 | /** |
||
| 30 | * An associative array of the page's callback, including the keys |
||
| 31 | * 'driver', which is a lowercase version of `$this->_currentPage` |
||
| 32 | * with any slashes removed, 'classname', which is the name of the class |
||
| 33 | * for this page, 'pageroot', which is the root page for the given page, (ie. |
||
| 34 | * excluding /saved/, /created/ or any sub pages of the current page that are |
||
| 35 | * handled using the _switchboard function. |
||
| 36 | * |
||
| 37 | * @see toolkit.AdministrationPage#__switchboard() |
||
| 38 | * @var array |
||
| 39 | */ |
||
| 40 | private $_callback = null; |
||
| 41 | |||
| 42 | /** |
||
| 43 | * Overrides the default Symphony constructor to add XSRF checking |
||
| 44 | */ |
||
| 45 | protected function __construct() |
||
| 54 | |||
| 55 | /** |
||
| 56 | * This function returns an instance of the Administration |
||
| 57 | * class. It is the only way to create a new Administration, as |
||
| 58 | * it implements the Singleton interface |
||
| 59 | * |
||
| 60 | * @return Administration |
||
| 61 | */ |
||
| 62 | public static function instance() |
||
| 70 | |||
| 71 | /** |
||
| 72 | * Returns the current Page path, excluding the domain and Symphony path. |
||
| 73 | * |
||
| 74 | * @return string |
||
| 75 | * The path of the current page, ie. '/blueprints/sections/' |
||
| 76 | */ |
||
| 77 | public function getCurrentPageURL() |
||
| 81 | |||
| 82 | /** |
||
| 83 | * Called by index.php, this function is responsible for rendering the current |
||
| 84 | * page on the Frontend. Two delegates are fired, AdminPagePreGenerate and |
||
| 85 | * AdminPagePostGenerate. This function runs the Profiler for the page build |
||
| 86 | * process. |
||
| 87 | * |
||
| 88 | * @uses AdminPagePreBuild |
||
| 89 | * @uses AdminPagePreGenerate |
||
| 90 | * @uses AdminPagePostGenerate |
||
| 91 | * @see core.Symphony#__buildPage() |
||
| 92 | * @see boot.getCurrentPage() |
||
| 93 | * @param string $page |
||
| 94 | * The result of getCurrentPage, which returns the $_GET['symphony-page'] |
||
| 95 | * variable. |
||
| 96 | * @throws Exception |
||
| 97 | * @throws SymphonyErrorPage |
||
| 98 | * @return string |
||
| 99 | * The HTML of the page to return |
||
| 100 | */ |
||
| 101 | public function display($page) |
||
| 157 | |||
| 158 | /** |
||
| 159 | * Given the URL path of a Symphony backend page, this function will |
||
| 160 | * attempt to resolve the URL to a Symphony content page in the backend |
||
| 161 | * or a page provided by an extension. This function checks to ensure a user |
||
| 162 | * is logged in, otherwise it will direct them to the login page |
||
| 163 | * |
||
| 164 | * @param string $page |
||
| 165 | * The URL path after the root of the Symphony installation, including a starting |
||
| 166 | * slash, such as '/login/' |
||
| 167 | * @throws SymphonyErrorPage |
||
| 168 | * @throws Exception |
||
| 169 | * @return HTMLPage |
||
| 170 | */ |
||
| 171 | private function __buildPage($page) |
||
| 275 | |||
| 276 | /** |
||
| 277 | * Overrides the Symphony isLoggedIn function to allow Authors |
||
| 278 | * to become logged into the backend when `$_REQUEST['auth-token']` |
||
| 279 | * is present. This logs an Author in using the loginFromToken function. |
||
| 280 | * A token may be 6 or 8 characters in length in the backend. A 6 or 16 character token |
||
| 281 | * is used for forget password requests, whereas the 8 character token is used to login |
||
| 282 | * an Author into the page |
||
| 283 | * |
||
| 284 | * @see core.Symphony#loginFromToken() |
||
| 285 | * @return boolean |
||
| 286 | */ |
||
| 287 | public static function isLoggedIn() |
||
| 297 | |||
| 298 | /** |
||
| 299 | * This function resolves the string of the page to the relevant |
||
| 300 | * backend page class. The path to the backend page is split on |
||
| 301 | * the slashes and the resulting pieces used to determine if the page |
||
| 302 | * is provided by an extension, is a section (index or entry creation) |
||
| 303 | * or finally a standard Symphony content page. If no page driver can |
||
| 304 | * be found, this function will return false. |
||
| 305 | * |
||
| 306 | * @uses AdminPagePostCallback |
||
| 307 | * @param string $page |
||
| 308 | * The full path (including the domain) of the Symphony backend page |
||
| 309 | * @return array|boolean |
||
| 310 | * If successful, this function will return an associative array that at the |
||
| 311 | * very least will return the page's classname, pageroot, driver, driver_location |
||
| 312 | * and context, otherwise this will return false. |
||
| 313 | */ |
||
| 314 | public function getPageCallback($page = null) |
||
| 315 | { |
||
| 316 | if (!$page && !is_null($this->_callback)) { |
||
| 317 | return $this->_callback; |
||
| 318 | } elseif (!$page && is_null($this->_callback)) { |
||
| 319 | trigger_error(__('Cannot request a page callback without first specifying the page.')); |
||
| 320 | } |
||
| 321 | |||
| 322 | $this->_currentPage = SYMPHONY_URL . preg_replace('/\/{2,}/', '/', $page); |
||
| 323 | $bits = preg_split('/\//', trim($page, '/'), 3, PREG_SPLIT_NO_EMPTY); |
||
| 324 | $callback = array( |
||
| 325 | 'driver' => null, |
||
| 326 | 'driver_location' => null, |
||
| 327 | 'context' => array(), |
||
| 328 | 'classname' => null, |
||
| 329 | 'pageroot' => null |
||
| 330 | ); |
||
| 331 | |||
| 332 | // Login page, /symphony/login/ |
||
| 333 | if ($bits[0] === 'login') { |
||
| 334 | $callback['driver'] = 'login'; |
||
| 335 | $callback['driver_location'] = CONTENT . '/content.login.php'; |
||
| 336 | $callback['classname'] = 'contentLogin'; |
||
| 337 | $callback['pageroot'] = '/login/'; |
||
| 338 | |||
| 339 | // Extension page, /symphony/extension/{extension_name}/ |
||
| 340 | } elseif ($bits[0] === 'extension' && isset($bits[1])) { |
||
| 341 | $extension_name = $bits[1]; |
||
| 342 | $bits = preg_split('/\//', trim($bits[2], '/'), 2, PREG_SPLIT_NO_EMPTY); |
||
| 343 | |||
| 344 | // check if extension is enabled, if it's not, pretend the extension doesn't |
||
| 345 | // even exist. #2367 |
||
| 346 | if (!ExtensionManager::isInstalled($extension_name)) { |
||
| 347 | return false; |
||
| 348 | } |
||
| 349 | |||
| 350 | $callback['driver'] = 'index'; |
||
| 351 | $callback['classname'] = 'contentExtension' . ucfirst($extension_name) . 'Index'; |
||
| 352 | $callback['pageroot'] = '/extension/' . $extension_name . '/'; |
||
| 353 | $callback['extension'] = $extension_name; |
||
| 354 | |||
| 355 | if (isset($bits[0])) { |
||
| 356 | $callback['driver'] = $bits[0]; |
||
| 357 | $callback['classname'] = 'contentExtension' . ucfirst($extension_name) . ucfirst($bits[0]); |
||
| 358 | $callback['pageroot'] .= $bits[0] . '/'; |
||
| 359 | } |
||
| 360 | |||
| 361 | View Code Duplication | if (isset($bits[1])) { |
|
| 362 | $callback['context'] = preg_split('/\//', $bits[1], -1, PREG_SPLIT_NO_EMPTY); |
||
| 363 | } |
||
| 364 | |||
| 365 | $callback['driver_location'] = EXTENSIONS . '/' . $extension_name . '/content/content.' . $callback['driver'] . '.php'; |
||
| 366 | // Extensions won't be part of the autoloader chain, so first try to require them if they are available. |
||
| 367 | if (!is_file($callback['driver_location'])) { |
||
| 368 | return false; |
||
| 369 | } else { |
||
| 370 | require_once $callback['driver_location']; |
||
| 371 | } |
||
| 372 | |||
| 373 | // Publish page, /symphony/publish/{section_handle}/ |
||
| 374 | } elseif ($bits[0] === 'publish') { |
||
| 375 | if (!isset($bits[1])) { |
||
| 376 | return false; |
||
| 377 | } |
||
| 378 | |||
| 379 | $callback['driver'] = 'publish'; |
||
| 380 | $callback['driver_location'] = CONTENT . '/content.publish.php'; |
||
| 381 | $callback['pageroot'] = '/' . $bits[0] . '/' . $bits[1] . '/'; |
||
| 382 | $callback['classname'] = 'contentPublish'; |
||
| 383 | |||
| 384 | // Everything else |
||
| 385 | } else { |
||
| 386 | $callback['driver'] = ucfirst($bits[0]); |
||
| 387 | $callback['pageroot'] = '/' . $bits[0] . '/'; |
||
| 388 | |||
| 389 | if (isset($bits[1])) { |
||
| 390 | $callback['driver'] = $callback['driver'] . ucfirst($bits[1]); |
||
| 391 | $callback['pageroot'] .= $bits[1] . '/'; |
||
| 392 | } |
||
| 393 | |||
| 394 | $callback['classname'] = 'content' . $callback['driver']; |
||
| 395 | $callback['driver'] = strtolower($callback['driver']); |
||
| 396 | $callback['driver_location'] = CONTENT . '/content.' . $callback['driver'] . '.php'; |
||
| 397 | } |
||
| 398 | |||
| 399 | // Parse the context |
||
| 400 | if (isset($callback['classname'])) { |
||
| 401 | $page = new $callback['classname']; |
||
| 402 | |||
| 403 | // Named context |
||
| 404 | if (method_exists($page, 'parseContext')) { |
||
| 405 | $page->parseContext($callback['context'], $bits); |
||
| 406 | |||
| 407 | // Default context |
||
| 408 | View Code Duplication | } elseif (isset($bits[2])) { |
|
| 409 | $callback['context'] = preg_split('/\//', $bits[2], -1, PREG_SPLIT_NO_EMPTY); |
||
| 410 | } |
||
| 411 | } |
||
| 412 | |||
| 413 | /** |
||
| 414 | * Immediately after determining which class will resolve the current page, this |
||
| 415 | * delegate allows extension to modify the routing or provide additional information. |
||
| 416 | * |
||
| 417 | * @since Symphony 2.3.1 |
||
| 418 | * @delegate AdminPagePostCallback |
||
| 419 | * @param string $context |
||
| 420 | * '/backend/' |
||
| 421 | * @param string $page |
||
| 422 | * The current URL string, after the SYMPHONY_URL constant (which is `/symphony/` |
||
| 423 | * at the moment. |
||
| 424 | * @param array $parts |
||
| 425 | * An array representation of `$page` |
||
| 426 | * @param array $callback |
||
| 427 | * An associative array that contains `driver`, `pageroot`, `classname` and |
||
| 428 | * `context` keys. The `driver_location` is the path to the class to render this |
||
| 429 | * page, `driver` should be the view to render, the `classname` the name of the |
||
| 430 | * class, `pageroot` the rootpage, before any extra URL params and `context` can |
||
| 431 | * provide additional information about the page |
||
| 432 | */ |
||
| 433 | Symphony::ExtensionManager()->notifyMembers('AdminPagePostCallback', '/backend/', array( |
||
| 434 | 'page' => $this->_currentPage, |
||
| 435 | 'parts' => $bits, |
||
| 436 | 'callback' => &$callback |
||
| 437 | )); |
||
| 438 | |||
| 439 | if (!isset($callback['driver_location']) || !is_file($callback['driver_location'])) { |
||
| 440 | return false; |
||
| 441 | } |
||
| 442 | |||
| 443 | return $callback; |
||
| 444 | } |
||
| 445 | |||
| 446 | /** |
||
| 447 | * If a page is not found in the Symphony backend, this function should |
||
| 448 | * be called which will raise a customError to display the default Symphony |
||
| 449 | * page not found template |
||
| 450 | */ |
||
| 451 | public function errorPageNotFound() |
||
| 459 | |||
| 460 | /** |
||
| 461 | * This function determines whether an administrative alert can be |
||
| 462 | * displayed on the current page. It ensures that the page exists, |
||
| 463 | * and the user is logged in and a developer |
||
| 464 | * |
||
| 465 | * @since Symphony 2.2 |
||
| 466 | * @return boolean |
||
| 467 | */ |
||
| 468 | private function __canAccessAlerts() |
||
| 476 | |||
| 477 | /** |
||
| 478 | * Scan the install directory to look for new migrations that can be applied |
||
| 479 | * to update this version of Symphony. If one if found, a new Alert is added |
||
| 480 | * to the page. |
||
| 481 | * |
||
| 482 | * @since Symphony 2.5.2 |
||
| 483 | * @return boolean |
||
| 484 | * Returns true if there is an update available, false otherwise. |
||
| 485 | */ |
||
| 486 | public function checkCoreForUpdates() |
||
| 514 | |||
| 515 | /** |
||
| 516 | * Checks all installed extensions to see any have an outstanding update. If any do |
||
| 517 | * an Alert will be added to the current page directing the Author to the Extension page |
||
| 518 | * |
||
| 519 | * @since Symphony 2.5.2 |
||
| 520 | */ |
||
| 521 | public function checkExtensionsForUpdates() |
||
| 538 | } |
||
| 539 |
If you define a variable conditionally, it can happen that it is not defined for all execution paths.
Let’s take a look at an example:
In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.
Available Fixes
Check for existence of the variable explicitly:
Define a default value for the variable:
Add a value for the missing path: