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

perl-critic

Question

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

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

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

Así es como siempre lo he hecho y, como se ha discutido tanto en PerlMonks como en Stack Overflow, tampoco es necesariamente malo .

Cambiando el fragmento de código de arriba para ...

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

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

... funciona también, pero me resulta más difícil de leer. También leí el libro Perl Best Practices de Damian Conway y no comprendo realmente cómo mi enfoque preferido para desempaquetar cae en su sugerencia de evitar el uso de @_ directamente, como lo indica Perl::Critic . Siempre he tenido la impresión de que Conway estaba hablando de cosas desagradables como:

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

El ejemplo anterior es malo y difícil de leer, y nunca consideraría escribir eso en un código de producción.

Entonces, en resumen, ¿por qué Perl::Critic considera mi manera preferida mala? ¿Realmente estoy cometiendo un crimen atroz desempaquetando usando shift?

¿Sería esto algo que la gente, aparte de mí mismo, debería plantearse con los mantenedores de Perl::Critic ?

Answer 1

¡Creo que generalmente debes evitar el cambio, si no es realmente necesario!

Simplemente se topó 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, existe la posibilidad de que cambie accidentalmente el orden de los turnos o tal vez se salte uno y todo salga hacia el sur. Además, es difícil de leer, porque no puedes estar seguro de que realmente veas todos los parámetros para el subcomité, porque algunas de las líneas a continuación podrían ser otro cambio en algún lugar ... Y si usas algunos Regex entre ellos, podrían reemplazar el contenido de $ _y cosas raras comienzan a suceder ...

Un beneficio directo de usar el desempaquetar mi (...) = @_ es que puedes copiar la parte (...) y pegarla donde llames al método y tener una buena firma :) incluso puedes usar la misma variable - ¡Nombra de antemano y no tienes que cambiar nada!

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

Answer 2

Ejecutar perlcritic con --verbose 11 explica las políticas. Sin embargo, no parece que ninguna de estas explicaciones se aplique a ti.

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 3

En algunos casos, Perl :: Critic no puede imponer las pautas de PBP con precisión, por lo que puede imponer una aproximación que intente ajustarse al espíritu de las pautas de Conway. Y es totalmente posible que hayamos malinterpretado o aplicado incorrectamente el PBP. Si encuentra algo que no huele bien, envíe un informe de error a [email protected] y lo analizaremos de inmediato.

Gracias,

-Jeff

Answer 4

Es importante recordar que muchas de las cosas que se encuentran en las Mejores Prácticas de Perl son solo la opinión de un hombre sobre lo que se ve mejor o más fácil de trabajar, y no importa si lo hace de otra manera. Damian lo dice en el texto introductorio al libro. Eso no quiere decir que todo sea así: hay muchas cosas que son absolutamente esenciales: el uso strict , por ejemplo.

Por lo tanto, a medida que escribe su código, debe decidir cuáles serán sus mejores prácticas, y usar PBP es un punto de partida tan bueno como cualquier otro. Entonces mantente consistente con tus propios estándares.

Trato de seguir la mayor parte de las cosas en PBP, pero Damian puede tener mi shift argumento de subrutina y mi, a unless cuando los saque de mis dedos fríos y muertos.

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

Answer 5

La respuesta simple es que Perl :: Critic no está siguiendo a PBP aquí. El libro afirma explícitamente que el cambio de idioma no solo es aceptable, sino que en realidad se prefiere en algunos casos.