PSR-2 standard for long if-conditions

Question

I did not find any standard for this case:

if ($a == $b && $b == $c && $c == $d && $g == $d) {

}

or

if (($a == $b && $b == $c) && ($c == $d && $g == $d)) {

}

Imagine the var-names are longer and 80 letters are exceeded. How should I handle this? It could look like:

if (
       $a == $b
    && $b == $c
    && $c == $d
    && $g == $d
) {

    }

Answer 1

Personally, I prefer

if ($a == $b
    && $b == $c
    && $c == $d
    && $g == $d
) {
    // code here...
}

For each line, you start with the double ampersand, indicating that the following statement is separate from the others. If you put the ampersand on the end of the line, it can become less obvious when the lines vary a lot in length.

For example;

if ($a == $b && 
    $b == $c && 
    $thisisamuchlongerstatementbecauseofthisvar == $d && 
    $g == $d
) {
    // code here...
}

In this case you have to scan the code more to know that each line is connected by a double ampersand.

Answer 2

There is no recommendation / convention for this case, and as Halcyon already mentioned this is a quite exceptional case.

However, there is a recommendation for a function call with a long list of parameters:

Argument lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument per line.

<?php
$foo->bar(
    $longArgument,
    $longerArgument,
    $muchLongerArgument
);

So if I had to create an if-statement similar to your's, I'd do this:

if (
    $a == $b &&
    $b == $c &&
    $c == $d &&
    $g == $d
) {
    // do something
}

As you can see, this is almost the same as the solution you proposed yourself, but I prefer adding the && operators after the conditions.

Answer 3

Worth mention that the new standard PSR-12, that replaces PSR-2, clarifies this question.

Expressions in parentheses MAY be split across multiple lines, where each subsequent line is indented at least once. When doing so, the first condition MUST be on the next line. The closing parenthesis and opening brace MUST be placed together on their own line with one space between them. Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.

<?php

if (
    $expr1
    && $expr2
) {
    // if body
} elseif (
    $expr3
    && $expr4
) {
    // elseif body
}

Source: https://www.php-fig.org/psr/psr-12/#51-if-elseif-else

Answer 4

Edit

One year later, I would strongly recommend to rewrite your code to have a shorter if statement. Through variables or function calls.

Original

I came into this situation, so I decided to go with the following format:

if (
    $a == $b &&
    $b == $c &&
    $c == $d &&
    $g == $d) {
}

But, I use phpcbf, which transformed (following the PSR2 standard) the previous code into:

if ($a == $b &&
    $b == $c &&
    $c == $d &&
    $g == $d) {
}

I wanted to know more: how does it know that this is the behaviour expected by the standard if it is not written anywhere? Well, the answer is simple: the case is taken into account by the standard, by the following sentence:

There MUST NOT be a space after the opening parenthesis

This explains why the second snippet is the one, and the only one, which follows the PSR-2 standard, as declared by php-fig.

Answer 5

I prefer putting logical operators in long if statements at the beginning of the line, mainly for readability and better behavior in version control.

Note that as also mentioned in the other answers it's usually a code smell to have long if statements. However sometimes you have to do it, or the code is already there and you can't rewrite it, so if it's already a bad thing then it helps not to make even more of a mess.

Also these things apply to if statements with just a single "and" where the different elements are so long you still need to split it to multiple lines (long variable or class names for instance).

if (
    $something->getValue() === 'some_value'
    || (
        $something instanceof SomeClass
        && $something->has($someNumber)
        && $someNumber > 42
    )
) {
    // do something
}

Readability: As all logical operators are vertically grouped you can instantly see which operator is on each line. As your eye scans the code it can just move straight vertically and it only needs to move horizontally when there is an actual extra logical level.

If the operators are at the end of the line your eye needs to move back and forth randomly between lines of uneven lenght.

Better behavior in version control: When an extra clause is added at the bottom of the if statement then this translates to 1 line added and 0 removed in version control.

diff --git a/3.php b/3.php
index 367c57c..2a40c3a 100644
--- a/3.php
+++ b/3.php
@@ -6,6 +6,7 @@ 
    if (
         $something instanceof SomeClass
         && $something->has($someNumber)
         && $someNumber > 42
+        && $anotherCase
    ) {
     // do something

If you put logical operators at the end then that will be 2 lines added and 1 removed. That in turn obscures useful information: your commit message for the last change will be shown for both lines when you Git annotate, so you would have to go to the previous version to see the commit message for the line you added the operator to.

diff --git a/4.php b/4.php
index f654780..2b9e0c5 100644
--- a/4.php
+++ b/4.php
@@ -5,7 +5,8 @@ 
    if (
        $something instanceof SomeClass &&
        $something->has($someNumber) &&
-       $someNumber > 42
+       $someNumber > 42 &&
+       $anotherCase
     ) {
     // do something

Answer 6

There is a recommendation nowadays... in PSR-12.

Expressions in parentheses MAY be split across multiple lines, where each subsequent line is indented at least once. When doing so, the first condition MUST be on the next line. The closing parenthesis and opening brace MUST be placed together on their own line with one space between them. Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.

<?php

if (
    $expr1
    && $expr2
) {
    // if body
} elseif (
    $expr3
    && $expr4
) {
    // elseif body
}

Answer 7

My favourite approach is to remove sub-expressions from the IF statement, as follows:

$c1 = $a == $b;
$c2 = $b == $c;
$c3 = $c == $d;
$c4 = $g == $d;
if ($c1 && $c2 && $c3 && $c4) {
}

This approach will make it also easier to debug.

The second case you expose is equivalent to the first one due to the associative property of the logic operators. Therefore, $a && $b && $c is the same as ($a && $b) && $c, which is the same as $a && ($b && $c)

Answer 8

I would suggest you try to think about the operation in different terms. For example:

if (count(array_unique([$a, $b, $c, $d, $g])) == 1)

You will perhaps find that you can express the whole algorithm as more of an operation on a set, use an array instead of individual variables and use logical operations on the set like shown above. That can lead to drastically different and more readable code.

Another example of refactoring:

namespace My;

UnexpectedValueException::assertAllEqual($a, $b, $c, $d, $g);


class UnexpectedValueException extends \UnexpectedValueException {

    public static function assertAllEqual(/* $value, ... */) {
        $args = func_get_args();
        if (count(array_unique($args)) > 1) {
            throw new static(sprintf('[%s] are not all equal', join(', ', $args)));
        }
    }

}

Answer 9

I prefer it at the beginning as well:

if (   self::LOG_ALL
    || (    self::DEBUG__EXECUTION_TIME__IS_ENABLED
        && (self::DEBUG__EXECUTION_TIME__THRESHOLD_SECONDS < $trxDurinationSeconds)
       )
) {
    doSomething();
}

Answer 10

I prefer to do it in this style:

if (condition1
|| (condition2_1 
    && condition2_2
    && condition2_3)
&& (c3 && c4) {
    // do something
}

But again, keep your if's as simple as possible.

Maybe it's a better idea to separate a big condition in multiple if's.

For your question, I would create a function which takes an array and returns true if all && are met. Then, in my main code you'd have like

$arr = [$a => $b, $b => $c, $c => $d];
// or you can create array of arrays [[$a, $b], [$b, $c] ...]

if (allTrue($arr))
    // do something