Beefy Boxes and Bandwidth Generously Provided by pair Networks
P is for Practical

Being impervious to an 'each' reset

by throop (Chaplain)
on Dec 13, 2006 at 23:29 UTC ( [id://589696]=perlquestion: print w/replies, xml ) Need Help??

throop has asked for the wisdom of the Perl Monks concerning the following question:


I was iterating over a hash:

while (my ($key,$val) = each(%some::very::long::named::module::foo)){ if(testconditions($key)){ some::very::long::named::module::drawSomethingPretty($val)}}
%foo has 56 keys, but the loop starts over again after the 3rd iteration. OK, code somewhere down in drawSomethingPretty must be performing a keys %foo and resetting my iteration. I started to rewrite as
my($val); foreach my $key (keys %some::very::long::named::module::foo){ $val = $some::very::long::named::module::foo{$key}; if(testconditions($key)){ some::very::long::named::module::drawSomethingPretty($val)}}
but I didn't like how it looked. Is there a succinct yet readable way still use each, but not be vulnerable to being reset?


Replies are listed 'Best First'.
Re: Being impervious to an 'each' reset
by Util (Priest) on Dec 14, 2006 at 06:15 UTC

    each() is vulnerable. Like it or not, your re-written version is the standard idiom for looping through a hash's key-value pairs when *anything* outside your control could interfere with an each() call.

    foreach my $key ( keys %hash ) { my $val = $hash{$key}; ... }
    For related info, see the thread Spot the bug, especially the replies by merlyn and ysth.

    To improve readability with your long package names, you might do some combination of these refactorings:

    • Move the scope of $val back inside the loop.
    • Align the long parts of the first two lines:
      foreach my $key ( keys %some::very::long::named::module::foo ) { my $val = $some::very::long::named::module::foo{$key};
    • Change the if block to next, or pre-filter the keys through grep().
    • Use scoped aliases via globs:
      { local *foo = *some::very::long::named::module::foo; local *draw = *some::very::long::named::module::drawSomethingPretty; foreach my $key ( grep { testconditions($_) } keys %foo ) { drawSomethingPretty( $foo{$key} ); } }
    • Import the identifiers. If such importing is not supported by the module, then submit a request or a patch to the module author.
      use some::very::long::named::module qw( foo drawSomethingPretty ); foreach my $key ( keys %foo ) { next if not testconditions($key); drawSomethingPretty( $foo{$key} ); }
      This is the most "correct" way to handle the issue; it is exactly this issue that Exporter is designed to resolve. Work with the author to nail down this part of the module's API. If direct access to %foo is truely part of the API, then it should definitely be available for import. Otherwise, some accessor method should be provided, or the module docs should make clear a proper path to meet the same needs.

Re: Being impervious to an 'each' reset
by SheridanCat (Pilgrim) on Dec 13, 2006 at 23:47 UTC
    You could assign %some::...::module::food to another hash and iterate over that, in which case you'll be iterating over an entirely different hash.
    use Data::Dumper; my %hash1 = ( key1 => 'hello', key2 => 'world' ); my %hash2 = %hash1; print Dumper \%hash1; print Dumper \%hash2;
    However, the code you posted makes me uncomfortable. You seem to be tinkering with some guts that shouldn't be tinkered with.
      You could avoid copying the large hash by using a reference:
      my $thingy_ref = \%Foo::Bar::Baz::thingy; foreach my $key ( keys %$thingy_ref ){ print "$key => $thingy_ref->{ $key }\n"; }

        Erm, then you'd have a reference to the same hash you're trying to avoid resetting the each iterator and the OP would be back to square one. The iterator is a property of the underlying hash, not how it's accessed.

Re: Being impervious to an 'each' reset
by ikegami (Patriarch) on Dec 15, 2006 at 00:35 UTC

    You could make your own iterator.

    sub gen_each_iter(\%) { my ($hash) = @_; my @keys = keys(%$hash); return sub { return if not @keys; my $key = shift(@keys); return wantarray ? ($key, $hash->{$key}) : $key; }; }; my $each = gen_each_iter(%foo); while (my ($key, $val) = $each->()) { ... }

    If you don't want a general case solution, the above simplifies to

    my @keys = keys(%foo); my $each = sub { return if not @keys; my $key = shift(@keys); return ($key, $foo{$key}); }; while (my ($key, $val) = $each->()) { ... }
Re: Being impervious to an 'each' reset
by Limbic~Region (Chancellor) on Dec 14, 2006 at 13:32 UTC
Re: Being impervious to an 'each' reset
by webfiend (Vicar) on Dec 15, 2006 at 00:04 UTC

    I would try to copy or alias the ugly names, if export wasn't an option.

    # Cruel module author doesn't allow exporting these! my $foo = \%some::very::long::named::module::foo; my $drawSomethingPretty = \&some::very::long::named::module::drawSomethingPretty; foreach my $key ( keys %{ $foo } ) { # It doesn't look like you need $val out here, so why not # just stick in the inner block? if ( testconditions($key) ) { my $val = $foo->{$key}; $drawSomethingPretty->($val); } }

    But if these names can't be exported, there are a couple of equally possible likelihoods:

    • The module author needs a gentle nudge to allow export of %foo and &drawSomethingPretty;
    • The module author had no intention of letting people get at %foo or &drawSomethingPretty and you are poking into the guts of the module rather than the defined interface. Nothing wrong with poking at guts occasionally, but it could be an explanation of why things aren't working right.

Log In?

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://589696]
Approved by Paladin
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others chanting in the Monastery: (4)
As of 2024-04-14 09:04 GMT
Find Nodes?
    Voting Booth?

    No recent polls found