PHP - Declaraciones de IF anidadas

Question

Me pregunto cuando es una mala idea utilizar múltiples declaraciones IF anidadas.

por ejemplo:

function change_password($email, $password, $new_password, $confirm_new_password) 
{ 
    if($email && $password && $new_password && $confirm_new_password) 
    { 
     if($new_password == $confirm_new_password) 
     { 
      if(login($email, $password)) 
      { 
       if(set_password($email, $new_password)) 
       { 
        return TRUE; 
       } 
      } 
     } 
    } 
}  

Esta función se utiliza de esta manera:

if(!change_password($email, $password, $new_password, $confirm_new_password) 
{ 
    echo 'The form was not filled in correctly!'; 
    exit; 
} 

que llamo todas mis funciones como esta, y me pregunto si hay algo mal con mi estilo de codificación. Tengo mis dudas porque si sigo este diseño, eso significa que cada función que escribo estará simplemente anidada con IF, verificando si hay errores en cada etapa. ¿Es esto lo que hacen otras personas?

No veo muchos otros guiones escritos de esta manera, con los IF anidados formando un triángulo y teniendo solo el resultado deseado en el medio. Si no se llega al medio, entonces algo se arruinó.

¿Es esta una buena estructura de funciones?

Answer 1

Anidar demasiado profundamente es generalmente una mala idea, es una lógica de spaghetti y difícil de seguir. Como cada uno de sus pasos de verificación depende de la etapa anterior de haber tenido éxito, no anidan en absoluto - sólo rescatar a una etapa cuando falla:

function change_password(blah blah blah) { 
    if (!$condition1) { 
     return false; 
    } 
    if (!$condition2) { 
     return false; 
    } 
    etc.... 


    // got here, must have succeeded 
    return true; 
} 

Eso hace que sea explícitamente claro cuál es la secuencia lógica.

Answer 2

creo que es sin duda bien legible y puede ser fácilmente entendido en comparación con el uso de un solo if declaración como

if (blah and blah and blah and blah and blah and blah and blah) {} 

Sin embargo todavía preferiría hacerlo de esta manera - demasiada sangría puede conseguir un poco molesto :

function change_password($email, $password, $new_password, $confirm_new_password) 
{ 
    if (!$email || !$password || !$new_password || !$confirm_new_password) return false; 
    if ($new_password != $confirm_new_password) return false; 
    if (!login($email, $password)) return false; 
    if (!set_password($email, $new_password)) return false; 

    return true; 
} 

Answer 3

Puede ser bueno anidarlos, porque al cambiar el orden puede evitar hacer comparaciones adicionales. Lo que está haciendo ahora se ve bien, sin embargo, su función sería menos eficiente si en vez escribió como:

function change_password($email, $password, $new_password, $confirm_new_password) 
{ 
    if($new_password == $confirm_new_password && $email && $password && $new_password && $confirm_new_password) 
    { 
     if(login($email, $password)) 
     { 
      if(set_password($email, $new_password)) 
      { 
       return TRUE; 
      } 
     } 

    } 
} 

Si $ new_password == $ confirm_new_password es cierto, pero $ correo electrónico está vacío, se le han hecho una comparación extra

Como han dicho otros, hay otras maneras de hacerlo sin anidar todo, que será funcionalmente equivalente.