Calling a function via a variable

[csdude55]

I have code that looks like this:

$action = param('action');

if ($action eq 'process_send_pm')      { process_send_pm(); }
elsif ($action eq 'process_purge')       { process_purge(); }
elsif ($action eq 'process_folders')      { process_folders(); }
elsif ($action eq 'process_ignore')      { process_ignore(); }
elsif ($action eq 'process_modify_profile') { process_modify_profile(); }

I was thinking about changing it to this, for the sake of brevity:

if (
 $action eq 'process_send_pm' ||
 $action eq 'process_purge'   ||
 $action eq 'process_folders'   ||
 $action eq 'process_ignore'   ||
 $action eq 'process_modify_profile') { &{$action}; }

I keep reading that this is a bad idea, but... why? If I'm testing to make sure that $action matches pre-approved strings before sending, am I still opening up to an unforeseen problem?

[NickMNS]

I have very little, in fact almost no experience, in PHP much less Perl. I code in Python and JS. But FWIW in this situation I would create an object of key/value pairs where the key is the action string and value is the function call:

action_map = {'process_send_pm': process_send_pm(), 
      'process_purge':process_purge(),
      'process_folders':process_folders(),
      'process_ignore':process_ignore(),
      'process_modify_profile': process_modify_profile() }

Then I would take the value of your $action and use it as the key to call the function:

// assuming action = 'process_purge'
action_map[action]
// this would call the function process_purge()

This avoids the entire if else || chain. It is clean and easier to read. Now whether or not that works in Perl is a whole other question.

[justpassing]

I keep reading that this is a bad idea, but... why? If I'm testing to make sure that $action matches pre-approved strings before sending, am I still opening up to an unforeseen problem?

I think that the main concern is about using a "variable" , which , by definition can "vary". So a good coding practice would be to avoid this kind of usage, because, between the moment the variable is set, and the moment it's used to invoke the procedure, it might have be changed / altered for "some reasons" (bad coding, malicious code injection etc..). However, In your example, I don't see this kind of problem, since you get the value, test it, and call the function in a row.

[robzilla]

Nothing can logically go wrong there, so I don't really see the problem either.

I'm not too proficient in Perl, but, since you like brevity, perhaps you could also do this?

if($action ~~ ('process_send_pm', 'process_purge', 'process_folders', 'process_ignore', 'process_modify_profile')) {
 &{$action};
}

Requires Perl 5.10+ and may throw an "experimental" warning, so only if you're feeling adventurous ;-)

[csdude55]

That's pretty cool, Rob! I'm using v5.10.1 but haven't done a huge amount of Perl work in a long time, so I haven't used the ~~ operator. I found some info on it, though, and it seems like I should have been using it all along:

[perlmaven.com...]

Nick, this was already in the middle of an if-else so I was going to have to use an if() statement, anyway; otherwise, your map solution would have been great and easy to read! Thanks for that tip, too.

Thanks for the help, all!

[csdude55]

I hate to revive an old thread, but for any future users, the proper way to put the array inline was with square brackets, not parentheses:

if ($action ~~ 
 [
  'process_send_pm',
  'process_purge',
  'process_folders',
  'process_ignore',
  'process_modify_profile'
 ]
) { &{$action}; }

But I'm concerned because I read where someone in 2017 stated:

Do not use if ("value" ~~ @array). ~~ is an experimental feature called Smartmatch. The experiment appears to be deemed a failure and will be removed or modified in a future version of Perl

Can anyone confirm this? I haven't found anything more updated than that.

I've also read that you can use Data::Munge for a replacement of smartmatch, or List::Util which is in core perl (meaning you don't have to install it separately). Using a module seems like overkill for me if I can safely use smartmatch, but it might be relevant for future readers.