BurningFlipside /
CommonCode
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 FlipPage 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 FlipPage, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 26 | class FlipPage extends WebPage |
||
| 27 | { |
||
| 28 | /** The currently logged in user or null if no user is logged in */ |
||
| 29 | public $user; |
||
| 30 | /** An array of links to put in the header */ |
||
| 31 | public $links; |
||
| 32 | /** An array of notifications to draw on the page */ |
||
| 33 | public $notifications; |
||
| 34 | /** Should we draw the header? */ |
||
| 35 | public $header; |
||
| 36 | /** The login page URL */ |
||
| 37 | public $loginUrl; |
||
| 38 | /** The logout page URL */ |
||
| 39 | public $logoutUrl; |
||
| 40 | /** Should we use minified JS/CSS? */ |
||
| 41 | protected $minified = null; |
||
| 42 | /** Should we use local JS/CSS or Content Delivery Networks? */ |
||
| 43 | protected $cdn = null; |
||
| 44 | /** Draw the analytics scripts */ |
||
| 45 | protected $analytics = true; |
||
| 46 | /** An instance of the Settings class */ |
||
| 47 | protected $settings; |
||
| 48 | |||
| 49 | public $wwwUrl; |
||
| 50 | public $wikiUrl; |
||
| 51 | public $profilesUrl; |
||
| 52 | public $secureUrl; |
||
| 53 | |||
| 54 | /** |
||
| 55 | * Create a webpage with JQuery, Bootstrap, etc |
||
| 56 | * |
||
| 57 | * @param string $title The webpage title |
||
| 58 | * @param boolean $header Draw the header bar? |
||
| 59 | * |
||
| 60 | * @SuppressWarnings("StaticAccess") |
||
| 61 | */ |
||
| 62 | public function __construct($title, $header = true) |
||
| 96 | |||
| 97 | /** |
||
| 98 | * Get the external site links for this page |
||
| 99 | * |
||
| 100 | */ |
||
| 101 | protected function getSites() |
||
| 105 | |||
| 106 | /** |
||
| 107 | * Add the links to be used in the header |
||
| 108 | * |
||
| 109 | * @SuppressWarnings("Superglobals") |
||
| 110 | * |
||
| 111 | * @todo Consider pulling the about menu from the settings file or a DB |
||
| 112 | */ |
||
| 113 | protected function addAllLinks() |
||
| 114 | { |
||
| 115 | View Code Duplication | if($this->user === false || $this->user === null) |
|
| 116 | { |
||
| 117 | if(isset($_SERVER['REQUEST_URI']) && strstr($_SERVER['REQUEST_URI'], 'logout.php') === false) |
||
| 118 | { |
||
| 119 | $this->addLink('Login', $this->loginUrl); |
||
| 120 | } |
||
| 121 | } |
||
| 122 | else |
||
| 123 | { |
||
| 124 | $this->add_links(); |
||
| 125 | $this->addLink('Logout', $this->logoutUrl); |
||
| 126 | } |
||
| 127 | if($this->aboutUrl !== false) |
||
| 128 | { |
||
| 129 | if(!empty($this->aboutMenu)) |
||
| 130 | { |
||
| 131 | $this->addLink('About', $this->aboutUrl, $this->aboutMenu); |
||
| 132 | } |
||
| 133 | else |
||
| 134 | { |
||
| 135 | $this->addLink('About', $this->aboutUrl); |
||
| 136 | } |
||
| 137 | } |
||
| 138 | } |
||
| 139 | |||
| 140 | /** |
||
| 141 | * Setup minified and cdn class varibles from defaults or the settings file |
||
| 142 | */ |
||
| 143 | private function setupVars() |
||
| 160 | |||
| 161 | /** |
||
| 162 | * Add a JavaScript file from its src URI |
||
| 163 | * |
||
| 164 | * @param string $uri The webpath to the JavaScript file |
||
| 165 | * @param boolean $async Can the JavaScript be loaded asynchronously? |
||
| 166 | */ |
||
| 167 | public function addJSByURI($uri, $async = true) |
||
| 179 | |||
| 180 | /** |
||
| 181 | * Add a Cascading Style Sheet file from its src URI |
||
| 182 | * |
||
| 183 | * @param string $uri The webpath to the Cascading Style Sheet file |
||
| 184 | * @param boolean $async Can the CSS be loaded asynchronously? |
||
| 185 | */ |
||
| 186 | public function addCSSByURI($uri, $async = false) |
||
| 196 | |||
| 197 | /** |
||
| 198 | * Add a JavaScript file from a set of files known to the framework |
||
| 199 | * |
||
| 200 | * @param string $jsFileID the ID of the JS file |
||
| 201 | * @param boolean $async Can the JS file be loaded asynchronously? |
||
| 202 | */ |
||
| 203 | public function addWellKnownJS($jsFileID, $async = false) |
||
| 210 | |||
| 211 | /** |
||
| 212 | * Add a CSS file from a set of files known to the framework |
||
| 213 | * |
||
| 214 | * @param string $cssFileID the ID of the CSS file |
||
| 215 | * @param boolean $async Can the CSS file be loaded asynchronously? |
||
| 216 | */ |
||
| 217 | public function addWellKnownCSS($cssFileID, $async = false) |
||
| 224 | |||
| 225 | /** |
||
| 226 | * Add files needed by the Bootstrap framework |
||
| 227 | */ |
||
| 228 | private function addBootstrap() |
||
| 235 | |||
| 236 | protected function getSiteLinksForHeader() |
||
| 247 | |||
| 248 | /** |
||
| 249 | * Get the link for the HREF |
||
| 250 | * |
||
| 251 | * @return string The HREF for the dropdown |
||
| 252 | */ |
||
| 253 | protected function getHrefForDropdown(&$link) |
||
| 263 | |||
| 264 | protected function getDropdown($link, $name) |
||
| 278 | |||
| 279 | protected function getLinkByName($name, $links) |
||
| 291 | |||
| 292 | protected function getLinksMenus() |
||
| 302 | |||
| 303 | /** |
||
| 304 | * Draw the header for the page |
||
| 305 | */ |
||
| 306 | protected function addHeader() |
||
| 332 | |||
| 333 | /** Notification that is green for success */ |
||
| 334 | const NOTIFICATION_SUCCESS = 'alert-success'; |
||
| 335 | /** Notification that is blue for infomrational messages */ |
||
| 336 | const NOTIFICATION_INFO = 'alert-info'; |
||
| 337 | /** Notification that is yellow for warning */ |
||
| 338 | const NOTIFICATION_WARNING = 'alert-warning'; |
||
| 339 | /** Notification that is red for error */ |
||
| 340 | const NOTIFICATION_FAILED = 'alert-danger'; |
||
| 341 | |||
| 342 | /** |
||
| 343 | * Add a notification to the page |
||
| 344 | * |
||
| 345 | * @param string $message The message to show in the notifcation |
||
| 346 | * @param string $severity The severity of the notifcation |
||
| 347 | * @param boolean $dismissible Can the user dismiss the notificaton? |
||
| 348 | */ |
||
| 349 | public function addNotification($message, $severity = self::NOTIFICATION_INFO, $dismissible = true) |
||
| 353 | |||
| 354 | /** |
||
| 355 | * Draw all notifications to the page |
||
| 356 | */ |
||
| 357 | private function renderNotifications() |
||
| 399 | |||
| 400 | /** |
||
| 401 | * Add the no script block |
||
| 402 | */ |
||
| 403 | private function addNoScript() |
||
| 412 | |||
| 413 | /** |
||
| 414 | * Add the analytics script block |
||
| 415 | */ |
||
| 416 | private function addAnalyticsBlock() |
||
| 433 | |||
| 434 | /** |
||
| 435 | * Add some global js vars |
||
| 436 | */ |
||
| 437 | private function addJSGlobals() |
||
| 445 | |||
| 446 | /** |
||
| 447 | * Draw the page |
||
| 448 | * |
||
| 449 | * @param boolean $header Draw the header |
||
| 450 | */ |
||
| 451 | public function printPage($header = true) |
||
| 463 | |||
| 464 | /** |
||
| 465 | * Add a link to the header |
||
| 466 | * |
||
| 467 | * @param string $name The name of the link |
||
| 468 | * @param boolean|string $url The URL to link to |
||
| 469 | * @param boolean|array $submenu Any submenu items for the dropdown |
||
| 470 | */ |
||
| 471 | public function addLink($name, $url = false, $submenu = false) |
||
| 481 | |||
| 482 | /** |
||
| 483 | * Add the login form to the page |
||
| 484 | * |
||
| 485 | * @SuppressWarnings("StaticAccess") |
||
| 486 | */ |
||
| 487 | public function add_login_form() |
||
| 524 | |||
| 525 | /** |
||
| 526 | * Add additional links |
||
| 527 | */ |
||
| 528 | public function add_links() |
||
| 531 | } |
||
| 532 | /* vim: set tabstop=4 shiftwidth=4 expandtab: */ |
||
| 533 |
Loading history...
In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code:
Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion: