Question

I am trying to compare my hash input to valid allowed options in my data structure, and if it's not one of the options then I set the default value for the key. I seem to be missing something here though.

Example of current data structure..

my $opts = {
     file => { require => 1 },
     head => {
               default => 1,
               allowed => [0,1],
             },
     type => { 
               default => 'foo', 
               allowed => [qw(foo bar baz)] 
             },
};

$args is my hash ref ( file => 'file.txt', type => 'foo', head => 1 )

Snippet of what I've tried..

for my $k ( keys %$opts ) {
   croak("Argument '$k' is required in constructor call!")
       if $opts->{$k}->{require} and !exists $args->{$k};

   if (exists $args->{$k}) {
     if (grep {!$args->{$k}} @{$opts->{$k}->{allowed}} ) {
       $args->{$k} = $opts->{$k}->{default};
     }
     ...
   } else {
     ..set our defaults
     $args->{$k} = $opts->{$k}->{default};
   }
}
Was it helpful?

Solution

The checking for allowed values is faulty.

The grep function takes a code block and a list. It sets the $_ variable to each element in the list in turn. If the block returns a true value, the element is kept. In scalar context, grep does not return a list of kept elements, but a count.

Your grep block is {!$args->{$k}}. This returns true when $args->{$k} is false and vice versa. The result does not depend on $_, and therefore doesn't check if the argument is one of the allowed values.

To see if the given value is allowed value, you'll have to test for some form of equivalence, e.g.

if (grep { $args->{$k} eq $_ } @{ $opts->{$k}{allowed} }) {
  # this is executed when the arg matches an allowed value
} else {
  # the arg is not allowed
}

An Excursion To Smart Matching and List::MoreUtils

If you can use a perl > v10, then smart matching is available. This would express above condition as

use 5.010;

$args->{$k} ~~ $opts->{$k}{allowed}

The lengthy table of possible type combinations states that this is roughly equivalent to the grep if the arg is a scalar (string/number), and the allowed arrayref holds only normal scalars as well.

However, smart matching was re-marked as experimantal in v18, and behaviour will likely change soon.

In the meantime, it might be better to stick to explicit grep etc. But we could implement two improvements:

  1. The grep will test all elements, even when a match was already found. This can be inefficient. The first function from List::Util core module has the same syntax as grep, but stops after the first element. If the block matches a value, this value is returned. If no value matches, it returns undef. This makes things complicated when undef might be a valid value, or even when false values may be allowed. But in your case, the grep could be replaced by

    use List::Util 'first';
    
    defined first { $_ eq $args->{$k} } @{ $opts->{$k}{allowed} }
    

    The List::MoreUtils module has even more functionality. It provides for example the any function, which corresponds to the mathematical ∃ (there exists) quantifier:

    use List::MoreUtils 'any';
    
    any { $_ eq $args->{$k} } @{ $opts->{$k}{allowed} }
    

    This only returns a boolean value. While it may not be as efficient as a plain grep or first, using any is quite self-documenting, and easier to use.

  2. Until now, I have assumed that we'll only ever do string comparision to the allowed values. This sometimes works, but it would be better to specify an explicit mode. For example

    croak qq(Value for "$k": "$args->{$k}" not allowed) unless
         $opts->{$k}{mode} eq 'str'   and any { $args->{$k} eq $_ } @{ $opts->{$k}{allowed} }
      or $opts->{$k}{mode} eq 'like'  and any { $args->{$k} =~ $_ } @{ $opts->{$k}{allowed} }
      or $opts->{$k}{mode} eq 'num'   and any { $args->{$k} == $_ } @{ $opts->{$k}{allowed} }
      or $opts->{$k}{mode} eq 'smart' and any { $args->{$k} ~~ $_ } @{ $opts->{$k}{allowed} }
      or $opts->{$k}{mode} eq 'code'  and any { $args->{$k}->($_) } @{ $opts->{$k}{allowed} };
    

Preventing unknown options

You may or may not want to forbid unknown options in your $args hash. Especially if you consider composability of classes, you may want to ignore unknown options, as a superclass or subclass may need these.

But if you choose to check for wrong options, you could delete those elements you already handled:

my $self = {};
for my $k (keys %$opts) {
  my $v = delete $args->{$k};
  ...; # use $v in the rest of the loop
  $self->{$k} = $v;
}

croak "Unknown arguments (" . (join ", ", keys %$args) . ") are forbidden" if keys %$args;

or grep for unknown args:

my @unknown = grep { not exists $opts->{$_} } keys %$args;
croak "Unknown arguments (" . (join ", ", @unknown) . ") are forbidden" if @unknown;

for my $k (keys %$opts) {
  ...;
}

or you could loop over the combined keys of $args and $opts:

use List::Util 'uniq';

for my $k (uniq keys(%$opts), keys(%$args)) {
  croak "Unknown argument $k" unless exists $opts->{$k};
  ...;
}

Scalar Context

I have assumed that you correctly initialized $args as a hash reference:

my $args = { file => 'file.txt', type => 'foo', head => 1 };

Using parens instead of curlies is syntactically valid:

my $args = ( file => 'file.txt', type => 'foo', head => 1 );

but this doesn't produce a hash. Instead, the => and , behave like the comma operator in C: the left operand is evaluated and discarded. That is, only the last element is kept:

my $args = 1; # equivalent to above snippet.
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top