|
Re: while(<>) { ... } considered harmful
by dws (Chancellor) on Sep 07, 2002 at 23:37 UTC
|
Since
map { substr($_, 1, 1) = "foo" } qw(bar baz);
and
grep { substr($_, 1, 1) = "foo" } qw(bar baz);
both exhibit the problem you've observed, perhaps the advice might better be phrased
Don't write to $_ from within map { } or grep { }
local $_; is one way to avoid writing to it, since the 'it' is now different.
Update: Hm.. It's a bit worse than that.
foreach ( qw(foo) ) { $_ = 1 }
also exhibits the problem.
| [reply]
[d/l]
[select] |
Re: while(<>) { ... } considered harmful
by shotgunefx (Parson) on Sep 07, 2002 at 23:30 UTC
|
Any module that uses $_ should localize it or any other globals they futz around with IMHO.
-Lee
"To be civilized is to deny one's nature." | [reply] |
|
|
| [reply] |
Re: while(<>) { ... } considered harmful
by Aristotle (Chancellor) on Sep 07, 2002 at 23:42 UTC
|
| [reply] |
Re: while(<>) { ... } considered harmful
by BrowserUk (Patriarch) on Sep 08, 2002 at 00:36 UTC
|
'scuse my ignorance of nether regions below Perl's skirts, but would it impose a huge performance penalty for map and grep to localise $_ on entry?
I visualise them as subs with their own local scope. Would a local $_; at the top cost so very much?
Well It's better than the Abottoire, but Yorkshire!
| [reply]
[d/l] |
|
|
Unless I totally misunderstood the above comments -- which is possible as I just briefly skimmed them -- the problem here is not that map and grep don't localize $_, but that they may execute code that may alter the value of $_, which in this case is aliased to a constant. Consider this code:
$_ = 'Ovid';
%hash = map { $_ => 1 } qw/ foo bar baz /;
print;
That code will print 'Ovid' because $_ has been localized within the map statement. Now, if you try to alter the $_, your code fails because you're trying to alter a constant:
$_ = 'Ovid';
%hash = map {
$_ .= 'asdf';
$_ => 1
} qw/ foo bar baz /;
print;
To get around that, you can do this:
$_ = 'Ovid';
%hash = map {
local $_ = $_;
$_ .= 'asdf';
$_ => 1
} qw/ foo bar baz /;
print;
That allows you to assign the aliased constant to a real variable, thus avoiding the problems outlined in this thread.
Cheers,
Ovid
Join the Perlmonks Setiathome Group or just click on the the link and check out our stats. | [reply]
[d/l]
[select] |
Re: while(<>) { ... } considered harmful
by converter (Priest) on Sep 08, 2002 at 17:00 UTC
|
The perlsub man page mentions localization of $_ in the "When to Still Use local()" section, and says:
In particular, it's important to "local"ize $_ in any routine that assigns to it. Look out for implicit assignments in "while" conditionals.
With all due respect, both you and the authors of the modules you mention are ignoring the sound advice offered in the Perl documentation and should consider making the appropriate changes to make your code "play well with others".
| [reply] |
|
|
IMHO it just too easy to do such mistake. Most of Perl operators that automagically assign to $_ (for/foreach, map, grep) do localize it so it doesn't affect outer scope. On the other hand magic in while(<>) construct doesn't work same way while newbie to Perl can expect Perl to do it. Frankly I've never though about such side effect of this construct before and I think I can say that I'm seasoned Perl programmer.
Anyway what I did wrong? I just have simple map construct (I've posted snipplet of real code in other reply to this thread) and that error was very confusing. Most of the time Perl does right thing with scopes and badly written external to my code module cannot affect my code so destructively. Usually I can treat them as black boxes.
Now instead of just writting use Net::DNS in my modules I will have to patch them to have something like { local $_; require Net::DNS } (until Net::DNS is fixed).
--
Ilya Martynov
(http://martynov.org/)
| [reply]
[d/l]
[select] |
Re: while(<>) { ... } considered harmful
by Juerd (Abbot) on Sep 08, 2002 at 00:55 UTC
|
map { require Net::DNS } 1;
Is that a map in void context? Nasty and evil. (I don't recall require ever returning anything useful.)
Personally I prefer always using while(my $line = <>) construct.
I don't always. Setting $_ lets one use regexes and simple commands easier. I do use a lexical for more complicated blocks, but I often find myself just slurping the file into an array first.
- Yes, I reinvent wheels.
- Spam: Visit eurotraQ.
| [reply] |
|
|
map { require Net::DNS } 1;
Is that a map in void context? Nasty and evil.
I believe that's a minimal demonstration of a bug,
rather than "real" code.
On the subject of while(<>) vs.
while(my $line = <>), I tend to prefer the
former:
- If I don't have to do much processing for each line,
it's usually much nicer to work on $_, as
Juerd points out.
- If I do have a lot of work to do, it's usually
in a subroutine call, where this sort of thing isn't an
issue (since the scope has changed).
I haven't really thought about it, but if a loop's so
complex that you have to assign to an explicit iterator
variable just to figure out what's going on (as opposed to
a loop where defaulting to $_ would be
inappropriate for other reasons), it's probably an
indication that the loop needs simplifying.
--
F
o
x
t
r
o
t
U
n
i
f
o
r
m
Found a typo in this node? /msg me
The hell with paco, vote for Erudil!
| [reply]
[d/l]
[select] |
|
|
my @other_connectors = map $ad_obj->connector(field => $_),
qw(header description region country state);
No require on the surface. It was deeply hidden in one of method calls. I though it was just a Perl bug when I got that error for first time.
--
Ilya Martynov
(http://martynov.org/)
| [reply]
[d/l] |
|
|
require returns the last evaluated expression in the file. Normally this is 1, but I've seen people do wacky things like have "return %hash" as the last expression and do "%hash = require Foo::Bar";
| [reply] |
|
|
require returns the last evaluated expression in the file
Only the first time:
> perl -de 0
...
DB<1> x require 'con'
'this is true';
^Z
0 'this is true'
DB<2> x require 'con'
0 1
DB<3> x require 'con'
0 1
DB<4>
I've seen people do wacky things like have "return %hash" as the last expression and do "%hash = require Foo::Bar";
So such code will break as soon as more than one use of the module is attempted from a single run. That's a bit of a shame too, since Perl could really use something along those lines.
- tye | [reply]
[d/l] |
|
|
Since my post, I have used eval { require ... } ? ...->import : *symbol = sub { dummy here } a lot.
Juerd
# { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }
| [reply]
[d/l] |
Re: while(<>) { ... } considered harmful
by Anonymous Monk on Sep 08, 2002 at 04:14 UTC
|
I probably wrong but I'd say map is considered harmful.
I use while(<>){...} all the time but never use map.
Would I run into this problem if I avoid using
map? I guess I just don't understand map.
Why use map? | [reply] |
|
|
Because something like
my @idx = map /(id\d+)/, @items;
is a lot more natural than
my @idx;
/(id\d+)/ && push @idx, $1 for @items;
(which is already pushing legibility). Basically when you build one list out of another, non-destructively, map is the ticket. It can be in other cases as well, but those can often go either way.
Makeshifts last the longest. | [reply]
[d/l]
[select] |
|
|
| [reply]
[d/l] |