why are assignments in conditions bad ?

Question

I am using NetBeans for PHP 6.5.

In my code I frequently use the following type of command:

if (($row = $db->get_row($sql))) {
        return $row->folder;
    } else {
        return FALSE;
    }

Netbeans tells me that I should not be using assignments in the IF statement.

Why ?

Answer 1

It's probably trying to help you avoid the dreaded typo:

if(a = b)
   //logic error

Although I would expect an enviroment smart enough to warn you about that, to also be smart enough to have "oh, don't worry about that case" conditions.

Answer 2

They are not bad, but they can lead to dangerous mistakes.

In c like languages, where an assignment is an expression, (to support for example a=b=c=1;) a common error is:

if (a = 1) { .. }

But you wanted to have

if (a == 1) { .. }

Some developers have learned to type

if (1 == a) { .. }

To create an error if one '=' is forgotten. But I think that it does not improve the readability.

However modern compilers, give a warning if you write

if (a = 1) { .. }

which I think is a better solution. In that case you are forced to check if it was what you really meant.

Answer 3

I use them all the time, with loops (not sure why that would make a difference), like:

$counter = 0;
while( $getWhateverDataObj = mysql_fetch_object( $sqlResult )) {
   $getWhateverObj->firstName[$counter] = $getWhateverDataObj->firstName;
   $getWhateverObj->lastName[$counter]  = $getWhateverDataObj->lastName;
   $counter++;
}

And it works fine.

Answer 4

In languages that allways return a value on assignments it's not bad (I think it's quite common in functional languages), but (as others allready have said while I typed this) it should usually be avoided since you or someone else might mistake it for a comparison. The compiler should usually warn about it, but it can be ignored if you're sure what you're doing...

Answer 5

conditionals often include short circuit operators. so, given this example:

if ( a=func(x) && b=func(y) ) { // do this }

it may not be immediately obvious, but the second assignment would only occur if the first returned >0, and if func(y) had other side effects that you were expecting, they would not happen either.

in short, if you know what you are doing and understand the side effects, then there is nothing wrong with it. however you musts consider the possibility that someone else may be maintaining your code when you're gone and they might not be as experienced as you.

also, future maintainers may think you intended the following:

if ( a==func(x) && b==func(y) ) ...

if they "fix" your code, the actually break it.

Answer 6

On a totally unrelated note. (I think assigning stuff in a If causes readability problems as many here have statet).

For the given snipped above there is the ? operator in C languages that do exactly this. I guess it's there in PHP too.

http://davidhayden.com/blog/dave/archive/2006/07/05/NullCoalescingOperator.aspx

Answer 7

how would a code look like if you do not assign the $row value in the loop condition this would be much more complicated i think... although not that good to read for some maintainers, no? well you can do it like

$next = mysql_fetch_assoc($result)
do{
...
...
...

$next = mysql_fetch_assoc($result) or break;
}while ($next)