¿Por qué a Perl :: Critic no le gusta usar shift para poblar variables de subrutina?

Question

Últimamente, he decidido comenzar a usar Perl::Critic con más frecuencia en mi código. Después de programar en Perl por casi 7 años, me he acostumbrado a la mayoría de las mejores prácticas de Perl durante mucho tiempo, pero sé que siempre hay margen de mejora. Sin embargo, una cosa que me ha estado molestando es el hecho de que Perl::Critic no le gusta la forma en que desempaqueco @_ para las subrutinas. A modo de ejemplo:

sub my_way_to_unpack { 
    my $variable1 = shift @_; 
    my $variable2 = shift @_; 

    my $result = $variable1 + $variable2; 
    return $result; 
} 

Ésta es la forma en que siempre lo he hecho, y, como su sido discutido en ambos PerlMonks y desbordamiento de pila, su not necessarily evil tampoco.

Cambiar el código anterior a ...

sub perl_critics_way_to_unpack { 
    my ($variable1, $variable2) = @_; 

    my $result = $variable1 + $variable2; 
    return $result; 
} 

... también funciona, pero me resulta difícil de leer. También he leído el libro de Damian Conway Perl Best Practices y realmente no entiendo cómo mi enfoque preferido para desempacar cae bajo su sugerencia de evitar el uso de @_ directamente, como lo implica Perl::Critic. Siempre he tenido la impresión de que estaba hablando de Conway maldad, tales como:

sub not_unpacking { 
    my $result = $_[0] + $_[1]; 
    return $result; 
} 

El ejemplo anterior es mala y difícil de leer, y me volvería nunca jamás que no hacer que en un trozo de código de producción.

En resumen, ¿por qué el Perl::Critic considera que mi forma de preferencia es mala? ¿Estoy realmente cometiendo un crimen atroz desempaquetando usando shift?

¿Sería algo que personas que no sean yo piensen que debería plantearse con los mantenedores de Perl::Critic?

Answer 1

La respuesta simple es que Perl :: Critic no está siguiendo PBP aquí. El libro establece explícitamente que la expresión de cambio no solo es aceptable, sino que en algunos casos se prefiere .

Answer 2

Es importante recordar que muchas de las cosas en Perl Best Practices son solo la opinión de un tipo sobre lo que parece mejor o es más fácil de trabajar, y no importa si lo haces de otra manera. Damian lo dice en el texto introductorio del libro. Eso no quiere decir que es todo así - hay muchas cosas que son absolutamente esenciales: usar strict, por ejemplo.

Para escribir su código, debe decidir por sí mismo cuáles serán sus mejores prácticas, y utilizar PBP es un punto de partida tan bueno como cualquier otro. Entonces mantén coherencia con tus propios estándares.

Intento seguir la mayoría de las cosas en PBP, pero Damian puede tener mi argumento de subrutina shift sy mi unless es cuando las arranca de mis dedos fríos y muertos.

En cuanto a Critic, puede elegir qué políticas desea aplicar, e incluso crear las suyas propias si aún no existen.

Answer 3

Corriendo perlcritic con --verbose 11 explica las políticas. Sin embargo, parece que ninguna de estas explicaciones se aplica a usted.

 
Always unpack @_ first at line 1, near 
'sub xxx{ my $aaa= shift; my ($bbb,$ccc) = @_;}'. 
    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 `(@_)'. 

Answer 4

En algunos casos, Perl :: Critic no puede hacer cumplir las pautas de PBP con precisión, por lo que puede hacer cumplir una aproximación que intenta coincidir con el espíritu de las directrices de Conway. Y es muy posible que hayamos malinterpretado o aplicado erróneamente PBP. Si encuentra algo que no huele bien, envíe un informe de error a bug-perl-critic@rt.cpan.org y lo analizaremos de inmediato.

Gracias,

-Jeff

Answer 5

Creo que en general debe evitar turno, si no es realmente necesario!

acabo de encontrar con un código como este:

sub way { 
    my $file = shift; 
    if (!$file) { 
    $file = 'newfile'; 
    } 
    my $target = shift; 
    my $options = shift; 
} 

Si comienza a cambiar algo en este código, hay una buena probabilidad de que pudiera cambiar accidantially el orden de los turnos o tal vez saltarse uno y todo va Southway . Además, es difícil de leer, porque no puede estar seguro de que realmente vea todos los parámetros para el sub, porque algunas líneas a continuación podrían ser otro cambio en alguna parte ... Y si usa algunas Regexes intermedias, podrían reemplazar el contenido de $ _ y cosas raras comienzan a suceder ...

Una ventaja directa de usar el desembalaje my (...) = @_ es que puedes simplemente copiar la parte (...) y pegarla donde llames al método y tener una bonita firma :) ¡incluso puede usar los mismos nombres de variables de antemano y no tiene que cambiar nada!

Creo que shift implica operaciones de lista donde la longitud de la lista es dinámica y quiere manejar sus elementos de uno en uno o cuando necesita explícitamente una lista sin el primer elemento. Pero si solo quiere asignar toda la lista a x parámetros, su código debería decirlo con my (...) = @_; nadie tiene que preguntarse.