Enalean /
tuleap
Complex classes like DivBasedTabbedLayout 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 DivBasedTabbedLayout, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 27 | class DivBasedTabbedLayout extends Layout |
||
| 28 | { |
||
| 29 | /** |
||
| 30 | * The root location for images |
||
| 31 | * |
||
| 32 | * @var string $imgroot |
||
| 33 | */ |
||
| 34 | var $imgroot; |
||
| 35 | |||
| 36 | /** |
||
| 37 | * DivBasedTabbedLayout - Constructor |
||
| 38 | */ |
||
| 39 | function __construct($root) { |
||
| 40 | // Parent constructor |
||
| 41 | parent::__construct($root); |
||
| 42 | $this->imgroot = $root.'/images/'; |
||
| 43 | } |
||
| 44 | |||
| 45 | function getBodyHeader($params) { |
||
| 46 | /* A 2x2 table header |
||
| 47 | Organisation logo | users actions |
||
| 48 | Separator or any image | Search box |
||
| 49 | NB: Original OsdnNavBar has been removed from first cell. <td align="center">'.$this->getOsdnNavBar() |
||
| 50 | */ |
||
| 51 | $output = ' |
||
| 52 | <table cellpadding="0" cellspacing="0" border="0" width="100%"> |
||
| 53 | <tr> |
||
| 54 | <td class="header_logo">'. $this->getBodyHeaderLogo() .'</td> |
||
| 55 | <td class="header_actions">'; |
||
| 56 | $output .= $this->getBodyHeaderActions($params); |
||
| 57 | $output .= '<div class="header_searchbox">'.$this->getSearchBox().'</div> |
||
| 58 | </td> |
||
| 59 | </tr> |
||
| 60 | </table>'; |
||
| 61 | return $output; |
||
| 62 | } |
||
| 63 | function getBodyHeaderLogo() { |
||
| 66 | function getBodyHeaderActions($params) { |
||
| 67 | $html = ''; |
||
| 68 | $html .= '<ul>'; |
||
| 69 | if (user_isloggedin()) { |
||
| 70 | |||
| 71 | $html .= '<li class="header_actions_nolink">'.$GLOBALS['Language']->getText('include_menu','logged_in').': '.user_getname().'</li>'; |
||
| 72 | $html .= '<li><a href="/account/logout.php">'.$GLOBALS['Language']->getText('include_menu','logout').'</a></li>'; |
||
| 73 | if((isset($GLOBALS['sys_use_project_registration']) && $GLOBALS['sys_use_project_registration'] ==1) || !isset($GLOBALS['sys_use_project_registration'])) { |
||
| 74 | $html .= '<li><a href="/project/register.php">'.$GLOBALS['Language']->getText('include_menu','register_new_proj').'</a></li>'; |
||
| 75 | } |
||
| 76 | if (!HTTPRequest::instance()->isPost()) { |
||
| 77 | $add_bookmark_url = http_build_query(array( |
||
| 78 | 'bookmark_url' => $_SERVER['REQUEST_URI'], |
||
| 79 | 'bookmark_title' => str_replace($GLOBALS['sys_name'].': ', '', $params['title']) |
||
| 80 | )); |
||
| 81 | $html .= '<li class="bookmarkpage"><a href="/my/bookmark_add.php?'.$add_bookmark_url.'">'.$GLOBALS['Language']->getText('include_menu','bookmark_this_page').'</a></li>'; |
||
| 82 | } |
||
| 83 | } else { |
||
| 84 | $html .= '<li class="header_actions_nolink highlight">'.$GLOBALS['Language']->getText('include_menu','not_logged_in').'</li>'; |
||
| 85 | |||
| 86 | $login_url = '/account/login.php'; |
||
| 87 | if ($_SERVER['REQUEST_URI'] != $login_url) { |
||
| 88 | $login_url .= '?return_to='.urlencode($_SERVER['REQUEST_URI']); |
||
| 89 | } |
||
| 90 | |||
| 91 | $html .= '<li><a href="'.$this->purifier->purify($login_url).'">'.$GLOBALS['Language']->getText('include_menu','login').'</a></li>'; |
||
| 92 | $em =& EventManager::instance(); |
||
| 93 | $display_new_user = true; |
||
| 94 | $em->processEvent('display_newaccount', array('allow' => &$display_new_user)); |
||
| 95 | if ($display_new_user) { |
||
| 96 | $html .= '<li><a href="/account/register.php">'.$GLOBALS['Language']->getText('include_menu','new_user').'</a></li>'; |
||
| 97 | } |
||
| 98 | |||
| 99 | } |
||
| 100 | $html .= '</ul>'; |
||
| 101 | return $html; |
||
| 102 | } |
||
| 103 | |||
| 104 | /** |
||
| 105 | * header() - "steel theme" top of page |
||
| 106 | * |
||
| 107 | * @param array Header parameters array |
||
| 108 | */ |
||
| 109 | function header($params) { |
||
| 110 | global $Language; |
||
| 111 | |||
| 112 | $this->generic_header($params); |
||
| 113 | ?> |
||
| 114 | |||
| 115 | <body class="<?php echo $this->getClassnamesForBodyTag($params) ?>"> |
||
| 116 | <div id="wrapper"> |
||
| 117 | <?php |
||
| 118 | $deprecated = $this->getBrowserDeprecatedMessage(); |
||
| 119 | if ($deprecated) { |
||
| 120 | echo '<div id="browser_deprecated">'.$deprecated.'</div>'; |
||
| 121 | } |
||
| 122 | echo $this->getMOTD(); |
||
| 123 | ?> |
||
| 124 | |||
| 125 | <div id="header"><?php echo $this->getBodyHeader($params); ?></div> |
||
| 126 | |||
| 127 | <div id="navigation"> |
||
| 128 | <?php |
||
| 129 | echo $this->outerTabs($params); |
||
| 130 | |||
| 131 | $main_body_class = ''; |
||
| 132 | if (isset($params['toptab']) && is_string($params['toptab'])) { |
||
| 133 | $main_body_class = 'service-' . $params['toptab']; |
||
| 134 | } |
||
| 135 | |||
| 136 | ?> |
||
| 137 | </div> <!-- headertab --> |
||
| 138 | |||
| 139 | <div class="main_body_row <?= $main_body_class;?>"> |
||
| 140 | |||
| 141 | <div class="contenttable"> |
||
| 142 | <?php |
||
| 143 | echo $this->getBreadCrumbs(); |
||
| 144 | echo $this->getToolbar(); |
||
| 145 | echo $this->_getFeedback(); |
||
| 146 | $this->_feedback->display(); |
||
| 147 | echo $this->getNotificationPlaceholder(); |
||
| 148 | } |
||
| 149 | |||
| 150 | function footer($params) { |
||
| 151 | if (!isset($params['showfeedback']) || $params['showfeedback']) { |
||
| 152 | echo $this->_getFeedback(); |
||
| 153 | } |
||
| 154 | ?> |
||
| 155 | </div> <!-- class="contenttable"> --> |
||
| 156 | </div> <!-- class="main_body_row"> --> |
||
| 157 | </div> <!-- wrapper --> |
||
| 158 | <?php echo $this->getCustomFooter(); ?> |
||
| 159 | |||
| 160 | <?php |
||
| 161 | $this->generic_footer($params); |
||
| 162 | } |
||
| 163 | |||
| 164 | function getCustomFooter() { |
||
| 167 | |||
| 168 | function _getTogglePlusForWidgets() { |
||
| 174 | |||
| 175 | /** |
||
| 176 | * boxTop() - Top HTML box |
||
| 177 | * |
||
| 178 | * @param string Box title |
||
| 179 | * @param bool Whether to echo or return the results |
||
| 180 | * @param string The box background color |
||
| 181 | */ |
||
| 182 | function boxTop($title) { |
||
| 183 | return ' |
||
| 184 | <!-- Box Top Start --> |
||
| 185 | |||
| 186 | <table cellspacing="1" width="100%" border="0" class="boxtable"> |
||
| 187 | <tr class="boxtitle"> |
||
| 188 | <td class="boxtop_center" width="100%"><span class="titlebar">'.$title.'</span></td> |
||
| 189 | </tr> |
||
| 190 | <tr> |
||
| 191 | <td> |
||
| 192 | <table cellspacing="0" cellpadding="2" width="100%" border="0"> |
||
| 193 | <tr align="left""> |
||
| 194 | <td> |
||
| 195 | <!-- Box Top End -->'; |
||
| 196 | } |
||
| 197 | |||
| 198 | /** |
||
| 199 | * boxMiddle() - Middle HTML box |
||
| 200 | * |
||
| 201 | * @param string Box title |
||
| 202 | * @param string The box background color |
||
| 203 | */ |
||
| 204 | function boxMiddle($title) { |
||
| 205 | return ' |
||
| 206 | <!-- Box Middle Start --> |
||
| 207 | </td> |
||
| 208 | </tr> |
||
| 209 | <tr align="center" class="boxitem"> |
||
| 210 | <td><span class="titlebar">'.$title.'</span></td> |
||
| 211 | </tr> |
||
| 212 | <tr align="left"> |
||
| 213 | <td colspan="2"> |
||
| 214 | <!-- Box Middle End -->'; |
||
| 215 | } |
||
| 216 | |||
| 217 | /** |
||
| 218 | * boxBottom() - Bottom HTML box |
||
| 219 | * |
||
| 220 | * @param bool Whether to echo or return the results |
||
| 221 | */ |
||
| 222 | function boxBottom() { |
||
| 223 | return ' |
||
| 224 | </td> |
||
| 225 | </tr> |
||
| 226 | </table> |
||
| 227 | </td> |
||
| 228 | </tr> |
||
| 229 | <!-- Box Bottom Start --> |
||
| 230 | </table><br /> |
||
| 231 | <!-- Box Bottom End -->'; |
||
| 232 | } |
||
| 233 | |||
| 234 | /** |
||
| 235 | * boxGetAltRowStyle() - Get an alternating row style for tables |
||
| 236 | * |
||
| 237 | * @param int Row number |
||
| 238 | */ |
||
| 239 | function boxGetAltRowStyle($i) { |
||
| 240 | if ($i % 2 == 0) { |
||
| 241 | return 'background="'.$this->imgroot.'vert-grad.png"'; |
||
| 242 | } else { |
||
| 243 | return 'background="'.$this->imgroot.'box-grad.png"'; |
||
| 244 | } |
||
| 245 | } |
||
| 246 | |||
| 247 | function outerTabs($params) { |
||
| 364 | |||
| 365 | /** |
||
| 366 | * projectTabs() - Prints out the project tabs, contained here in case |
||
| 367 | * we want to allow it to be overriden |
||
| 368 | * |
||
| 369 | * @param string Is the tab currently selected |
||
| 370 | * @param string Is the group we should look up get title info |
||
| 371 | */ |
||
| 372 | function projectTabs($toptab,$group) { |
||
| 375 | |||
| 376 | function project_tabs($toptab,$group_id) { |
||
| 377 | $pm = ProjectManager::instance(); |
||
| 378 | $project=$pm->getProject($group_id); |
||
| 379 | if ($project->isError()) { |
||
| 380 | //wasn't found or some other problem |
||
| 381 | return; |
||
| 382 | } |
||
| 383 | $menuTree = new TreeNode(); |
||
| 384 | $output = ''; |
||
| 385 | $tabs = $this->_getProjectTabs($toptab, $project); |
||
| 386 | $nb = count($tabs); |
||
| 387 | $selected = false; |
||
| 388 | for($i = 0; $i < $nb ; $i++) { |
||
| 389 | if ($tabs[$i]['enabled'] === true) { |
||
| 390 | $selected = true; |
||
| 391 | } |
||
| 392 | $menuTree->addChild(new TreeNode(array('link'=>$tabs[$i]['link'] |
||
| 393 | ,'title'=>$tabs[$i]['label'] |
||
| 394 | ,'selected'=>$tabs[$i]['enabled']))); |
||
| 395 | } |
||
| 396 | //$output .= $this->tabGenerator($TABS_DIRS,$TABS_TITLES,true,$selected, 2); |
||
| 397 | //echo $output; |
||
| 398 | return $menuTree; |
||
| 399 | } |
||
| 400 | |||
| 401 | /** |
||
| 402 | * @param sel_tab_bgcolor DEPRECATED |
||
| 403 | */ |
||
| 404 | function tabGenerator($TABS_DIRS,$TABS_TITLES,$nested=false,$selected=false,$level) { |
||
| 405 | $count=count($TABS_DIRS); |
||
| 406 | $width=intval((100/$count)); |
||
| 407 | |||
| 408 | $return = ''; |
||
| 409 | |||
| 410 | $return .= ' |
||
| 411 | |||
| 412 | <!-- start tabs --> |
||
| 413 | <ul id="level_'.$level.'">'; |
||
| 414 | |||
| 415 | if ($nested) { |
||
| 416 | $inner='bottomtab'; |
||
| 417 | } else { |
||
| 418 | $inner='toptab'; |
||
| 419 | } |
||
| 420 | $rowspan = ''; |
||
| 421 | for ($i=0; $i<$count; $i++) { |
||
| 422 | // |
||
| 423 | // middle tabs |
||
| 424 | // |
||
| 425 | $wassel=($selected==$i-1); |
||
| 426 | $issel=($selected==$i); |
||
| 427 | |||
| 428 | if($issel) |
||
| 429 | $address = '<span>'.$TABS_TITLES[$i].'</span>'; |
||
| 430 | else |
||
| 431 | $address = '<a href="'.$TABS_DIRS[$i].'">'.$TABS_TITLES[$i].'</a>'; |
||
| 432 | |||
| 433 | $return .= ' |
||
| 434 | <li>'.$address.'</li>'; |
||
| 435 | |||
| 436 | } |
||
| 437 | $return .= ' |
||
| 438 | </ul> |
||
| 439 | <!-- end tabs --> |
||
| 440 | '; |
||
| 441 | return $return; |
||
| 442 | } |
||
| 443 | |||
| 444 | //diplaying search box in body |
||
| 445 | function bodySearchBox() { |
||
| 447 | } |
||
| 448 | |||
| 449 | ?> |
||
| 450 |
An exit expression should only be used in rare cases. For example, if you write a short command line script.
In most cases however, using an
exitexpression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage.