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:
| 1 | <?php |
||
| 11 | class UserController extends Controller { |
||
| 12 | |||
| 13 | public function indexAction() { |
||
| 25 | |||
| 26 | public function editAction(Request $request) { |
||
| 142 | |||
| 143 | |||
| 144 | View Code Duplication | public function deleteAction(Request $request) { |
|
| 168 | |||
| 169 | public function impersonateAction(Request $request) { |
||
| 183 | |||
| 184 | |||
| 185 | // public function twofactorAction(Request $request) { |
||
| 186 | // ################################################ SETTINGS ################################################ |
||
| 187 | // $name = 'User'; |
||
| 188 | // $repo = 'VivaitAuthBundle:User'; |
||
| 189 | // $formtpl['title'] = '2-Factor Authentication'; |
||
| 190 | // $key = $this->get('security.context')->getToken()->getUser(); |
||
| 191 | // ############################################################################################################ |
||
| 192 | // |
||
| 193 | // $obj = $this->getDoctrine() |
||
| 194 | // ->getRepository($repo) |
||
| 195 | // ->find($key); |
||
| 196 | // |
||
| 197 | // if(!$obj) { |
||
| 198 | // $this->get('session')->getFlashBag()->add('error', sprintf("Could not find the %s", $name)); |
||
| 199 | // } |
||
| 200 | // |
||
| 201 | // |
||
| 202 | // $form = $this->createFormBuilder(); |
||
| 203 | // if($obj->getTfkey()) { |
||
| 204 | // $formtpl['content'] = '2-Factor authentication has been enabled for this account, to disable it click the button below.'; |
||
| 205 | // $form->add('disable', 'submit', array('label' => 'Disable')); |
||
| 206 | // } else { |
||
| 207 | // $formtpl['content'] = '2-Factor authentication protects your account by making sure that to access it, you need your username, password and your token generator (typically a mobile device)'; |
||
| 208 | // $form->add('enable', 'submit', array('label' => 'Enable')); |
||
| 209 | // } |
||
| 210 | // $form = $form->getForm(); |
||
| 211 | // |
||
| 212 | // if($request->isMethod('POST')) { |
||
| 213 | // $form->bind($request); |
||
| 214 | // if($form->isValid()) { |
||
| 215 | // |
||
| 216 | // if($obj->getTfkey() && $form->get('disable')->isClicked()) { |
||
| 217 | // $obj->setTfkey(null); |
||
| 218 | // } else { |
||
| 219 | // |
||
| 220 | // $bytes = openssl_random_pseudo_bytes(10); |
||
| 221 | // $string = bin2hex($bytes); |
||
| 222 | // $this->get('session')->getFlashBag()->add('success', $string); |
||
| 223 | // $obj->setTfkey($bytes); |
||
| 224 | // } |
||
| 225 | // |
||
| 226 | // |
||
| 227 | // $em = $this->getDoctrine()->getManager(); |
||
| 228 | // $em->persist($obj); |
||
| 229 | // $em->flush(); |
||
| 230 | // $this->get('session')->getFlashBag()->add('success', sprintf('2-Factor authentication has been %s', $obj->getTfkey() ? 'enabled' : 'disabled')); |
||
| 231 | // return $this->render('VivaitBootstrapBundle:Default:redirect.html.twig', array('redirect' => $request->query->get('parent', $request->request->get('parent', $request->headers->get('referer'))))); |
||
| 232 | // } |
||
| 233 | // } |
||
| 234 | // |
||
| 235 | // if(isset($form)) { |
||
| 236 | // $formtpl['form'] = $form->createView(); |
||
| 237 | // } |
||
| 238 | // $formtpl['action'] = $this->generateUrl($this->container->get('request')->get('_route'), $request->query->all()); |
||
| 239 | // return $this->render('VivaitBootstrapBundle:Default:form.html.twig', array('form' => array_merge($formtpl, array('parent' => $request->query->get('parent', $request->request->get('parent', $request->headers->get('referer'))))))); |
||
| 240 | // |
||
| 241 | // |
||
| 242 | // } |
||
| 243 | // |
||
| 244 | // private static function base32_decode($b32) { |
||
| 245 | // $lut = array("A" => 0, "B" => 1, |
||
| 246 | // "C" => 2, "D" => 3, |
||
| 247 | // "E" => 4, "F" => 5, |
||
| 248 | // "G" => 6, "H" => 7, |
||
| 249 | // "I" => 8, "J" => 9, |
||
| 250 | // "K" => 10, "L" => 11, |
||
| 251 | // "M" => 12, "N" => 13, |
||
| 252 | // "O" => 14, "P" => 15, |
||
| 253 | // "Q" => 16, "R" => 17, |
||
| 254 | // "S" => 18, "T" => 19, |
||
| 255 | // "U" => 20, "V" => 21, |
||
| 256 | // "W" => 22, "X" => 23, |
||
| 257 | // "Y" => 24, "Z" => 25, |
||
| 258 | // "2" => 26, "3" => 27, |
||
| 259 | // "4" => 28, "5" => 29, |
||
| 260 | // "6" => 30, "7" => 31 |
||
| 261 | // ); |
||
| 262 | // |
||
| 263 | // $b32 = strtoupper($b32); |
||
| 264 | // $l = strlen($b32); |
||
| 265 | // $n = 0; |
||
| 266 | // $j = 0; |
||
| 267 | // $binary = ""; |
||
| 268 | // |
||
| 269 | // for($i = 0; $i < $l; $i++) { |
||
| 270 | // |
||
| 271 | // $n = $n << 5; |
||
| 272 | // $n = $n + $lut[$b32[$i]]; |
||
| 273 | // $j = $j + 5; |
||
| 274 | // |
||
| 275 | // if($j >= 8) { |
||
| 276 | // $j = $j - 8; |
||
| 277 | // $binary .= chr(($n & (0xFF << $j)) >> $j); |
||
| 278 | // } |
||
| 279 | // } |
||
| 280 | // |
||
| 281 | // return $binary; |
||
| 282 | // } |
||
| 283 | } |
||
| 284 |
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.