2009-07-28 17 views
6

Estoy haciendo algo de Perl y ver mis declaraciones "si" anidadas me está volviendo loco. Logré reducir algunos de ellos con bloques de guardia en otra sección, pero estoy atrapado aquí.¿Debo evitar declaraciones anidadas si en Perl?

¿Cree que puedo dejar el código como está, o existe una forma "adecuada" de refactorizar lo siguiente? (También reconozco que soy relativamente nuevo en Perl)

Esto es realmente una subrutina que solicita la entrada del usuario en cada parámetro de una lista (archivo externo). $ [3] es el patrón coincidente, $ [2] es el valor predeterminado para el parámetro considerado (NULL si no hay ninguno), $ _ [1] especifica si es obligatorio o no. la instrucción 'siguiente' se refiere al siguiente parámetro leído (while loop).

Con la ayuda de todos (gracias!), Aquí está la última versión.

100   if ($input ne '' && ($input !~ $match || $input =~ /'.+'/)) { 
101    print "! Format not respected. Match : /$match/ (without \' \')\n"; 
102    next; 
103   } 
104   if ($input eq '') { 
105    if ($default eq 'NULL') { 
106     if ($manda eq 'y') { 
107      print "! Mandatory parameter not filled in\n"; 
108      next; 
109     } 
110     print "+ Ignoring parameter.\n"; 
111     $input = ''; 
112    } 
113    else { 
114     print "+ Using default value\n"; 
115     $input = $default; 
116    } 
117   } 

98  if($input eq ''){ 
99    if($_[2] eq 'NULL'){ 
100     if($_[1] eq 'y'){ 
101      print "! Mandatory parameter not filled in\n"; 
102      next; 
103     } 
104     else{ 
105      print "+ Ignoring parameter.\n"; 
106      $input = ''; 
107     } 
108    } 
109    else{ 
110     print "+ Using default value\n"; 
111     $input = $_[2]; 
112    } 
113   } 
114   elsif($input !~ $_[3] || $input =~ /'.+'/){ 
115     print "! Format not respected. Match : /$_[3]/ (without \' \')\n"; 
116     next; 
117    } 
118   } 
+1

¿Qué es específicamente lo que te está volviendo loco acerca de esos if anidados? – innaM

+0

No sé, solo siento que hay otra forma de escribir eso ... –

Respuesta

10

Aquí es una versión ligeramente más legible del caos respuesta:

# Set sane variable names... 
my ($is_required, $default, $pattern) = @_ 

# Convert the external string convention for simpler evaluation... 
$default = undef if $default eq 'NULL' 

# Refuse problematic input before going any further... 
if ($input ne '' && $input !~ $pattern || $input =~ /'.+'/) { 
    print "! Format not respected. Match : /$pattern/ (without \' \')\n"; 
    next; 
} 


# If there was no input string... 
if($input eq '') { 

    # Set the default, if one was provided... 
    if($default) { 
     print "+ Using default value\n"; 
     $input = $default; 
    } 
    # otherwise, complain if a default was required... 
    else { 
     if($is_required eq 'y'){ 
      print "! Mandatory parameter not filled in\n"; 
      next; 
     } 
     print "+ Ignoring parameter (no input or default provided).\n"; 
    } 
} 

Los puntos clave son:

  • No es necesario demás si va a salir del bucle actual con siguiente
  • casos excepcionales se deben manejar primero
  • Puede mejorar en gran medida la legibilidad mediante el uso de variables con nombre
+0

Gracias por eso. Nuestras respuestas se cruzaron (ver mi primera publicación). Por cierto, en mi código, $ is_required obliga al usuario a completarlo, no es una queja: p (Sin embargo, no cambia nada y no ha leído toda la subrutina) –

+0

Disculpe la doble publicación, pero me gusta su forma de comentar ; y por "no has leído" quise decir "no he publicado". –

+0

Gracias, aunque no puedo reclamar ningún crédito. El estilo de comentario es de Damian Conway, lo adopté después de leer sus cosas porque también me gustó. ;) –

1

Si lógica requerida if anidado entonces supongo que no hay nada malo con ellos.

Sin embargo, se podría mejorar la legibilidad de su código por

  1. Usando sólo un poco más espacio en blanco y
  2. Mediante el uso de sus propias variables en vez de operar directamente sobre @_

Isn' ¿Esto es un poco más legible?

98  if ($input eq '') { 
99    if ($default eq 'NULL'){ 
100     if ($input eq 'y'){ 
101      print "! Mandatory parameter not filled in\n"; 
102      next; 
103     } 
104     else { 
105      print "+ Ignoring parameter.\n"; 
106      $input = ''; 
107     } 
108    } 
109    else { 
110     print "+ Using default value\n"; 
111     $input = $default; 
112    } 
113   } 
114   elsif ($input !~ $foo || $input =~ /'.+'/) { 
115     print "! Format not respected. Match : /$foo/ (without \' \')\n"; 
116     next; 
117    } 
118   } 
+0

Acerca del punto 1., estoy usando Perltidy de vez en cuando. Actualmente estoy tratando de resolver problemas y depurar de antemano. 2. Leí (en el libro de O'reilly y en otros lugares) que $ _ y $ @ se usaron ampliamente, sin mencionar ningún inconveniente. –

+0

1. Tratar de resolver problemas es mucho más fácil con un código fácil de leer. 2. El inconveniente es la legibilidad. Al reemplazar los diversos usos de $ _ [x], por ejemplo, inmediatamente noté que su descripción de lo que contienen esas variables no puede ser del todo correcta. – innaM

+0

1. Tienes toda la razón. Me gusta el código tal como es, pero sin duda lo mantendrán otros (primero siendo usted). 2. @_ es una matriz que contiene la lista de parámetros pasados ​​(por valor) a la función. Correcto ? En cuanto a $ _, solo lo estoy usando cuando r/w (into) filehandle. –

2

La práctica común es definir constantes para sus índices de matriz y darles nombres significativos. Tales como:

use constant MANDATORY => 1, 
      DEFAULT => 2, 
      PATTERN => 3; 
... 
if($_[DEFAULT] eq 'NULL') { 
    ... 
} 

En cuanto a la jerarquización - A menudo se debe tratar de reducir el guión (que significa mantener el nivel de anidación baja), pero nunca hacerlo a expensas de mantener el código comprensible. No tengo ningún problema con el nivel de anidación, pero esto también es solo un pequeño corte de tu código. Si realmente es un problema, puede dividir las condiciones en subrutinas separadas.

+0

Como dije, es una subrutina, todas mis variables son locales. Por lo tanto, ¿no puedo usar la declaración de "uso"? Anidar no es un problema en el resto del código, afortunadamente. Gracias por tu respuesta. –

+1

'constante de uso' tiene algunos inconvenientes. Las constantes creadas con la constante de uso no se pueden interpolar (por ejemplo, en cadenas), ya que no tienen sigilo y no tienen un alcance léxico en el tiempo de ejecución. Tienen un alcance de paquete y se generan en tiempo de compilación. Esto es un problema si tiene un conjunto constante en tiempo de ejecución, p. por una función. Esto se describe mucho mejor, y con excelentes ejemplos, en el libro 'Mejores prácticas Perl', de Damian Conway. –

0

Teniendo en cuenta que es probable que hacer un espagueti goto lo contrario, absolutamente Nº

Lo que podría ser mejor es un switch case.

+0

Aprendí mi lección: goto es HELL. ;) Nunca lo pensé. La caja del interruptor realmente no se aplica aquí, ya que estoy probando 4 variables diferentes. –

+0

¿No quieres decir 'given' /' when'? –

+0

¿No está implementado solo con 'use feature' o 'use switch' perl 6.0 ''? –

3
if($input ne '' && ($input !~ $_[3] || $input =~ /'.+'/)) { 
    print "! Format not respected. Match : /$_[3]/ (without \' \')\n"; 
    next; 
} 
if($input eq '') { 
    if($_[2] eq 'NULL') { 
     if($_[1] eq 'y'){ 
      print "! Mandatory parameter not filled in\n"; 
      next; 
     } 
     print "+ Ignoring parameter.\n"; 
     $input = ''; 
    } else { 
     print "+ Using default value\n"; 
     $input = $_[2]; 
    } 
} 
+0

¡Me gusta eso! Gracias ! (excepto el abrazado más: p) –

0

puede simplificar a lo siguiente si no le gusta todo lo demás.

if($input eq ''){ 
    $input = $_[2]; 
    $output = "+ Using default value\n"; 
    if($_[2] eq 'NULL'){ 
     $input = ''; 
     $output = "+ Ignoring parameter.\n"; 
     if($_[1] eq 'y'){ 
      $output = "! Mandatory parameter not filled in\n"; 
     } 
    } 
} 
elsif($input !~ $_[3] || $input =~ /'.+'/){ 
    $output = "! Format not respected. Match : /$_[3]/ (without \' \')\n"; 
} 
print $output; 
+0

imho, perdió algo de legibilidad. Gracias de todos modos;) –

3

La principal preocupación es mantener el código legible.

Si puede obtener un código legible con declaraciones if anidadas, continúe. Pero mantenga el sentido común activo todo el tiempo.

+0

Si tuviera que mantener mi código, ¿tendría dificultades para hacerlo? –

+0

@Isaac, no. (Pero estoy entrenado para mantener el código difícil de leer, así que no soy una buena referencia). –

-2

supongo que podría hacer combinaciones lógicas para aplanarlo:

if(($input eq '')&&($_[2] eq 'NULL')&&($_[1] eq 'y')){ 
    print "! Mandatory parameter not filled in\n"; 
    next; 
} 

elsif(($input eq '')&&($_[2] eq 'NULL')){ 
    print "+ Ignoring parameter.\n"; 
    $input = ''; 
} 

elsif($input eq ''){ 
    print "+ Using default value\n"; 
    $input = $_[2]; 
    next; 
} 


elsif($input !~ $_[3] || $input =~ /'.+'/){ 
    print "! Format not respected. Match : /$_[3]/ (without \' \')\n"; 
} 

print $output; 

entonces se podría usar un interruptor para que sea un poco mejor.

+0

Esta en realidad era mi versión anterior, sin embargo, las pruebas "&&" no me satisfacían. –

+0

Esto disminuye la legibilidad y la velocidad. – Imagist

+0

Esto ni siquiera es Perl. Perl no tiene 'elseif' solo' elsif'. –

0

Creo que la razón principal (si no solo) de las preocupaciones relativas a la anidación es la complejidad del algoritmo. En los otros casos, debe preocuparse por la legibilidad y la capacidad de mantenimiento, que se puede abordar con comentarios y sangría de propoer.

siempre encuentro un buen ejercicio de maintanability para leer el código antiguo de la mina, no sólo para comentarios sobre la legibilidad, sino también en la técnica ...

4

Un enfoque alternativo que a veces ayuda a la legibilidad es poner algunos o todos de las ramas en referencias de código bien nombradas. Aquí está el comienzo de la idea:

$format_not_respected = sub { 
    return 0 if ...; 
    print "! Format not respected...."; 
    return 1; 
} 
$missing_mandatory_param = sub { 
    return 0 if ...; 
    print "! Mandatory parameter not filled in\n"; 
    return 1; 
} 

next if $format_not_respected->(); 
next if $missing_mandatory_param->(); 
etc... 
+0

¡No sabía nada de eso, gracias! Incluso si en mi caso, ya estoy en una subrutina (anidado si o sub-sub-rutina, esa es la pregunta), eso podría ser útil para otras secuencias de comandos. –

+0

Eso va a compilar un par de suscriptores cada vez que llame a esta función: ¿eso es realmente * lo que quiere hacer? Es una idea clara, y te di una oportunidad, pero si esto se va a llamar mucho, es mejor no usar el comportamiento de captura. (Creo.) – Axeman

Cuestiones relacionadas