Hi,
A week or two ago, a colleague asked me about a strange behaviour he
experienced, with seemingly-duplicate data within a foreach block. Upon
further examination, we determined that this was caused by assigning to
a referenced variable:
<?php
$i = array('zero','one','two');
foreach ($i AS &$j) {}
foreach ($i AS $j) {
echo $j . " ";
}
?>
(Output is: "zero one one")
I asked around about this, and was given the technical explanation (see
http://sean.caedmon.net/notsoweird.phps if you're curious), which
cleared it up.
However, upon further discussion, a number of us agreed that it would be
pertinent to raise an E_NOTICE when foreach assigns to a pre-existing
reference target. Certainly, the behaviour demonstrated above is
non-obvious, and contrary to the normal "PHP way" (simplicity).
Developers, at least in my opinion, don't expect foreach to WRITE to the
iterated array.
Technical explanation aside (yes, we realize that this is "normal"
behaviour, as demonstrated in the unroll), I believe this represents a
usability problem.
Can we throw an E_NOTICE when foreach targets a reference? (The other
option we kicked around was implicitly unset()ing the target variable
before the first iteration, but this was deemed "too magical".)
If so, when? HEAD only? I'd like to see it in 5.2 as well, if that ever
exists, but I agree that it's too late for 5.1.
S
try:
<?php
$i = array('zero','one','two');
foreach ($i AS &$j) {}
unset($j) // <-------- this
foreach ($i AS $j) {
echo $j . " ";
}
?>
it's known behavior. may app rely on this.
However, upon further discussion, a number of us agreed that it would be
pertinent to raise anE_NOTICEwhen foreach assigns to a pre-existing
reference target. Certainly, the behaviour demonstrated above is
non-obvious, and contrary to the normal "PHP way" (simplicity).
Developers, at least in my opinion, don't expect foreach to WRITE to the
iterated array.Technical explanation aside (yes, we realize that this is "normal"
behaviour, as demonstrated in the unroll), I believe this represents a
usability problem.Can we throw an
E_NOTICEwhen foreach targets a reference? (The other
there's many way to use reference, not only in foreach. don't do
reference unless u know it's really needed.
option we kicked around was implicitly unset()ing the target variable
before the first iteration, but this was deemed "too magical".)If so, when? HEAD only? I'd like to see it in 5.2 as well, if that ever
exists, but I agree that it's too late for 5.1.
due to php's auto gc, no one unset variable after using it.
but one of my "good design rule" is:
always unset the referenced variable immediately after it's used,
this helps a lot, and is almost important as escape the string before
sql query. no?S
Can we throw an
E_NOTICEwhen foreach targets a reference? (The other
there's many way to use reference, not only in foreach. don't do
reference unless u know it's really needed.
Sean's not so much referring to his own problem as (like you said) the
solution is a fairly simple matter of following strict coding practices.
He's trying to address the problem of unexperienced developers seeing this
behavior and (wrongly) reporting it as a bug, or worse, assuming PHP is just
"broken" and therefore shouldn't be used.
That said, I'm not convinved this needs a notice, but I'm certainly not
against it. Perhaps we should split the difference with an E_STRICT.
-Sara
Sean's not so much referring to his own problem as (like you said) the
solution is a fairly simple matter of following strict coding practices.
yeah, i see.
but...
when foreach assigns to a pre-existing
reference target
it's not the problem of the second foreach, any usage of $j after the
1st foreach as &$j will hurt
<?php
$i = array('zero','one','two');
foreach ($i as &$j) { do something here }
for ($j = 0; $j < 10; $j ++) {
...
}
$i[2] is modified
?>
iirc, there was a discussion about foreach behavior(leave assigned
key/value variable not unset after foreach) last year already.
i'd like to see it solved too, but with a real fix.
it's not the problem of the second foreach, any usage of $j after the
1st foreach as &$j will hurt
Yes. I thought it was clear that I understand this. I guess not.
My point is that foreach is doing something that isn't immediately
obvious. The same is true of your for loop, but to a lesser extent, IMO
(as I don't expect your for loop to ONLY read from $i).
I don't want to start a discussion on references. I'm just trying to
clear up a non-obvious case.
S
Sean Coates wrote:
it's not the problem of the second foreach, any usage of $j after the
1st foreach as &$j will hurtYes. I thought it was clear that I understand this. I guess not.
My point is that foreach is doing something that isn't immediately
obvious. The same is true of your for loop, but to a lesser extent, IMO
(as I don't expect your for loop to ONLY read from $i).I don't want to start a discussion on references. I'm just trying to
clear up a non-obvious case.
How would you supress the notice (I know error suppression is ugly), but
we need to make it possible for people to quickly adapt to this change
if they indeed relied on this.
would this work:
foreach ($i as @&$j) {}
regards,
Lukas
Can we throw an
E_NOTICEwhen foreach targets a reference? (The other
there's many way to use reference, not only in foreach. don't do
reference unless u know it's really needed.Sean's not so much referring to his own problem as (like you said) the
solution is a fairly simple matter of following strict coding practices. He's
trying to address the problem of unexperienced developers seeing this behavior
and (wrongly) reporting it as a bug, or worse, assuming PHP is just "broken"
and therefore shouldn't be used.That said, I'm not convinved this needs a notice, but I'm certainly not
against it. Perhaps we should split the difference with an E_STRICT.
It has to be an E_STRICT because it's something related to language
and possible semantic errors. E_NOTICE is not for that. However, I don't
think that we should add this, as it's perfectly normal and expected
behavior (albeit a bit weird). And indeed, some people might rely on
this "feature".
Derick
It has to be an
E_STRICTbecause it's something related to language
and possible semantic errors.E_NOTICEis not for that.
Isn't it the same as the following?
sean@iconoclast:~$ php5 -d"error_reporting=2047" -r '$a=$b;'
Notice: Undefined variable: b in Command line code on line 1
According to the manual:
E_NOTICE == Run-time notices. Indicate that the script encountered
something that could indicate an error, but could also happen in the
normal course of running a script.
S
Being the colleague Sean refered to in his first post I thought I
might weigh in.
While I agree that once I looked at the base case that Sean worked out
of my code the problem didn't take too long to recognize, that's not
where I first experianced the problem. Problems first rear their head
deep within your code, looking at it there I had no idea what was
going on, all I could tell was that for some reason I was getting two
copies of an object in a foreach loop (I hadn't yet noticed that I was
missing one).
Once Sean was able to break my code down into the simple base case and
replicate the issue I understood what was happening, but just looking
at my code as a whole? Not a chance.
A warning on strict or or notice would have helped a lot, and that's
really what I think a lot of those messages are, notices to the
programmer that something odd /may/ happen.
as a side note, running the zend code analyzer did not give me any
warnings either.
paul