Yes, exactly what I was thinking of, and what I ran into.

Here's a complete chunk of code that illustrates the problematic behavior (I'll leave it exposed as an added hint):

my @cb = (\&callback);
$_->() foreach (@cb);
sub callback {
  open(my $fh, "/tmp/somefile") or die;
  while(<$fh>) {
    print "orange!\n" if /red/;
  }
}
[download]

The observed behavior was an 'undefined subroutine' error -- and yet, if you cut & paste the above code, it will run fine. (So the above is buggy, but simply running it will not reveal the error.)

Spoilers ahead...

Read more... (889 Bytes)

In this case, I'd say it's bad style to use $_ instead of a named variable for that loop anyway. You could localize $_, but I'd consider that an uglier solution.

This is exactly why I try not to use $_ unless in map or grep (but it's not like I have a choice there anyway). This foreach shortcut is ripe with problems just like this. I have always felt that it is much better to take the few seconds of time to type what you mean explicity rather than have to spend alot more time hunting around for an insidious bug like this. That is the right kind of laziness IMO.

I hate to say it, but its idioms like this that give perl such a bad rap. Sure you can bang out a script in 30 seconds that does the work of 6 Java programmers, but at what cost? There is nothing worse than having to maintain a 12-headed monster that was born as that 30 second script. Do future programmers a favor, hell do the perl community as a whole a favor, and take the time to type the few extra characters (Note to OP: this is not directed at you specifically but at the perl community-at-large).

Now go ahead do what you will, I care not for the XP ;-P

-stvn

Read more... (519 Bytes)

---

;$;=sub{$/};@;=map{my($a,$b)=($_,$;);$;=sub{$a.$b->()}} split//,".rekcah lreP rehtona tsuJ";$\=$;[-1]->();print

Are we still avoiding spoilers? I'll just play it safe.

Read more... (708 Bytes)

Sorry, but I don't see why (on its own) this is a bad idea. If @callbacks is under your complete control and you know that all your callback subs behave according to some set of rules then I see little wrong with it. If however @callbacks is not under your control then you have a different issue. At that point I would recommend a series of checks:

foreach my $callback (@callbacks) {
    (ref($callback) eq "CODE") || die "$callback is not a code ref";
    my @temp_args = @{[@args]};
    $callback->(@temp_args);
    if (verify(@temp_args)) {
        @args = @temp_args;
    }
    else {
        die "callback did something funky to args";
    }
}
[download]

Please provide a bit more information on the code, and how things like @callbacks and @args get populated, along with maybe a bit more context as to where this all gets executed.

You're trying too hard, IMO.

my @copy_args;
foreach( @callback ) {
    local $_ = $_;
    $_->( @copy_args = @args );
}
[download]

Makeshifts last the longest.

Maybe, but TIMTOWTDI. Your code solves the problem for sure, my code assumes that everyone is out to get me, and that nothing is safe. But if you read my post, I suggested that code if the sub refs in @callbacks we "untrusted", not a just a solution to the problem. But again, TIMTOWTDI, and my way is certainly the more paranoid way.

- stvn

Btw, this is superflous:

my @temp_args = @{[@args]};
[download]

You are making a copy of a copy of the original. Semantically it is the same as

my @temp_args = @args;
[download]

Makeshifts last the longest.

Aristotle,

Your right, I am making an assumption of a single level structure and that would be copied-by-value anyway. And considering that I do this:

    if (verify(@temp_args)) {
        @args = @temp_args;
    }
    else {
        die "callback did something funky to args";
    }
[download]

Then actually doing a proper deep-copy of @args would only make sense if you planned to catch the exception being thrown in the else block. Basically, my intention was to build a sandbox for the callbacks to run in with commit and rollback functionality. A real implementation would require much more knowledge of the data in @args and the functionality of the app as a whole.
-stvn

Did you test this?

use strict;
use Test::More tests => 1;

my @cb   = 
( 
    sub { $_[0] = reverse $_[0] },
    sub { $_[1] = reverse $_[1] },
);

my @args = qw( one two );

for my $cb (@cb)
{   
    $cb->((@args));
}

is_deeply( \@args, [qw( one two )] );
[download]

Besides that, any argument passing in Perl has that drawback.

Dude... those parentheses don't cause the array to be copied. If you want do an in-line copy like that, I recommend:

foreach (@callbacks) {
  $_->( @{[ @args ]} );
}
[download]

------------
:Wq
Not an editor command: Wq
[download]

bracket happy? :)

$->( @{ \@args } );
[download]

Erm, and that's any different than explicitly calling multiple subs with the same array as an argument how? Just because you're using a coderef to call it or not doesn't change the fact that subs receive aliases.

## I.e. just as "dangerous"
foo( @args );
blah( @args );
zorch( @args );
[download]

How are your two arrays, @callbacks and @args, populated?

How is any of this any surprise? It's just Yet Another Imprudent Use Of $_. We have seen so many cases like these. A common trap seems to be calling a function with a while(<>){} from inside a map or such. There are examples up the wazoo about this kind of trap.

Always localize $_ before you clobber it!!

It's that simple. Don't act all surprised when you get bitten in the rear if you don't. The only time you can skip this step is foreach, map, and grep, because they alias, rather than assign, to $_. Other than those, localize without even thinking. Make it a habit. Make it a habit. Really, make it a habit.

foreach (@callbacks) {
  $_->(@args);
  $_->(@otherargs);
}
[download]