6 unicity traps that sideline PHP code

Sometimes, code is being ignored by PHP : code is painstakingly written but never gets executed. How is that even possible? May be, because it was not so painstakingly coded.

Indeed, there are some PHP coding structures which natively and silently overwrites or ignore code. This leads to dead code. It stays in the code base, and never gets removed. So, to help trim source code of those dead branches, here are no less than six places to check, and some static analysis to help spot them.

Keys in arrays

The keys of an array are unique; the values are not. Keys cannot be repeated, as they identify one entry in the array. One classic mistake is the repeat of a key. It may also be like here, when there is some similarity between the display of the keys.

<?php
const MY_CONST = 'd';

$array = [
    'a' => 1,
    'b' => 3,
    'c' => 123,
    'd' => 45.33,
    'c' => 'z',
    'f' => new stdClass,
    MY_CONST => 0,
];

?>

Also, be aware that constants hide those repeated values : it is the value of the constant that is used for the key, not its name. Hence, MY_CONST is actually a double of ‘d’.

Moreover, indices in arrays are typed strings or integers. With other data types, some type juggling happen, which lead to extra collisions. Finally, strings that contain integers will be turned into integers, leading to some really special cases :

<?php

$a['1'] = 1;
$a[1] = 1;
$a['1a'] = 1;

print_r($a);

?>

In the case of arrays, the last value is the one that stays : in reality, the last written value overwrites the previous ones.

Exakat’s rule : Multiple Index Definition

Cases in switch() or match()

Very similar to the array problem, switch() and match() case’s happily accept redundant clauses, and only use the first one. It is the first found, so this is the one used.

Interestingly, the default case is immune to this behavior, and may very well be placed anywhere in the switch. It is also the only clause that will be detected as a double by PHP linter.

<?php

switch ($a) {
    default: echo 20; break 1;
    case 1: echo 1; break 1;
    case 2: echo 2; break 1;
    case 1: echo 3; break 1;
    case ONE: echo 4; break 1;
}
?>

Just like for arrays, constants may break the readability of the clauses’s conditions.

Exakat’s rule : Multiples Identical Case

If / elseif / then

If / elseif / then structures do not lend itself to much to repetition, until there are a lot of them. Then, the list of conditions may stretch past the bottom of the screen, and become invisible. All, in all, it is a problem very similar to the one with switch().

<?php

    if ($object->property == 1) { /* doSomething with 1 */ }
elseif ($object->property == 2) { /* doSomething with 2 */ }
elseif ($object->property == 3) { /* doSomething with 3 */ }
elseif ($object->property == 2) { /* doSomething with 4 */ }

?>

As for switch(), the first valid condition is used, and all the next ones are omitted.

Exakat’s rule : Multiples Identical Case

implements too much the same interface

implements is an option for classes, to indicate which interfaces are implemented. There is no need to repeat it, and PHP checks that the same interface is not implemented twice at execution time. It reports a Fatal error when it finds one.

Though, there is a dead angle here : the same interface may be implemented also in children classes, even if the parent class has already done the job.

<?php

interface i {} 

class A implements i {}

class B extends A implements i {}

?>

This one is silently ignored by PHP. This is merely a repeat, and since all the work is done in the parent class, it is already structurally sound. The side effects may be very limited, besides writing clean code.

Exakat’s rule : Already Parents Interface

use too much the same trait

use includes a trait in a PHP’s class or another trait. Contrary to interfaces, PHP doesn’t check if the trait is already used, even when there are multiple calls to it. The first one is added, and all the remaining ones are ignored.

The same happens with children classes : when, again, the same trait is included, it is just ignored.

<?php

trait t {} 

class A { use t, t, t; }

class B extends A implements i { use t, t;}

?>

The discrepancy of behavior between interfaces and traits is actually somewhat intriguing.

Exakat’s rule : Already Parents Interface

catch clauses

Catch clauses are added after a try block to intercept exceptions, depending on their class type. This structure also accept multiple times the same clause, and only process the first one, in coding order.

<?php

use A as C; 

try {
    // something that throws an exception
} 
catch (A $a) {  print "Exception A\n"; }
catch (B $b) {  print "Exception B\n"; }
catch (C $c) {  print "Exception C\n"; }
catch (A $d) {  print "Exception D\n"; }

?>

Like for switch, some aliasing may be messing with the readability of the clauses : here, C is an alias of A, which leads to a double clause.

PHP will not check double clauses : it will simply process them in order until one can process it. In fact, PHP also doesn’t check if the caught exception exists or not. PHP only matches the current exception with the requested one. This is dead code, all by itself.

This structure has a second trap : inheritance. The clause may be using a parent of the class. That way, the name of the exception is not used, but its parent is. This may also lead to a dead code among the catch clauses.

<?php

try {

} 
catch (Exception $e) {  print "Exception Generic\n"; }

// If A extends Exception, this is now dead code
catch (A $a) {  print "Exception A\n"; }

?>

Exakat’s rule : Multiple Exceptions Catch()

Conclusion

Checking for double use of an element in the syntax is a classic trap in coding. We listed them for PHP here, though every other language has the same problem. One of the thorny issue is to compare values that may actually be very different. In the case of catch clauses, the same exception may have a fully qualified name, a relative name, an alias, a parent or even a class_alias() class. The last ones are only available at execution time, hence, the late validation.

Most of the time, those structures are statically coded, and they are caught by static analysis tools. All the situations above have a specific rule in Exakat’s engine to spot them.

And indeed, the impact of dead code is a larger burden of maintenance, and should only be dealt with at code review.