Complex classes like Pageadmin 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 Pageadmin, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 32 | class Pageadmin extends Base |
||
| 33 | { |
||
| 34 | /** |
||
| 35 | * Pageadmin constructor. |
||
| 36 | * @param ServiceManager $serviceManager |
||
| 37 | */ |
||
| 38 | public function __construct(ServiceManager $serviceManager) |
||
| 42 | |||
| 43 | /** |
||
| 44 | * |
||
| 45 | */ |
||
| 46 | public function preparePage() |
||
| 47 | { |
||
| 48 | $this->P = new \HaaseIT\HCSF\CorePage($this->serviceManager); |
||
| 49 | $this->P->cb_pagetype = 'content'; |
||
| 50 | $this->P->cb_subnav = 'admin'; |
||
| 51 | |||
| 52 | $this->P->cb_customcontenttemplate = 'pageadmin'; |
||
| 53 | |||
| 54 | // adding language to page here |
||
| 55 | if (filter_input(INPUT_REQUEST, 'action') === 'insert_lang') { |
||
| 56 | $this->insertLang(); |
||
| 57 | } |
||
| 58 | |||
| 59 | $getaction = filter_input(INPUT_GET, 'action'); |
||
| 60 | if ($getaction === null) { |
||
| 61 | $this->P->cb_customdata['pageselect'] = $this->showPageselect(); |
||
| 62 | } elseif (!empty(filter_input(INPUT_REQUEST, 'page_key')) && ($getaction === 'edit' || $getaction === 'delete')) { |
||
| 63 | if ($getaction === 'delete' && filter_input(INPUT_POST, 'delete') === 'do') { |
||
| 64 | $this->handleDeletePage(); |
||
| 65 | } else { // edit or update page |
||
| 66 | $this->handleEditPage(); |
||
| 67 | } |
||
| 68 | } elseif ($getaction === 'addpage') { |
||
| 69 | $this->handleAddPage(); |
||
| 70 | } |
||
| 71 | } |
||
| 72 | |||
| 73 | protected function handleDeletePage() |
||
| 84 | |||
| 85 | protected function handleAddPage() |
||
| 112 | |||
| 113 | /** |
||
| 114 | * @return array |
||
| 115 | */ |
||
| 116 | protected function showPageselect() { |
||
| 153 | |||
| 154 | protected function insertLang() |
||
| 169 | |||
| 170 | protected function handleEditPage() |
||
| 171 | { |
||
| 172 | $requestpagekey = filter_input(INPUT_REQUEST, 'page_key', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW | FILTER_FLAG_STRIP_HIGH); |
||
| 173 | if ($requestpagekey !== null && $Ptoedit = new UserPage($this->serviceManager, $requestpagekey, true)) { |
||
| 174 | if (filter_input(INPUT_REQUEST, 'action_a') === 'true') { |
||
| 175 | $Ptoedit = $this->updatePage($Ptoedit); |
||
| 176 | } |
||
| 177 | $this->P->cb_customdata['page'] = $Ptoedit; |
||
| 178 | $this->P->cb_customdata['admin_page_types'] = HelperConfig::$core['admin_page_types']; |
||
| 179 | $this->P->cb_customdata['admin_page_groups'] = HelperConfig::$core['admin_page_groups']; |
||
| 180 | $aOptions = ['']; |
||
| 181 | foreach (HelperConfig::$navigation as $sKey => $aValue) { |
||
| 182 | if ($sKey === 'admin') { |
||
| 183 | continue; |
||
| 184 | } |
||
| 185 | $aOptions[] = $sKey; |
||
| 186 | } |
||
| 187 | $this->P->cb_customdata['subnavarea_options'] = $aOptions; |
||
| 188 | unset($aOptions); |
||
| 189 | |||
| 190 | // show archived versions of this page |
||
| 191 | if ($Ptoedit->oPayload->cl_id != NULL) { |
||
| 192 | |||
| 193 | $dbal = $this->serviceManager->get('dbal'); |
||
| 194 | |||
| 195 | /** @var \Doctrine\DBAL\Query\QueryBuilder $queryBuilder */ |
||
| 196 | $queryBuilder = $dbal->createQueryBuilder(); |
||
| 197 | $queryBuilder |
||
| 198 | ->select('*') |
||
| 199 | ->from('content_lang_archive') |
||
| 200 | ->where('cl_id = ?') |
||
| 201 | ->andWhere('cl_lang = ?') |
||
| 202 | ->setParameter(0, $Ptoedit->oPayload->cl_id) |
||
| 203 | ->setParameter(1, HelperConfig::$lang) |
||
| 204 | ->orderBy('cla_timestamp', 'DESC') |
||
| 205 | ; |
||
| 206 | $statement = $queryBuilder->execute(); |
||
| 207 | $iArchivedRows = $statement->rowCount(); |
||
| 208 | |||
| 209 | if ($iArchivedRows > 0) { |
||
| 210 | $aListSetting = [ |
||
| 211 | ['title' => 'cla_timestamp', 'key' => 'cla_timestamp', 'width' => '15%', 'linked' => false,], |
||
| 212 | ['title' => 'cl_html', 'key' => 'cl_html', 'width' => '40%', 'linked' => false, 'escapehtmlspecialchars' => true,], |
||
| 213 | ['title' => 'cl_keywords', 'key' => 'cl_keywords', 'width' => '15%', 'linked' => false, 'escapehtmlspecialchars' => true,], |
||
| 214 | ['title' => 'cl_description', 'key' => 'cl_description', 'width' => '15%', 'linked' => false, 'escapehtmlspecialchars' => true,], |
||
| 215 | ['title' => 'cl_title', 'key' => 'cl_title', 'width' => '15%', 'linked' => false, 'escapehtmlspecialchars' => true,], |
||
| 216 | ]; |
||
| 217 | $aData = $statement->fetchAll(); |
||
| 218 | $this->P->cb_customdata['archived_list'] = \HaaseIT\Toolbox\Tools::makeListtable( |
||
| 219 | $aListSetting, |
||
| 220 | $aData, |
||
| 221 | $this->serviceManager->get('twig') |
||
| 222 | ); |
||
| 223 | } |
||
| 224 | } |
||
| 225 | } else { |
||
| 226 | \HaaseIT\HCSF\Helper::terminateScript(HardcodedText::get('pageadmin_exception_pagenotfound')); |
||
| 227 | } |
||
| 228 | } |
||
| 229 | |||
| 230 | protected function updatePage(UserPage $Ptoedit) |
||
| 262 | } |
||
| 263 |
Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.
Let’s take a look at an example:
As you can see in this example, the array
$myArrayis initialized the first time when the foreach loop is entered. You can also see that the value of thebarkey is only written conditionally; thus, its value might result from a previous iteration.This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.