Phpinspectionsea: Non-optimal if conditions: False positive if PHP is used for templating

Created on 9 Mar 2017  路  4Comments  路  Source: kalessil/phpinspectionsea

<?php if ($a !== null || $b !== null): ?>
    <p>
        <?php if ($a !== null): ?>
            <?=$this->e($a)?>
        <?php endif ?>
        <?php if ($b !== null): ?>
            <?=$this->e($b)?>
        <?php endif ?>
    </p>
<?php endif ?>
question

Most helpful comment

Thanks for the quick answer, I think using variables like you suggested will solve the problem for me.

All 4 comments

Hi @MazeChaZer,

Independently from plain php/templating I can not confirm false-positives here: checks are clearly the same. I don't see much options how to fix this case (ideally the logic should be moved out from the template) and would suggest doing something like this:

<?php
    $violation1Reported = $a !== null;
    $violation2Reported = $b !== null;
?>
<?php if ($violation1Reported || $violation2Reported): ?>
...

or with PHP7

<?php [$violation1Reported, $violation2Reported] = [$a !== null, $b !== null]; /* slower */ ?>
<?php if ($violation1Reported || $violation2Reported): ?>
...

EDIT: the inspection will not complain in this way.

It's really a non-optimal if condition: you are double calculating an expression:

  • if ($a !== null || $b !== null): on first line;
  • if ($a !== null): and if ($b !== null): on 3rd and 6th line, respectively;

As you need to uses the same expression result twice, you should store it on a variable, like @kalessil shows - _note for him: array assignments is 20% lower than direct variable assignment, and is few readable too_. 馃槃

But exists where it really throw a false-positive:

if (mt_rand() > 0.5) {
    if (mt_rand() > 0.5) {
    }
}

Basically where you depends of a method, you can't know if it will generate differents results if compared with first case (in case of mt_rand() that I am using here as an example, it is very possible). Anyway, is very hard that EA does this check if there no way to know (currently) if this method is _side-effect_, _rand-effect_ or _user-dependent-effect_, for instance.

In all cases, I suggests you do a workaround (that not is ugly, and I guess that is inevitable here):

if (mt_rand() > 0.5) {
    $secondRand = mt_rand();
    if ($secondRand > 0.5) {
    }
}

Thanks for the quick answer, I think using variables like you suggested will solve the problem for me.

Perfect! Closing the issue.

Was this page helpful?
0 / 5 - 0 ratings