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 |
||
| 14 | class Administration extends Symphony |
||
| 15 | { |
||
| 16 | /** |
||
| 17 | * The path of the current page, ie. '/blueprints/sections/' |
||
| 18 | * @var string |
||
| 19 | */ |
||
| 20 | private $_currentPage = null; |
||
| 21 | |||
| 22 | /** |
||
| 23 | * An associative array of the page's callback, including the keys |
||
| 24 | * 'driver', which is a lowercase version of `$this->_currentPage` |
||
| 25 | * with any slashes removed, 'classname', which is the name of the class |
||
| 26 | * for this page, 'pageroot', which is the root page for the given page, (ie. |
||
| 27 | * excluding /saved/, /created/ or any sub pages of the current page that are |
||
| 28 | * handled using the _switchboard function. |
||
| 29 | * |
||
| 30 | * @see toolkit.AdministrationPage#__switchboard() |
||
| 31 | * @var array |
||
| 32 | */ |
||
| 33 | private $_callback = null; |
||
| 34 | |||
| 35 | /** |
||
| 36 | * The class representation of the current Symphony backend page, |
||
| 37 | * which is a subclass of the `HTMLPage` class. Symphony uses a convention |
||
| 38 | * of prefixing backend page classes with 'content'. ie. 'contentBlueprintsSections' |
||
| 39 | * @var HTMLPage |
||
| 40 | */ |
||
| 41 | public $Page; |
||
| 42 | |||
| 43 | /** |
||
| 44 | * Overrides the default Symphony constructor to add XSRF checking |
||
| 45 | */ |
||
| 46 | protected function __construct() |
||
| 47 | { |
||
| 48 | parent::__construct(); |
||
| 49 | |||
| 50 | // Ensure the request is legitimate. RE: #1874 |
||
| 51 | if (self::isXSRFEnabled()) { |
||
| 52 | XSRF::validateRequest(); |
||
| 53 | } |
||
| 54 | } |
||
| 55 | |||
| 56 | /** |
||
| 57 | * This function returns an instance of the Administration |
||
| 58 | * class. It is the only way to create a new Administration, as |
||
| 59 | * it implements the Singleton interface |
||
| 60 | * |
||
| 61 | * @return Administration |
||
| 62 | */ |
||
| 63 | public static function instance() |
||
| 64 | { |
||
| 65 | if (!(self::$_instance instanceof Administration)) { |
||
| 66 | self::$_instance = new Administration; |
||
| 67 | } |
||
| 68 | |||
| 69 | return self::$_instance; |
||
| 70 | } |
||
| 71 | |||
| 72 | /** |
||
| 73 | * Returns the current Page path, excluding the domain and Symphony path. |
||
| 74 | * |
||
| 75 | * @return string |
||
| 76 | * The path of the current page, ie. '/blueprints/sections/' |
||
| 77 | */ |
||
| 78 | public function getCurrentPageURL() |
||
| 82 | |||
| 83 | /** |
||
| 84 | * Overrides the Symphony isLoggedIn function to allow Authors |
||
| 85 | * to become logged into the backend when `$_REQUEST['auth-token']` |
||
| 86 | * is present. This logs an Author in using the loginFromToken function. |
||
| 87 | * A token may be 6 or 8 characters in length in the backend. A 6 or 16 character token |
||
| 88 | * is used for forget password requests, whereas the 8 character token is used to login |
||
| 89 | * an Author into the page |
||
| 90 | * |
||
| 91 | * @see core.Symphony#loginFromToken() |
||
| 92 | * @return boolean |
||
| 93 | */ |
||
| 94 | public static function isLoggedIn() |
||
| 102 | |||
| 103 | /** |
||
| 104 | * Given the URL path of a Symphony backend page, this function will |
||
| 105 | * attempt to resolve the URL to a Symphony content page in the backend |
||
| 106 | * or a page provided by an extension. This function checks to ensure a user |
||
| 107 | * is logged in, otherwise it will direct them to the login page |
||
| 108 | * |
||
| 109 | * @param string $page |
||
| 110 | * The URL path after the root of the Symphony installation, including a starting |
||
| 111 | * slash, such as '/login/' |
||
| 112 | * @throws SymphonyErrorPage |
||
| 113 | * @throws Exception |
||
| 114 | * @return HTMLPage |
||
| 115 | */ |
||
| 116 | private function __buildPage($page) |
||
| 217 | |||
| 218 | /** |
||
| 219 | * Scan the install directory to look for new migrations that can be applied |
||
| 220 | * to update this version of Symphony. If one if found, a new Alert is added |
||
| 221 | * to the page. |
||
| 222 | * |
||
| 223 | * @since Symphony 2.5.2 |
||
| 224 | * @return boolean |
||
| 225 | * Returns true if there is an update available, false otherwise. |
||
| 226 | */ |
||
| 227 | public function checkCoreForUpdates() |
||
| 228 | { |
||
| 229 | // Is there even an install directory to check? |
||
| 230 | if ($this->isInstallerAvailable() === false) { |
||
| 231 | return false; |
||
| 232 | } |
||
| 233 | |||
| 234 | try { |
||
| 235 | // The updater contains a version higher than the current Symphony version. |
||
| 236 | if ($this->isUpgradeAvailable()) { |
||
| 237 | $message = __('An update has been found in your installation to upgrade Symphony to %s.', array($this->getMigrationVersion())) . ' <a href="' . URL . '/install/">' . __('View update.') . '</a>'; |
||
| 238 | |||
| 239 | // The updater contains a version lower than the current Symphony version. |
||
| 240 | // The updater is the same version as the current Symphony install. |
||
| 241 | } else { |
||
| 242 | $message = __('Your Symphony installation is up to date, but the installer was still detected. For security reasons, it should be removed.') . ' <a href="' . URL . '/install/?action=remove">' . __('Remove installer?') . '</a>'; |
||
| 243 | } |
||
| 244 | |||
| 245 | // Can't detect update Symphony version |
||
| 246 | } catch (Exception $e) { |
||
| 247 | $message = __('An update script has been found in your installation.') . ' <a href="' . URL . '/install/">' . __('View update.') . '</a>'; |
||
| 248 | } |
||
| 249 | |||
| 250 | $this->Page->pageAlert($message, Alert::NOTICE); |
||
| 251 | |||
| 252 | return true; |
||
| 253 | } |
||
| 254 | |||
| 255 | /** |
||
| 256 | * Checks all installed extensions to see any have an outstanding update. If any do |
||
| 257 | * an Alert will be added to the current page directing the Author to the Extension page |
||
| 258 | * |
||
| 259 | * @since Symphony 2.5.2 |
||
| 260 | */ |
||
| 261 | public function checkExtensionsForUpdates() |
||
| 262 | { |
||
| 263 | $extensions = Symphony::ExtensionManager()->listInstalledHandles(); |
||
| 264 | |||
| 265 | if (is_array($extensions) && !empty($extensions)) { |
||
| 266 | foreach ($extensions as $name) { |
||
| 267 | $about = Symphony::ExtensionManager()->about($name); |
||
| 268 | |||
| 269 | if (array_key_exists('status', $about) && in_array(EXTENSION_REQUIRES_UPDATE, $about['status'])) { |
||
| 270 | $this->Page->pageAlert( |
||
| 271 | __('An extension requires updating.') . ' <a href="' . SYMPHONY_URL . '/system/extensions/">' . __('View extensions') . '</a>' |
||
| 272 | ); |
||
| 273 | break; |
||
| 274 | } |
||
| 275 | } |
||
| 276 | } |
||
| 277 | } |
||
| 278 | |||
| 279 | /** |
||
| 280 | * This function determines whether an administrative alert can be |
||
| 281 | * displayed on the current page. It ensures that the page exists, |
||
| 282 | * and the user is logged in and a developer |
||
| 283 | * |
||
| 284 | * @since Symphony 2.2 |
||
| 285 | * @return boolean |
||
| 286 | */ |
||
| 287 | private function __canAccessAlerts() |
||
| 295 | |||
| 296 | /** |
||
| 297 | * This function resolves the string of the page to the relevant |
||
| 298 | * backend page class. The path to the backend page is split on |
||
| 299 | * the slashes and the resulting pieces used to determine if the page |
||
| 300 | * is provided by an extension, is a section (index or entry creation) |
||
| 301 | * or finally a standard Symphony content page. If no page driver can |
||
| 302 | * be found, this function will return false. |
||
| 303 | * |
||
| 304 | * @uses AdminPagePostCallback |
||
| 305 | * @param string $page |
||
| 306 | * The full path (including the domain) of the Symphony backend page |
||
| 307 | * @return array|boolean |
||
| 308 | * If successful, this function will return an associative array that at the |
||
| 309 | * very least will return the page's classname, pageroot, driver, driver_location |
||
| 310 | * and context, otherwise this will return false. |
||
| 311 | */ |
||
| 312 | public function getPageCallback($page = null) |
||
| 313 | { |
||
| 314 | if (!$page && $this->_callback) { |
||
| 315 | return $this->_callback; |
||
| 316 | } elseif (!$page && !$this->_callback) { |
||
| 317 | trigger_error(__('Cannot request a page callback without first specifying the page.')); |
||
| 318 | } |
||
| 319 | |||
| 320 | $this->_currentPage = SYMPHONY_URL . preg_replace('/\/{2,}/', '/', $page); |
||
| 321 | $bits = preg_split('/\//', trim($page, '/'), 3, PREG_SPLIT_NO_EMPTY); |
||
| 322 | $callback = array( |
||
| 323 | 'driver' => null, |
||
| 324 | 'driver_location' => null, |
||
| 325 | 'context' => array(), |
||
| 326 | 'classname' => null, |
||
| 327 | 'pageroot' => null |
||
| 328 | ); |
||
| 329 | |||
| 330 | // Login page, /symphony/login/ |
||
| 331 | if ($bits[0] === 'login') { |
||
| 332 | $callback['driver'] = 'login'; |
||
| 333 | $callback['driver_location'] = CONTENT . '/content.login.php'; |
||
| 334 | $callback['classname'] = 'contentLogin'; |
||
| 335 | $callback['pageroot'] = '/login/'; |
||
| 336 | |||
| 337 | // Extension page, /symphony/extension/{extension_name}/ |
||
| 338 | } elseif ($bits[0] === 'extension' && isset($bits[1])) { |
||
| 339 | $extension_name = $bits[1]; |
||
| 340 | $bits = preg_split('/\//', trim($bits[2], '/'), 2, PREG_SPLIT_NO_EMPTY); |
||
| 341 | |||
| 342 | // check if extension is enabled, if it's not, pretend the extension doesn't |
||
| 343 | // even exist. #2367 |
||
| 344 | if (!ExtensionManager::isInstalled($extension_name)) { |
||
| 345 | return false; |
||
| 346 | } |
||
| 347 | |||
| 348 | $callback['driver'] = 'index'; |
||
| 349 | $callback['classname'] = 'contentExtension' . ucfirst($extension_name) . 'Index'; |
||
| 350 | $callback['pageroot'] = '/extension/' . $extension_name. '/'; |
||
| 351 | $callback['extension'] = $extension_name; |
||
| 352 | |||
| 353 | if (isset($bits[0])) { |
||
| 354 | $callback['driver'] = $bits[0]; |
||
| 355 | $callback['classname'] = 'contentExtension' . ucfirst($extension_name) . ucfirst($bits[0]); |
||
| 356 | $callback['pageroot'] .= $bits[0] . '/'; |
||
| 357 | } |
||
| 358 | |||
| 359 | View Code Duplication | if (isset($bits[1])) { |
|
| 360 | $callback['context'] = preg_split('/\//', $bits[1], -1, PREG_SPLIT_NO_EMPTY); |
||
| 361 | } |
||
| 362 | |||
| 363 | $callback['driver_location'] = EXTENSIONS . '/' . $extension_name . '/content/content.' . $callback['driver'] . '.php'; |
||
| 364 | // Extensions won't be part of the autoloader chain, so first try to require them if they are available. |
||
| 365 | if (!is_file($callback['driver_location'])) { |
||
| 366 | return false; |
||
| 367 | } else { |
||
| 368 | require_once $callback['driver_location']; |
||
| 369 | } |
||
| 370 | |||
| 371 | // Publish page, /symphony/publish/{section_handle}/ |
||
| 372 | } elseif ($bits[0] === 'publish') { |
||
| 373 | if (!isset($bits[1])) { |
||
| 374 | return false; |
||
| 375 | } |
||
| 376 | |||
| 377 | $callback['driver'] = 'publish'; |
||
| 378 | $callback['driver_location'] = CONTENT . '/content.publish.php'; |
||
| 379 | $callback['pageroot'] = '/' . $bits[0] . '/' . $bits[1] . '/'; |
||
| 380 | $callback['classname'] = 'contentPublish'; |
||
| 381 | |||
| 382 | // Everything else |
||
| 383 | } else { |
||
| 384 | $callback['driver'] = ucfirst($bits[0]); |
||
| 385 | $callback['pageroot'] = '/' . $bits[0] . '/'; |
||
| 386 | |||
| 387 | if (isset($bits[1])) { |
||
| 388 | $callback['driver'] = $callback['driver'] . ucfirst($bits[1]); |
||
| 389 | $callback['pageroot'] .= $bits[1] . '/'; |
||
| 390 | } |
||
| 391 | |||
| 392 | $callback['classname'] = 'content' . $callback['driver']; |
||
| 393 | $callback['driver'] = strtolower($callback['driver']); |
||
| 394 | $callback['driver_location'] = CONTENT . '/content.' . $callback['driver'] . '.php'; |
||
| 395 | } |
||
| 396 | |||
| 397 | // Parse the context |
||
| 398 | if (isset($callback['classname'])) { |
||
| 399 | $page = new $callback['classname']; |
||
| 400 | |||
| 401 | // Named context |
||
| 402 | if (method_exists($page, 'parseContext')) { |
||
| 403 | $page->parseContext($callback['context'], $bits); |
||
| 404 | |||
| 405 | // Default context |
||
| 406 | View Code Duplication | } elseif (isset($bits[2])) { |
|
| 407 | $callback['context'] = preg_split('/\//', $bits[2], -1, PREG_SPLIT_NO_EMPTY); |
||
| 408 | } |
||
| 409 | } |
||
| 410 | |||
| 411 | /** |
||
| 412 | * Immediately after determining which class will resolve the current page, this |
||
| 413 | * delegate allows extension to modify the routing or provide additional information. |
||
| 414 | * |
||
| 415 | * @since Symphony 2.3.1 |
||
| 416 | * @delegate AdminPagePostCallback |
||
| 417 | * @param string $context |
||
| 418 | * '/backend/' |
||
| 419 | * @param string $page |
||
| 420 | * The current URL string, after the SYMPHONY_URL constant (which is `/symphony/` |
||
| 421 | * at the moment. |
||
| 422 | * @param array $parts |
||
| 423 | * An array representation of `$page` |
||
| 424 | * @param array $callback |
||
| 425 | * An associative array that contains `driver`, `pageroot`, `classname` and |
||
| 426 | * `context` keys. The `driver_location` is the path to the class to render this |
||
| 427 | * page, `driver` should be the view to render, the `classname` the name of the |
||
| 428 | * class, `pageroot` the rootpage, before any extra URL params and `context` can |
||
| 429 | * provide additional information about the page |
||
| 430 | */ |
||
| 431 | Symphony::ExtensionManager()->notifyMembers('AdminPagePostCallback', '/backend/', array( |
||
| 432 | 'page' => $this->_currentPage, |
||
| 433 | 'parts' => $bits, |
||
| 434 | 'callback' => &$callback |
||
| 435 | )); |
||
| 436 | |||
| 437 | if (!isset($callback['driver_location']) || !is_file($callback['driver_location'])) { |
||
| 438 | return false; |
||
| 439 | } |
||
| 440 | |||
| 441 | return $callback; |
||
| 442 | } |
||
| 443 | |||
| 444 | /** |
||
| 445 | * Called by index.php, this function is responsible for rendering the current |
||
| 446 | * page on the Frontend. Two delegates are fired, AdminPagePreGenerate and |
||
| 447 | * AdminPagePostGenerate. This function runs the Profiler for the page build |
||
| 448 | * process. |
||
| 449 | * |
||
| 450 | * @uses AdminPagePreBuild |
||
| 451 | * @uses AdminPagePreGenerate |
||
| 452 | * @uses AdminPagePostGenerate |
||
| 453 | * @see core.Symphony#__buildPage() |
||
| 454 | * @see boot.getCurrentPage() |
||
| 455 | * @param string $page |
||
| 456 | * The result of getCurrentPage, which returns the $_GET['symphony-page'] |
||
| 457 | * variable. |
||
| 458 | * @throws Exception |
||
| 459 | * @throws SymphonyErrorPage |
||
| 460 | * @return string |
||
| 461 | * The HTML of the page to return |
||
| 462 | */ |
||
| 463 | public function display($page) |
||
| 514 | |||
| 515 | /** |
||
| 516 | * If a page is not found in the Symphony backend, this function should |
||
| 517 | * be called which will raise a customError to display the default Symphony |
||
| 518 | * page not found template |
||
| 519 | */ |
||
| 520 | public function errorPageNotFound() |
||
| 528 | } |
||
| 529 |
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: