Help me improve this sub with named params!

[waxmop]

This subroutine allows the user to override some default values by using named params:

sub draw_form
{
    my %args = (
                'buttons' => [],
                'action' => 'parse.cgi',
                @_
                );
    while ( my($k, $v) = each(%args) ) {
        if ( ref($v) eq 'ARRAY' ) {
            $v = "@$v";
        }
        print "$k: $v.\n";
    }
    print "---\n";
}

That works well enough. But I'm worried about potentially harmful stuff happening like this:

#I don't want people adding stuff like this!
draw_form(logged_in_already => 'true');

So, I can improve the draw_form subroutine like so:

sub safer_draw_form
{
    my %args = (@_);

    my @buttons = ();
    if ( $args{'buttons'} ) {
        push @buttons, @{$args{'buttons'}};
    }

    my $action = 'parse.cgi';
    $action = $args{'action'} if $args{'action'};

    print "action: $action.\n";
    print "buttons: @buttons.\n";

    print "---\n";
}

However, as the list of params increases, all that handling at the beginning is going to get clunkier and clunkier. I hope the community can help me smooth this code out.

[perrin]

Params::Validate does what you want, and it's fast.

[blokhead]

It sounds like there are only a certain number of parameters you consider "valid"... You can explicitly specify them and just ignore the rest of the hash.

my @valid = qw/buttons action foo bar xyz/;
my %args = (
    buttons => [],
    action  => 'parse.cgi',
    @_
);

%args = map { exists $args{$_} ? $_ => $args{$_} : () }
        @valid;

while (my ($k,$v) = each %args) {
   ## etc...
}

This code is careful not to add additional key=>undef pairs to the hash, thus the exists check.

Alternately, you could just iterate over all valid keys and only print the ones that are available in the hash (instead of using each). This method would always print the list in a predictable order, if that's important to you:

my @valid = qw/buttons action foo bar xyz/;
my %args = (
    buttons => [],
    action  => 'parse.cgi',
    @_
);

foreach my $k (@valid) {
    next if not exists $args{$k};
    my $v = $args{$k};
    $v = @$v if ref $v eq 'ARRAY';

    print "$k => $v\n";
}

blokhead

[BUU]

for a slightly nicer version, what about:

my @valid = qw/foo bar baz/;
my %args=(stuff);

for(@args{@valid})
{
 next if not defined;
}

[blokhead]

I thought of doing something similar, but on the off chance that foobar( valid_arg => undef ); would be actually allowed I wrote what I did. Your code wouldn't be able to distinguish that call from a call that omitted the valid_arg parameter. Also, you'd have to add slightly more code to get key,value pairs (instead of just iterating over the values as you are now). But yours probably is nicer if we don't need to worry about undef parameter values.

blokhead

[Abigail-II]

I'm sorry, but I fail to understand what your question is. Perhaps it helps to understand if you explain why

    draw_form(logged_in_already => 'true');

is a problem. Since draw_form isn't using logged_in_already, what's the problem?

Abigail