Hi Alan

You are unpacking `@_` in a way, but perlcritic doesn't recognise doing it this 
way.

I think you'd be better off without dereferencing the hash, and using a slice 
to assign your local variables. I would write your subroutine like this

    sub modrec {
        my ($args) = @_;

        my ($fn, $ar, $ad, $dr) = @{$args}{qw/ FN AR AD DR /};
        ...
    }

And if you must, you can dereference the array with

    my @dr = @$dr;

but it's generally better to access the elements directly from the reference, 
with `$dr->[0]`, `$dr->[1]` etc.

Rob


       

On 15 January 2017 20:09:53 GMT+00:00, al...@myfastmail.com wrote:
>Hi,
>
>I have a simple script with a subroutine that I pass scalar & array
>arguments to,
>
>       #!/usr/bin/perl
>
>       use 5.01201;
>       use strict;
>       use warnings;
>
>       my $this_fn = "input.txt";
>       my @this_dr = qw(
>         /path/1
>         /path/2
>       );
>       my $this_rn = "recname";
>       my $this_ad = "1.2.3.4.";
>
>       sub modrec {
>               my %args = %{ shift @_ };
>
>               my $fn = $args{FN};
>               my $ar = $args{AR};
>               my $ad = $args{AD};
>               my @dr = @{$args{DR}};
>
>               return;
>       }
>
>       modrec (
>        {
>          FN=>$this_fn,
>          DR=>\@this_dr,
>          AR=>$this_rn,
>          AD=>$this_ad,
>        }
>       );
>
>The script *executes* just fine.
>
>But when I exec perlcritic on it
>
>       perlcritic --verbose 11 -harsh test.pl
>               Always unpack @_ first at line 15, near 'sub modrec {'.
>                 Subroutines::RequireArgUnpacking (Severity: 4)
>                   Subroutines that use `@_' directly instead of unpacking the
>arguments to
>                   local variables first have two major problems. First, they 
> are
>very hard
>                   to read. If you're going to refer to your variables by 
> number
>instead of
>                   by name, you may as well be writing assembler code! Second, 
> `@_'
>                   contains aliases to the original variables! If you modify 
> the
>contents
>                   of a `@_' entry, then you are modifying the variable 
> outside of
>your
>                   subroutine. For example:
>
>                      sub print_local_var_plus_one {
>                          my ($var) = @_;
>                          print ++$var;
>                      }
>                      sub print_var_plus_one {
>                          print ++$_[0];
>                      }
>
>                      my $x = 2;
>                      print_local_var_plus_one($x); # prints "3", $x is still 2
>                      print_var_plus_one($x);       # prints "3", $x is now 3 !
>                      print $x;                     # prints "3"
>
>                   This is spooky action-at-a-distance and is very hard to 
> debug if
>it's
>                   not intentional and well-documented (like `chop' or 
> `chomp').
>
>                   An exception is made for the usual delegation idiom
>                   `$object->SUPER::something( @_ )'. Only `SUPER::' and 
> `NEXT::'
>are
>                   recognized (though this is configurable) and the argument 
> list
>for the
>                   delegate must consist only of `( @_ )'.
>
>What's wrong with the way I'm unpacking the arguments passed to the
>subroutine,
>
>       my %args = %{ shift @_ };
>
>Is there a different, recommended way?
>
>AJ
>
>-- 
>To unsubscribe, e-mail: beginners-unsubscr...@perl.org
>For additional commands, e-mail: beginners-h...@perl.org
>http://learn.perl.org/

-- 
Sent from Kaiten Mail. Please excuse my brevity.

Reply via email to