Can't use string ("1") as a subroutine ref while "strict refs" in use

Question

In a Perl daemon reacting to various events I'm trying to use a Null object pattern in 2 cases by creating anonymous subroutines, which should just return a value of 1 aka "true" (please scroll to the right to see the check subroutines for LOGIN and ALIVE events):

package User;

our %EVENTS = (
        LOGIN   => {handler => \&handleLogin,   check => sub {1},     },
        CHAT    => {handler => \&handleChat,    check => \&mayChat,   },
        JOIN    => {handler => \&handleJoin,    check => \&mayJoin,   },
        LEAVE   => {handler => \&handleLeave,   check => \&mayLeave,  },
        ALIVE   => {handler => sub {},          check => sub {1},     },
        BID     => {handler => \&handleBid,     check => \&checkArgs, },
        TAKE    => {handler => \&handleTake,    check => \&checkArgs, },
  # .... more events ....
);


sub action($$$) {
        my $user  = shift;
        my $event = shift;
        my $arg   = shift;
        my $game  = $user->{GAME};

        unless (exists $EVENTS{$event}) {
                print STDERR "wrong event: $event\n";
                return;
        }

        my $handler = $EVENTS{$event}->{handler};
        my $check   = $EVENTS{$event}->{check};

        return unless $user->$check->($arg); # XXX fails
        $user->$handler->($arg);
}

sub mayChat($$) {
        my $user = shift;

        return if $user->{KIBITZER};
}

# ...... more methods here ...

1;

Unfortunately I get the runtime error for LOGIN event:

Can't use string ("1") as a subroutine ref while "strict refs" in use

Does anybody please know how to fix it here?

How to provide a "function pointer" to an anonymous Perl subroutine?

The handler => \&sub { 1 } doesn't do it either.

Using perl 5.8.8 and perl 5.10.1 on CentOS 5.x and 6.x

UPDATE:

I've also tried following:

    my $check = $EVENTS{$event}->{check};
    return unless $check->($user, $arg);

but it doesn't help. I think this rules out the "missing blessing" suggested in some answers.

UPDATE 2:

I have extended the source code snippet in my original question. The background is: I'm in the process of refactoring of my source code and thus I've created the %EVENTS hash as listed above, so that for each incoming event (a string sent over TCP-socket from a Flash client) there is a reference to a subroutine (check) which validates the event and a reference to another subroutine (handler) which performs some actions. I'm not sure if other subroutines work - I'm stuck already at the first LOGIN event.

I also don't understand why doesn't check => sub { 1 } above work - isn't sub supposed to return a reference to an anonymous subroutine (when the name is omitted - according to perldoc perlref section 4)?

UPDATE 3:

The output of print Dumper(\%EVENTS) -

$VAR1 = {
          'PLAY' => {
                      'check' => sub { "DUMMY" },
                      'handler' => sub { "DUMMY" },
                    },
          'JOIN' => {
                      'check' => sub { "DUMMY" },
                      'handler' => sub { "DUMMY" },
                    },
          'OVER1' => {
                       'check' => sub { "DUMMY" },
                       'handler' => sub { "DUMMY" },
                     },
          'ALIVE' => {
                       'check' => sub { "DUMMY" },
                       'handler' => sub { "DUMMY" },
                     },
          'DISCARD' => {
                         'check' => $VAR1->{'PLAY'}{'check'},
                         'handler' => sub { "DUMMY" },
                       },
          'MISS1' => {
                       'check' => sub { "DUMMY" },
                       'handler' => sub { "DUMMY" },
                     },
          'LOGIN' => {
                       'check' => sub { "DUMMY" },
                       'handler' => sub { "DUMMY" },
                     },
          'TAKE' => {
                      'check' => $VAR1->{'PLAY'}{'check'},
                      'handler' => sub { "DUMMY" },
                    },
          'ONEMORE' => {
                         'check' => sub { "DUMMY" },
                         'handler' => sub { "DUMMY" },
                       },
          'OVER2' => {
                       'check' => sub { "DUMMY" },
                       'handler' => sub { "DUMMY" },
                     },
          'MISS2' => {
                       'check' => sub { "DUMMY" },
                       'handler' => sub { "DUMMY" },
                     },
          'EXACT' => {
                       'check' => sub { "DUMMY" },
                       'handler' => sub { "DUMMY" },
                     },
          'TRUST' => {
                       'check' => $VAR1->{'PLAY'}{'check'},
                       'handler' => sub { "DUMMY" },
                     },
          'LEAVE' => {
                       'check' => sub { "DUMMY" },
                       'handler' => sub { "DUMMY" },
                     },
          'DEFEND' => {
                        'check' => $VAR1->{'PLAY'}{'check'},
                        'handler' => sub { "DUMMY" },
                      },
          'OPEN' => {
                      'check' => $VAR1->{'PLAY'}{'check'},
                      'handler' => sub { "DUMMY" },
                    },
          'REVEAL' => {
                        'check' => sub { "DUMMY" },
                        'handler' => sub { "DUMMY" },
                      },
          'CHAT' => {
                      'check' => sub { "DUMMY" },
                      'handler' => sub { "DUMMY" },
                    },
          'DECLARE' => {
                         'check' => $VAR1->{'PLAY'}{'check'},
                         'handler' => sub { "DUMMY" },
                       },
          'BACK' => {
                      'check' => sub { "DUMMY" },
                      'handler' => sub { "DUMMY" },
                    },
          'MISERE' => {
                        'check' => sub { "DUMMY" },
                        'handler' => sub { "DUMMY" },
                      },
          'BID' => {
                     'check' => $VAR1->{'PLAY'}{'check'},
                     'handler' => sub { "DUMMY" },
                   }
        };

Hi Jack, I've marked it with an "XXX fails", please scroll to the right — Alexander Farber, Mar 29 '12 at 16:50
`$obj->$code->(@args)` is like `my $val = $obj->$code; $val->(@args);' because of the precedence of the `->` operator (see `perldoc perlop`). I think what you're looking for rather is `$obj->$code(@args)` which is like `$code->($obj, @args)`. — Anonymous, Mar 29 '12 at 20:14

mob · Answer 1 · 2012-03-29T19:35:19.217

3

$check is already a code reference, so you could say

return unless $check->($arg);

Your existing code could also be salvaged if $check were a reference to code that returned a code reference:

our %EVENTS = ( LOGIN => { ..., check => sub { sub { 1 } }, } ... );

Think of sub { } as a "code reference" operator the way that \ is an operator to create a scalar reference, or [...] is an operator to create an array reference.

edited Mar 29 '12 at 19:35

answered Mar 29 '12 at 17:08

mob

117,087
18
149
283

But if I call just $check->($arg) then it won't work for the other events - because mayChat(), mayJoin(), mayLeave(), etc. are methods to be called on the $user object. – Alexander Farber Mar 29 '12 at 17:50
You and darch hit on the right answer. You'll want to make that `$check->($user,$arg)`. – mob Mar 29 '12 at 21:57

score 3 · Accepted Answer · answered Mar 29 '12 at 21:37

The problem is not with the particular event that is surfacing the problem; the actual bug is in action. In particular, the line

    return unless $user->$check->($arg); # XXX fails

doesn't do what you think it does. Between the presence of prototypes and Perl's willingness to try and call a sub specified by name, you wind up eventually calling User:: for the CHAT event. Which doesn't seem to be what you intended for it to do.

The more correct call looks like

    return unless $check->($user, $arg);

This expects $check to contain a subref (which it does), dereferences it and calls it. This works even though sometimes $check will refer to a prototyped function.

That leaves the problem that this procedural code doesn't respect inheritance. To do that, you have to rephrase %EVENTS a bit. Thus:

our %EVENTS = (
        LOGIN   => {handler => \&handleLogin,   check => sub {1},     },
        CHAT    => {handler => \&handleChat,    check => sub { shift->mayChat(@_) },
        ...
);

Note that you are strongly discouraged to mix function prototypes and Perl OO programming precisely because it can lead to hard-to-diagnose problems like this one.

In reference to your other question: my $foo = sub { } is indeed how you construct anonymous subroutines. But you do need to call them appropriately.

+1 When `$check` is a code ref to a `User` method, `$check->($user,$arg)` is the same as `$user->name_of_method($arg)`, and when `$check` is more trivial (`$check=sub{1}`), `$check->($user,$arg)` is still valid and meaningful. — mob, Mar 29 '12 at 21:54

score 2 · Answer 3 · 2012-03-29T17:00:39.117

2

Unless $check is a code reference,

$user->$check->($arg);

won't work.

As a self-contained example:

    > perl -e 'use strict;use warnings;my $a=17;$a->("whatever");'
Can't use string ("17") as a subroutine ref while "strict refs" in use at -e line 1.

So you'll have to look more carefully at the structure of your data and avoid treating scalars as code references.

edited Mar 29 '12 at 17:00

answered Mar 29 '12 at 16:51

That's a good comment, thank you. Doesn't answer the main question though. By the way $user is a User.pm object, so I think it will work in my case. – Alexander Farber Mar 29 '12 at 16:53
Well, to fix it, you'll need to look more closely at the structure of your data. [`Data::Dumper`](http://perldoc.perl.org/Data/Dumper.html) is your friend. – Mar 29 '12 at 16:54
$check is not an object, it is a method defined in User.pm. And $user is an User.pm object. And to rephrase my question would probably be: perl -e '%events = (check=>sub {1}); print "ok\n" if $events{check}->()==1;' - here calling the anonymous subroutine seems to work, but in my code not, for some reason. – Alexander Farber Mar 29 '12 at 16:58
Yes, I use Data::Dumper a lot. Why not just provide the answer if you've spotted the error? – Alexander Farber Mar 29 '12 at 17:00
Edit made: to be more specific, `$check` seems to be a scalar, and not a subroutine reference. Thus, any attempt to call it as a code reference will produce an error. – Mar 29 '12 at 17:01
Yes, I understood sofar too, thanks. But how to make it a sub ref? – Alexander Farber Mar 29 '12 at 17:02
You'll have to change the structure of your data. Make it a sub ref instead of a scalar. Take a look at [`perldoc perlref`](http://perldoc.perl.org/perlref.html). – Mar 29 '12 at 17:02
Answer is simply wrong: $check is a code reference, its not being a code reference cannot be the problem. – darch Mar 29 '12 at 18:48
@darch - With only the partial code given the question, it's far from obvious that `$check` is a code reference. – Mar 30 '12 at 12:00

Bjorn Harpe · Answer 4 · 2012-03-29T18:58:47.197

2

looks like $event is either LOGIN or ALIVe, both have anonymous subs for the check key that return 1. $check is locally defined to that subroutine, and returns 1, the code then attempts to access the value '1' in the user hash/object as a hash

edited Mar 29 '12 at 18:58

answered Mar 29 '12 at 17:05

Bjorn Harpe

374
4
13

score 1 · Answer 5 · answered Mar 29 '12 at 20:08

1

My own answer - the following seems to work, I probably was dereferencing my sub refs in a wrong way...

sub action($$$) {
        my $user  = shift;
        my $event = shift;
        my $arg   = shift;

        my $handler = $EVENTS{$event}->{handler};
        my $check   = $EVENTS{$event}->{check};

        return unless &$check($user, $arg);
        &$handler($user, $arg);
}

answered Mar 29 '12 at 20:08

Alexander Farber

21,519
75
241
416

1

Right. `&$check(@args)` is the same thing as `$check->(@args)`, as `perl -MO=Deparse ...` will verify. – mob Mar 29 '12 at 21:56

score 0 · Answer 6 · answered Mar 29 '12 at 19:01

0

The key fact about your program that you left out of your question is this: mayChat and friends all return subroutine references. These are then evaluated at

    return unless $user->$check->($arg); # XXX falis

. Entries in %EVENTS are references to routines that return subroutine references. Once we've stated it in those terms, the problem with your original code becomes obvious.

$EVENTS{LOGIN}->{check} is a reference to a sub that returns an integer, when what is expected is a reference to a sub that returns a reference to a sub. If we make it a reference to a sub that returns a reference to a sub:

    LOGIN   => {phase => undef,      handler => \&handleLogin,   check => sub { sub {1} },     },

, it works.

The fix for your problem is to (1) make those entries be subs that return subs and (2) document the interface of %EVENTS so you are the last person to have this problem.

answered Mar 29 '12 at 19:01

darch

4,200
1
20
23

I'm sorry, but this isn't true. I have extended the source code snippet in my original question, so that this more visible. I'm in the process of refactoring of my source code and thus I've created the %EVENTS hash as listed above, so that for each incoming "event" (a string sent over TCP-socket from Flash client) there is a reference to a subroutine (check) which validates the event and a reference to another subroutine (handler) which performs some actions. I'm not sure if other subroutines work - I'm stuck already at the first LOGIN event. – Alexander Farber Mar 29 '12 at 19:29
Wait. You're saying you don't know that the other calls work? It's true the above was predicated on the idea that everything you didn't mention as not working worked. If what you're saying is that `action` has never worked for any input, that's a completely different kettle of fish. – darch Mar 29 '12 at 21:19
You keep implying that I have some very special setup or have omitted some important key facts. This isn't so, I have a mere hash of hashes with few subroutine references. I think, I have provided enough information in my question too. – Alexander Farber Mar 30 '12 at 08:06
The above answer was based on unwarranted assumptions, primarily that the code in question worked in some cases. That made it wrong. There is in fact one important piece of information omitted from the question: That the code in question does not work for any case. If you'd mentioned that, you'd have gotten an answer more quickly. – darch Apr 01 '12 at 23:46

Can't use string ("1") as a subroutine ref while "strict refs" in use

6 Answers6

Linked