Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl Monk, Perl Meditation
 
PerlMonks  

Re: foreach argument modification inside loop, followed by return from loop

by rjt (Deacon)
on Jul 10, 2013 at 10:31 UTC ( #1043438=note: print w/ replies, xml ) Need Help??


in reply to foreach argument modification inside loop, followed by return from loop

Such code works fine,

Maybe. Have you tested this with every significant code path in the Perl interpreter on multiple platforms? Do you even know the significant code paths? Can you guarantee it will remain stable with tied variables? XS subs? Refs? Resized arrays? There are a lot of permutations you would need to go through before I'd be convinced this array modification "works fine".

However I wondering is that considered a bad code

I think the documentation is pretty clear: "don't do that". Just because the implementation happens to (seem to) work for whatever case you've come up with, directly going against the documented limitations of a fundamental Perl loop keyword is just begging for trouble, either now, or (possibly with greater consequences), later on down the road when everyone has forgotten the stupid unnecessary risk you took, with a for loop or two buried in a large program.

Your code:

my @a = (1,2,3); for (@a) { if ($_ == 2) { pop @a; last; } } print @a;

What's wrong with print grep { $_ != 2 } @a;1, anyway? More efficient, and guaranteed not to get you fired from your job.

Sorry if this seems a bit doom and gloom, but I really just don't get what you hope to gain, here. Perhaps you could explain?

________
1. Well, if your goal is obfuscation, you got me. I just realized your code is actually along the lines of pop @a if grep { $_ == 3 } @a;. Cute, but I still don't see the point?


Comment on Re: foreach argument modification inside loop, followed by return from loop
Select or Download Code
Re^2: foreach argument modification inside loop, followed by return from loop
by vsespb (Hermit) on Jul 10, 2013 at 10:40 UTC
    Maybe. Have you tested this with every significant code path in the Perl interpreter on multiple platforms?
    that's what I actually ask in my question.
    What's wrong with print grep { $_ != 3 } @a;, anyway?
    that was just proof-of-concept code
      that was just proof-of-concept code

      Understood, and thanks for posting your real code farther down the thread. Based on that, the relevant logic you have is this:

      my $idx = 0; for my $j (@a) { if ($j == $target) { splice(@a, $idx, 1); last; } ++$idx; }

      Which helps clarify what you're trying to do: remove an element from an array by value. I'm still far from convinced that there is any benefit to a splice/last approach, even based solely on the grounds that it's slower and more complex than something like this:

      my $idx = firstidx { $_ == $target } @a; splice(@a, $idx, 1) if $idx >= 0;

      Timing for 1e6 entries:

      Rate for firstidx for 24.3/s -- -22% firstidx 31.1/s 28% --

      Certainly in this case, the for approach seems clearly suboptimal (List::MoreUtils' XS code does essentially the same thing as the for loop, much faster and more cleanly). Results were similar with any array sizes where efficiency is likely to matter.

        I'm still far from convinced that there is any benefit to a splice/last approach, even based solely on the grounds that it's slower and more complex than something like this:
        I agree. This can be optimized-out.
        Problem that I mentioned in my posting Re^2: foreach argument modification inside loop, followed by return from loop that "for" loop is in get_task(). i.e. there is another outer "for" loop.
Re^2: foreach argument modification inside loop, followed by return from loop
by shawnhcorey (Pilgrim) on Jul 10, 2013 at 12:20 UTC

    Acutally, it's more like:

    my @a = ( 1, 2, 3, 4, 2, 5, 7 ); my $last = $#a; my $i = 0; while( $i <= $last ){ if( $a[$i] == 2 ){ $last --; last if $i > $last; } }continue{ $i ++; } $#a = $last;
      Actually, it's more like:

      No, it isn't.

      The OP's code stops at the first occurrence. Yours does not. Hence, you (may) end up removing additional elements, per your own example:

      Test code as copied from original nodes:

      Output:

      shawnhcorey: 1 2 3 4 2 OP vsespb: 1 2 3 4 2 5

      Anyway, this is moot now that the OP has posted actual code which uses splice to remove an internal element rather than the last element via pop.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://1043438]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others contemplating the Monastery: (13)
As of 2014-08-27 21:25 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (253 votes), past polls