HaaseIT /
HCSF
| Conditions | 19 |
| Paths | 280 |
| Total Lines | 146 |
| Lines | 0 |
| Ratio | 0 % |
| Changes | 0 | ||
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
| 1 | <?php |
||
| 89 | private function handleCustomerAdmin($CUA, $twig) |
||
| 90 | { |
||
| 91 | $sType = 'all'; |
||
| 92 | $type = filter_input(INPUT_GET, 'type'); |
||
| 93 | if ($type !== null) { |
||
| 94 | if ($type === 'active') { |
||
| 95 | $sType = 'active'; |
||
| 96 | } elseif ($type === 'inactive') { |
||
| 97 | $sType = 'inactive'; |
||
| 98 | } |
||
| 99 | } |
||
| 100 | $return = ''; |
||
| 101 | if (filter_input(INPUT_GET, 'action') === null) { |
||
| 102 | $querybuilder = $this->dbal->createQueryBuilder(); |
||
| 103 | $querybuilder |
||
| 104 | ->select(DB_ADDRESSFIELDS) |
||
| 105 | ->from('customer') |
||
| 106 | ->orderBy('cust_no', 'ASC') |
||
| 107 | ; |
||
| 108 | |||
| 109 | if ($sType === 'active') { |
||
| 110 | $querybuilder |
||
| 111 | ->where('cust_active = ?') |
||
| 112 | ->setParameter(0, 'y') |
||
| 113 | ; |
||
| 114 | } elseif ($sType === 'inactive') { |
||
| 115 | $querybuilder |
||
| 116 | ->where('cust_active = ?') |
||
| 117 | ->setParameter(0, 'n') |
||
| 118 | ; |
||
| 119 | } |
||
| 120 | $stmt = $querybuilder->execute(); |
||
| 121 | if ($stmt->rowCount() !== 0) { |
||
| 122 | $aData = $stmt->fetchAll(); |
||
| 123 | $return .= \HaaseIT\Toolbox\Tools::makeListtable($CUA, $aData, $twig); |
||
| 124 | } else { |
||
| 125 | $aInfo['nodatafound'] = true; |
||
|
|
|||
| 126 | } |
||
| 127 | } elseif (filter_input(INPUT_GET, 'action') === 'edit') { |
||
| 128 | $iId = filter_input(INPUT_GET, 'id', FILTER_SANITIZE_NUMBER_INT); |
||
| 129 | $aErr = []; |
||
| 130 | if (filter_input(INPUT_POST, 'doEdit') === 'yes') { |
||
| 131 | $sCustno = trim(filter_input(INPUT_POST, 'custno', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW)); |
||
| 132 | if (strlen($sCustno) < $this->config->getCustomer('minimum_length_custno')) { |
||
| 133 | $aErr['custnoinvalid'] = true; |
||
| 134 | } else { |
||
| 135 | $querybuilder = $this->dbal->createQueryBuilder(); |
||
| 136 | $querybuilder |
||
| 137 | ->select(DB_ADDRESSFIELDS) |
||
| 138 | ->from('customer') |
||
| 139 | ->where('cust_id != ?') |
||
| 140 | ->andWhere('cust_no = ?') |
||
| 141 | ->setParameter(0, $iId) |
||
| 142 | ->setParameter(1, $sCustno) |
||
| 143 | ; |
||
| 144 | $stmt = $querybuilder->execute(); |
||
| 145 | |||
| 146 | if ($stmt->rowCount() === 1) { |
||
| 147 | $aErr['custnoalreadytaken'] = true; |
||
| 148 | } |
||
| 149 | |||
| 150 | $querybuilder = $this->dbal->createQueryBuilder(); |
||
| 151 | $querybuilder |
||
| 152 | ->select(DB_ADDRESSFIELDS) |
||
| 153 | ->from('customer') |
||
| 154 | ->where('cust_id != ?') |
||
| 155 | ->andWhere('cust_email = ?') |
||
| 156 | ->setParameter(0, $iId) |
||
| 157 | ->setParameter(1, filter_input(INPUT_POST, 'email', FILTER_SANITIZE_EMAIL)) |
||
| 158 | ; |
||
| 159 | $stmt = $querybuilder->execute(); |
||
| 160 | if ($stmt->rowCount() === 1) { |
||
| 161 | $aErr['emailalreadytaken'] = true; |
||
| 162 | } |
||
| 163 | $aErr = $this->helperCustomer->validateCustomerForm($this->config->getLang(), $aErr, true); |
||
| 164 | if (count($aErr) === 0) { |
||
| 165 | $querybuilder = $this->dbal->createQueryBuilder(); |
||
| 166 | $querybuilder |
||
| 167 | ->update('customer') |
||
| 168 | ->set('cust_no', ':cust_no') |
||
| 169 | ->set('cust_email', ':cust_email') |
||
| 170 | ->set('cust_corp', ':cust_corp') |
||
| 171 | ->set('cust_name', ':cust_name') |
||
| 172 | ->set('cust_street', ':cust_street') |
||
| 173 | ->set('cust_zip', ':cust_zip') |
||
| 174 | ->set('cust_town', ':cust_town') |
||
| 175 | ->set('cust_phone', ':cust_phone') |
||
| 176 | ->set('cust_cellphone', ':cust_cellphone') |
||
| 177 | ->set('cust_fax', ':cust_fax') |
||
| 178 | ->set('cust_country', ':cust_country') |
||
| 179 | ->set('cust_group', ':cust_group') |
||
| 180 | ->set('cust_emailverified', ':cust_emailverified') |
||
| 181 | ->set('cust_active', ':cust_active') |
||
| 182 | ->where('cust_id = :cust_id') |
||
| 183 | ->setParameter(':cust_no', $sCustno) |
||
| 184 | ->setParameter(':cust_email', trim(filter_input(INPUT_POST, 'email', FILTER_SANITIZE_EMAIL))) |
||
| 185 | ->setParameter(':cust_corp', trim(filter_input(INPUT_POST, 'corpname', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 186 | ->setParameter(':cust_name', trim(filter_input(INPUT_POST, 'name', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 187 | ->setParameter(':cust_street', trim(filter_input(INPUT_POST, 'street', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 188 | ->setParameter(':cust_zip', trim(filter_input(INPUT_POST, 'zip', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 189 | ->setParameter(':cust_town', trim(filter_input(INPUT_POST, 'town', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 190 | ->setParameter(':cust_phone', trim(filter_input(INPUT_POST, 'phone', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 191 | ->setParameter(':cust_cellphone', trim(filter_input(INPUT_POST, 'cellphone', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 192 | ->setParameter(':cust_fax', trim(filter_input(INPUT_POST, 'fax', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 193 | ->setParameter(':cust_country', trim(filter_input(INPUT_POST, 'country', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 194 | ->setParameter(':cust_group', trim(filter_input(INPUT_POST, 'custgroup', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW))) |
||
| 195 | ->setParameter(':cust_emailverified', (filter_input(INPUT_POST, 'emailverified') === 'y') ? 'y' : 'n') |
||
| 196 | ->setParameter(':cust_active', (filter_input(INPUT_POST, 'active') === 'y') ? 'y' : 'n') |
||
| 197 | ->setParameter(':cust_id', $iId) |
||
| 198 | ; |
||
| 199 | |||
| 200 | if (!empty(filter_input(INPUT_POST, 'pwd'))) { |
||
| 201 | $querybuilder |
||
| 202 | ->set('cust_password', ':cust_password') |
||
| 203 | ->setParameter(':cust_password', password_hash(filter_input(INPUT_POST, 'pwd'), PASSWORD_DEFAULT)) |
||
| 204 | ; |
||
| 205 | $aInfo['passwordchanged'] = true; |
||
| 206 | } |
||
| 207 | |||
| 208 | $querybuilder->execute(); |
||
| 209 | $aInfo['changeswritten'] = true; |
||
| 210 | } |
||
| 211 | } |
||
| 212 | } |
||
| 213 | $querybuilder = $this->dbal->createQueryBuilder(); |
||
| 214 | $querybuilder |
||
| 215 | ->select(DB_ADDRESSFIELDS) |
||
| 216 | ->from('customer') |
||
| 217 | ->where('cust_id = ?') |
||
| 218 | ->setParameter(0, $iId) |
||
| 219 | ; |
||
| 220 | $stmt = $querybuilder->execute(); |
||
| 221 | if ($stmt->rowCount() === 1) { |
||
| 222 | $aUser = $stmt->fetch(); |
||
| 223 | $aPData['customerform'] = $this->helperCustomer->buildCustomerForm($this->config->getLang(), 'admin', $aErr, $aUser); |
||
| 224 | } else { |
||
| 225 | $aInfo['nosuchuserfound'] = true; |
||
| 226 | } |
||
| 227 | } |
||
| 228 | $aPData['customeradmin']['text'] = $return; |
||
| 229 | $aPData['customeradmin']['type'] = $sType; |
||
| 230 | if (isset($aInfo)) { |
||
| 231 | $aPData['customeradmin']['info'] = $aInfo; |
||
| 232 | } |
||
| 233 | return $aPData; |
||
| 234 | } |
||
| 235 | |||
| 237 |
Loading history...
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.