Assignments in conditions should be avoided

[code green]

I have just switched from phpdesigner to Netbeans as my script editor.
Netbeans is criticising my code with this warning

Possible accidental assignment, assignments in conditions should be avoided

What is your opinion of the warning when doing this for example?

 if($data = $file($filepath))

The alternative is

$data = $file($filepath);
if($data)

which seems less professional.

[Dheeraj Joshi]

@code green
I don't know why it complains but.....

My organization insist me to write code in this way.

$data = $file($filepath);
if($data)
 

If i club more statements in a condition like

if($data = $file($filepath))
 

i will get review comments.

Regards
Dheeraj Joshi

[Dheeraj Joshi]

I dont know about that problem, but my organization has some coding standards, as per that.

If i try to write code as

if($data = $file($filepath))
 

my managers gives review comments to this code saying it to change as

$data = $file($filepath);
if($data)
 

Regards
Dheeraj Joshi

Regards
Dheeraj Joshi

[code green]

I have had a Google and it seems the reasoning behind this is to avoid the schoolboy error of confusing

if($data = $file($filepath)) 

and

if($data == $file($filepath)) 

Are script editors now assuming the lowest common denomitor of programming skill?

[Dormilich]

don’t know about that… but admittedly, if someone other than you looks at the statement

if ($var = function($param))

(s)he might not be sure, whether it is intended or a mistake.

[Markus]

@code green
This is discouraged in most languages and mainly because of readability, I think. TBH, I could glance over that code and think that what I saw was a comparison check. To avoid this ambiguity, I'd do it like:

if (($data = somefunc()) != 0) { ...
 

[code green]

So it seems to be discouraged for maintainability.
Hmm, I have never made the comparator/assignment error, even when reading.
But I like to comply with best practice protocol and would hate future coders criticising my legacy code.
I quite like Markus' method.
Maybe I should listen to Netbeans and modify where required

[Markus]

I wonder how the following code is evaluated: if ($data = somefunc())?

Is the value of somefunc() assigned to $data and then $data's value is checked, or is the assignment checked (does the assignment fail)?

I'm going to ask on the PHP dev mailing list.

[Dormilich]

I don’t think an assignment can fail… it’s usually expressions that fail.

[code green]

I wonder how the following code is evaluated: if ($data = somefunc())

Logic tells me that the most inner bracket command is carried out first,
then working outwards.
So the return value of somefunc() is assigned to $data even if this value is FALSE.
Then the if() condition is evaluated.

If PHP was strictly typed then $data could not be a boolean or say an array.
So does loosely typed allow potential bugs to creep in

[Markus]

@Dormilich
An assignment is an expression?

[Markus]

You're correct, I think. The end value of the expression is used. You see, this is where the ambiguity comes in. If I write it as if (($data = somefunc())), then it makes perfect sense that the result from the innermost expression is used.

/doh.

[Dormilich]

@Markus
is my English that bad?

[Markus]

@Dormilich
No. My understanding is that an assignment is an expression.

Edit: I guess not.

[code green]

A note to Netbeans users, the warning can be disabled in
Tools>>Options then Hint tab. Select PHP for Language.
Uncheck the "Possible accidental assignment..." option under Stable.

This of course does not mean it is acceptable practice.

I checked the "Experimental" option under hints which revealed a lot more sins in my code.
Mainly unitialised variables, but I can live with those.

[Markus]

Here is a good reply that I got:

Assignment operations in PHP have the "side effect" of returning the
assignment. For example:

function return_false() {
return false;
}

var_dump(return_false()); //bool(false);
var_dump($a = return_false()); //bool(false);
var_dump($a = 1); // int(1)
var_dump($a = "hello world!"); //string...

So the same thing that allows you to do:

$a = $b = $c = $d = 154;

which works because "$d = 154" returns 154, which is assigned to $c,
which returns 154... is how assignment in conditionals or looping
works:
if($a = return_false()) { }
var_dump($a); //bool(false)

if($a = "hello") {}
var_dump($a); //string, "hello"

So what's really happening is the return value of the expression "$a =
____" is evaluated and that's used to determine the truth of the
conditionality. if($a = return_false()) is exactly the same thing as
if(return_false()) save for you "capture" the output of the function,
rather than just allow the conditional operator to see it. It's
functionally equivalent to $a = return_false(); if($a) {} but it's
important to understand that __assigning a variable to a value in PHP
is an expression with a return value___ and that return value is the
value that you assigned to the variable.

[code green]

As I come from a C++ background,
in my own functions if I wish to return a value and/or an error code,
I prefer to pass a pointer to the function and return the error code

$data = array();
if(getData($data)){....}
 
function getData(&$retData){
    $success = false;
    ...
   //load data into $retData
  //If successful then $success = true;
   ...
    return $success;
}  

But PHP inbuilt functions don't do it this way.
Very few even accept a pointer as a parameter.