Beefy Boxes and Bandwidth Generously Provided by pair Networks
Your skill will accomplish
what the force of many cannot
 
PerlMonks  

naming anonymous subroutines inner variables

by writch (Sexton)
on Jun 22, 2016 at 20:04 UTC ( [id://1166298]=perlquestion: print w/replies, xml ) Need Help??

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

I might be being a bit more clever than I should be, but it seemed such a good idea at the time.

I have some heavily query driven code. I intended to use a hash of SQL queries to build out a number of prepared queries. That part is working fine.

Then, I realized, rather than write functions to return the result sets, I maybe could, in the same loop which is doing the preparation of the queries, make an anonymous subroutine and assign it, rather than writing it by hand.

The problem is in the sixth line. When I do my 'test' query, $self->{test} is the prepared query, and $self->{ftest}() is the code that would execute it.

But the $_ that turned out fine in the earlier uses is useless in the code itself.

The error is 'Can't call method "execute" on an undefined value' because (of course) the line is actaully $self->{}->execute(@_) instead of $self->{test}->execute(@_), as $_ isn't populated when it's being executed, just when it's being created.

for (keys %{$self->{SQL}}) { $self->{$_} = $self->{DBO}->prepare($self->{SQL}->{$_}); $self->{"f$_"} = sub { my $self = shift; my @res; $self->{$_}->execute(@_); while (my $dbrow = $self->{$_}->fetchrow_h +ashref()) { push @res, \$dbrow; } return \@res; }; }

Replies are listed 'Best First'.
Re: naming anonymous subroutines inner variables
by AnomalousMonk (Archbishop) on Jun 22, 2016 at 20:17 UTC

    For the anon subroutine to access each key at its | the subroutine's runtime, it needs to create a closure over a lexical variable (not, of course, access the global $_ variable) when the subroutine is created. Try something like (untested):

    for my $k (keys %{$self->{SQL}}) { ...; $self->{"f$k"} = sub { ...; $self->{$k}->execute(@_); while (my $dbrow = $self->{$k}->fetchrow_ +hashref()) { ...; } return ...; }; }
    Basically, all the  $_ scalars become  $k scalars.


    Give a man a fish:  <%-{-{-{-<

      Didn't do anything except make the value that fails $self->{$k} instead of $self->{$_}.
        Works for me:
        #!/usr/bin/env perl use 5.014; use strict; use warnings; my $self; for my $k (qw(foo bar baz)) { $self->{$k} = sub { return join $k, @_ }; $self->{"f_$k"} = sub { my $self = shift; return $self->{$k}->(@_); }; } for my $k (qw(foo bar baz)) { say $self->{"f_$k"}->($self, qw(a b c)); } __END__ afoobfooc abarbbarc abazbbazc
        It would be more productive if you would show something we could actually run.
Re: naming anonymous subroutines inner variables
by GrandFather (Saint) on Jun 22, 2016 at 21:22 UTC

    As a general thing don't use $_ where you can instead use a named variable. Names are vital pieces of documentation that help write correct code and facilitate maintenance by that future stranger who has to deal with your code.

    In this case it would highlight the fact that the $_ content is being set during the for loop execution and maybe make you ponder what that implies for the content of the variable when the sub is actually called.

    Even better, take the sub out of line. That fixes the immediate problem and also makes the business logic of the loop clearer, especially if you give the sub a sensible name.

    Premature optimization is the root of all job security
Re: naming anonymous subroutines inner variables
by BrowserUk (Patriarch) on Jun 22, 2016 at 21:03 UTC

    Do:

    for (keys %{$self->{SQL}}) { my $closure = $_; $self->{$_} = $self->{DBO}->prepare($self->{SQL}->{$_}); $self->{"f$_"} = sub { my $self = shift; my @res; $self->{ $closure }->execute(@_); while (my $dbrow = $self->{ $closure }->fe +tchrow_hashref()) { push @res, \$dbrow; } return \@res; }; }

    What you have at the moment is like this:

    undef %h; for( qw[ the quick brown fox ] ){ $h{ $_ } = sub { print "$_"; }; }; pp \%h; do { my $a = { brown => sub { "???" }, fox => 'fix', quick => 'fix', the +=> 'fix' }; $a->{fox} = $a->{brown}; $a->{quick} = $a->{brown}; $a->{the} = $a->{brown}; $a; } $_='joe'; for my $key ( keys %h ) { $h{ $key }->(); };; joe joe joe joe

    Ie. The subroutine is referencing the variable $_; and when it is called, it will use whatever the current value of $_ happens to be at the time.

    What you need is this which creates a separate closure for each subroutine instantiated in the loop:

    undef %h; for( qw[ the quick brown fox ] ){ my $closure = $_; $h{ $_ } = sub { print $closure; }; }; pp \%h; { brown => sub { "???" }, fox => sub { "???" }, quick => sub { "???" }, the => sub { "???" }, } $_='joe'; for my $key ( keys %h ) { $h{ $key }->(); };; the fox brown quick

    With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    "Science is about questioning the status quo. Questioning authority". I knew I was on the right track :)
    In the absence of evidence, opinion is indistinguishable from prejudice. Not understood.
      Same as the 'my $k' example. I have a line in the anonymous subroutine that says: $self->{$closure}->execute(@_); and now it's $closure that's undefined instead of $_ or $k.

        Not so. Look above again.


        With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority". I knew I was on the right track :)
        In the absence of evidence, opinion is indistinguishable from prejudice. Not understood.
Re: naming anonymous subroutines inner variables
by runrig (Abbot) on Jun 22, 2016 at 21:43 UTC
    Seems a waste to copy the sql from, e.g., $self->{SQL}->{test} to $self->{test}, and I would assume $self is already a closure, so you shouldn't need to pass it to the sub, and the sth itself can be a closure and so not stored directly in $self, so I might go with:
    for ( keys %{$self->{SQL}} ) { my $name = $_; my $dbh = $self->{DBO}; my $sth = $dbh->prepare($self->{SQL}{$name}; $self->{STH}{$name} = sub { my @res; $sth->execute(@_); while (my $dbrow = $sth->fetchrow_hashref()) { # Not sure why you're making a reference to a hashref here push @res, \$dbrow; } return \@res; }; }
    And if you don't need to have references to hashrefs in the arrayref, then the execute() and the while loop can be replaced with:
    my $res = $dbh->selectall_arrayref($sth, {Slice => {}}, @_); return $res;
    Or even (only $sth needs to be a closure):
    return $sth->{Database}->selectall_arrayref($sth, {Slice => {}}, @_);
Re: naming anonymous subroutines inner variables
by zwon (Abbot) on Jun 22, 2016 at 21:04 UTC
    and $self->{ftest}() is the code that would execute it
    That should be $self->{ftest}($self)

    Update alternatively you can get rid of my $self = shift;

      This is actually the controlling issue. $_ doesn't work in any case, but either the $k or the $closure method work with either $self supplied and shifted or ignored and not shifted

        either the $k or the $closure method work

        Be careful! Whilst the for my $name... works, it is dangerous:

        my @names = qw[ the quick brown fox ]; undef %h; for my $alias ( @names ){ $h{ $alias } = sub { print $alias; $alias = 'fred'; ####### Mysterious action at a distance +here }; }; pp \%h; { brown => sub { "???" }, fox => sub { "???" }, quick => sub { "???" }, the => sub { "???" }, } for my $key ( keys %h ) { $h{ $key }->(); };; the fox brown quick print @names;; fred fred fred fred

        With the for my $name ( ... ) method, the closures are and remain aliases to the source list in the for loop; which means that if you assign to the closure within the subroutine, you will cause spooky action at a distance to the content of that source.

        And perhaps worse, changes to the source of the for list, will remotely change the contents of the closures:

        [0]{} Perl> my @names = qw[ the quick brown fox ]; undef %h; for my $alias ( @names ){ $h{ $alias } = sub { print $alias; }; }; pp \%h; { brown => sub { "???" }, fox => sub { "???" }, quick => sub { "???" }, the => sub { "???" }, } $names[ 1 ] .= 'Mysterious changes'; ###### And again here! for my $key ( keys %h ) { $h{ $key }->(); };; the fox brown quickMysterious changes

        With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority". I knew I was on the right track :)
        In the absence of evidence, opinion is indistinguishable from prejudice. Not understood.
Re: naming anonymous subroutines inner variables
by anonymized user 468275 (Curate) on Jun 24, 2016 at 08:57 UTC
    It's hard to believe you expect the random order of SQL statements returned from a hash to remain acceptable for long. But rather than pick at details, I believe the greatest improvement would be to think about code design at the highest level. Consider organising your functionality into separate utility modules. I use a master-class module for methods I want to call from anywhere and a DB module for utilities that relate to databases and files. In this case you probably want a subclass called e.g. DB::Queue for managing a statement queue object and methods for appending, executing and transaction behavior (e.g. commit or rollback the lot) for these queues. It is rarely a good idea to build complex code that only services local blocks. Often you will be rewarded when generic code proves actually much easier to write and test.

    One world, one people

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://1166298]
Approved by sweetblood
Front-paged by Corion
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others having an uproarious good time at the Monastery: (6)
As of 2024-03-28 12:10 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found