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, [email protected] 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: [email protected]
>For additional commands, e-mail: [email protected]
>http://learn.perl.org/
--
Sent from Kaiten Mail. Please excuse my brevity.
