Reducing the complexity of dispatch tables

[hacker]

With the help and patience of wfsp, Anno and others on the CB, I was able to trim out some fat in my existing dispatch tables for a site I'm rewriting.

What I'm currently left with (which works, so far), is this:

 my %dispatch = (
        home            => \&home,
        donate          => \&donate,
        news            => \&news,
        samp            => sub { \&samples($dbh) },
        ...
);

$action         = $apr->param('a') || 'home';
$dispatch{$action}->($action);

sub render_template {
        my ($action, $params) = @_; 
 
        my $template    = HTML::Template->new(
                                die_on_bad_params => '0',
                                filename => "$action");
        my $content     = $template->output;
        print $content;
}

sub home {
        my ($action, $params) = @_;
        return render_template($action, $params);
}

sub donate {
        my ($action, $params) = @_;
        return render_template($action, $params);
}

sub news {
        my ($action, $params) = @_;
        return render_template($action, $params);
}

sub samples {

        my ($dbh) = shift;
        ....
}

Note: There's one small bug I just found when composing this (if I pass an unknown param, it should default to 'home', but it doesn't, it just dies with Can't use string ("") as a subroutine ref while "strict refs" in use). I'll try to fix that shortly.

What I'm wondering now, is with these subs being identical now except for their sub name... can I make these into anonymous subs, and pass the $action down to them, reducing the 20+ identical subs down to 1?

[ikegami]

You could just refer to the same sub more than once:

my %dispatch = (
   home   => \&default,
   donate => \&default,
   news   => \&default,
   samp   => sub { \&samples($dbh) },
   ...
);

$action = $apr->param('a');
$action = 'home' if !exists $dispatch{$action};
$dispatch{$action}->($action);

sub default {
   my ($action, $params) = @_;
   return render_template($action, $params);
}

sub samples {
   my ($action, $dbh) = shift;
   ...
}

Shortcut:

my %dispatch = (
   (map { $_ => \&default } qw(
      home donate news
   )),
   samp => sub { \&samples($dbh) },
   ...
);

[jdporter]

I'd consider using a namespace (package) rather than a dispatch table (hash). The concept is the same, but you get name checking for free and it's probably faster.

use strict; use warnings;

{
package Dispatch;

    sub render_template {
        my ($action, $params) = @_; 
        print "render_template( action => $action, params => $params )
+\n";
    }

*home   = 
*donate = 
*news   = sub {
    my $pkg = shift;
    my ($action, $params) = @_;
    return render_template($action, $params);
};

    sub samples {
        my ($dbh) = shift;
        print "samples( dbh = $dbh )\n";
    }

sub samp {
    my $pkg = shift;
    samples('$DBH');
}

sub AUTOLOAD {
    my $pkg = shift;
    our $AUTOLOAD;
    print "Error: no such function $AUTOLOAD ( @_ )\n";
}

}

# test:

for my $act ( qw( home donate news samp bogus ))
{
    Dispatch->$act($act,"PARAMS");
}

Note - You'll get a "used only once" warning on the assignments to *home etc. To squelch it, you can enclose those assignments in a block like so:

{
  no warnings 'once';
  . . .
}

[BrowserUk]

Your entire dispatch table can be replaced by the following 2 lines:

$action = $apr->param('a') || 'home';
render_template( $action, $action );

[moritz]

Not quite: in this version an invalid but defined $action will reach render_template.

[BrowserUk]

True, but with the addition of a check for failure of HTML::Template to find an appropriate file...

[jdporter]

Um... What about the samp entry, which runs the sample subroutine?

[BrowserUk]

Oh gosh. You got me?

.oO(rename the template...or $action =~ s[samp][sample]...or use a hash to tranform short names to long ones...or)