Setting a variable during an if loop [closed]

Question

I've seen the pattern:

<?php
  if ($node = node_load(30) && $node->type == 'bogus') {
    print $node->title . 'is bogus';
  }

But when I tried to use it I got a warning: variable not defined... Has this pattern of declaring variables while you loop in an if been deprecated?

Answer 1

I have created the following function and class to test your code with:-

function node_load($load)
{
    return new node();
}

class node
{
    public $type = 'bogus';
    public $title = 'Node title ';
}

Let's take a look at your code step by step, so we can work out what is wrong with it.

First let's see what happens if we var_dump the output of node_load():-

$node = node_load(1);
var_dump($node);

Output:-

object(node)[35]
  public 'type' => string 'bogus' (length=5)
  public 'title' => string 'Node title ' (length=11)

That looks about right, so let's see what happens if we put it in an if statement.

if($node = node_load(1)){
        var_dump($node);
    }

Output:-

object(node)[36]
  public 'type' => string 'bogus' (length=5)
  public 'title' => string 'Node title ' (length=11)

There doesn't seem to be any problem there either. So, lets try the compound if statement you have:-

if($node = node_load(1) && $node->type = 'bogus' ){
    var_dump($node);
}

Output:-

boolean true

Hmm, what's happened here? Well if you take a look at how logical operators work, you'll see that the result of a logical statement like node_load(1) && $node->type = 'bogus' is a boolean true or false, which you've just assigned to the variable $node. You've lost the object that was previously referenced by that variable. Hence when you try to do:-

print $node->title . 'is bogus';

you will get a "Warning: Attempt to assign property of non-object " as $node is now a boolean and not the object you were expecting.

That seems to me to be a pretty good reason for not assigning variable in an if statement as you are doing. Assign before the if statement and then test:-

$node = node_load(30);
if ('bogus' === $node->type) {
    print $node->title . 'is bogus';
}

Output:-

Node title is bogus

Which is what is wanted. I'm sure you'll agree the re-factored code is much easier to read and as an added bonus, it works!

You may notice that I have reversed the normal order of the comparison, using 'bogus' === $node->type rather than $node->type === 'bogus'. This will raise an exception if you type '=' instead of '==' or '===' by mistake and make debugging much easier. It is a good habit to get into, as is using strict type comparisons with '===' rather than loose comparisons with '=='.

I might add that doing little step by step tests like this is a good way of working out what code actually does and reinforces the concepts of programming in your mind. I always do it when I come across functions/concepts that are new to me.

Answer 2

I could only wish... No, it has not been deprecated, but it should be. I seriously suggest that you don't use this syntax. It is too easy to accidentally assign a variable in a statement, which can result in similar errors. If you make a habit of avoiding this syntax it will make debugging that much easier. Say you get a similar error to this and while debugging you spot an assignment in one of your statements, then you will be able to stop debugging right there, because more than likely you have just found your error. Not just this, but it will make reading your code that much easier. Say this assignment is nested deep in a tree of statements. It can quickly become difficult to track those assignments.

The reason you are probably getting an error is because if the first variable proves to be FALSE, the second will not get assigned due to the && shortcircuiting and breaking from the statement early. This would result in $node->type not getting set.