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.