¿Está usando ELSE mala programación? [cerrado]

18

A menudo me he encontrado con errores causados ​​por el uso de la ELSEconstrucción. Un buen ejemplo es algo similar a:

If (passwordCheck() == false){
    displayMessage();
}else{
    letThemIn();
}

Para mí esto grita problema de seguridad. Sé que es probable que passwordCheck sea booleano, pero no pondría la seguridad de mis aplicaciones en él. ¿Qué pasaría si es una cadena, int, etc.?

Por lo general, trato de evitar el uso ELSE, y en su lugar opto por dos declaraciones IF completamente separadas para probar lo que espero. Cualquier otra cosa se ignora O se maneja específicamente.

Seguramente esta es una mejor manera de evitar errores / problemas de seguridad que ingresan a su aplicación.

¿Como lo hacen ustedes?

billy.bob
fuente
3
¿Cuál es el problema de seguridad para ti? ¿Qué significa "passwordCheck"? Hubo una comprobación de contraseña? Tiene que haber una verificación de contraseña? El usuario ha pasado? ¿El usuario no ha podido ingresar la contraseña correcta?
LennyProgrammers
20
I know that passwordCheck is likely to be a boolean...¿Qué quieres decir? En cualquier lenguaje de tipo fuerte. passwordCheckserá lo que quieras que sea.
Bobby
31
Creo que las prácticas de sangría malos conducen a más errores que el uso de elsedeclaraciones ...
gablin
15
Esto parece extraño Primero, se queja de que el posible tipo de devolución passwordCheck()posiblemente no sea booleano (lo que puede ser una preocupación razonable), y luego lo culpa else. No veo qué problemas las elsecausas.
David Thornley
8
mmm, creo que preguntar si usar otra cosa es mala programación es mala programación
Muad'Dib

Respuestas:

90

El elsebloque siempre debe consistir en lo que desea que sea el comportamiento predeterminado.

No hay necesidad de evitarlos, solo tenga cuidado de usarlos adecuadamente.

En su ejemplo, el estado predeterminado debería ser no permitir el acceso. Un poco de refactorización te deja con:

If (passwordCheck)
{
   letThemIn();
}
else
{
   displayMessage();
}

es decir, si la verificación de contraseña funciona, déjelos entrar, de lo contrario siempre es válido mostrar algún mensaje de error.

Por supuesto, puede agregar verificaciones adicionales a su lógica mediante el uso else ifde ifdeclaraciones en lugar de declaraciones completamente separadas .

RYFN
fuente
2
@ dave.b en el contexto del ejemplo, supongo que sería un problema de seguridad, pero si esto está por todas partes en la base de código que está viendo, es más una señal de que quien lo escribió necesita un poco más práctica :)
RYFN
99
¿Alguien puede dar más detalles sobre el aspecto de seguridad de esto? if (! something) {do a} else {do ​​b} versus if (something) {do b} else {do ​​a} es lógicamente equivalente no? Estoy tratando de entender cuál es la diferencia en términos de seguridad de esto.
Chris
11
@ Chris: Creo que el OP utiliza un lenguaje débilmente tipado. Por lo tanto, passwordCheckpodría ser cualquier cosa, Fe null, lo que haría passwordCheck == falseque falsey sería permitir que el usuario de inicio de sesión a causa de un error interno.
Bobby
1
@ Chris, entendí que el estado predeterminado era permitir el acceso, lo cual no es necesariamente aconsejable.
RYFN
3
Sí, esto depende mucho del idioma. En C #, donde se ifrequiere a bool, las variables deben asignarse definitivamente, todas las rutas deben devolver un valor, etc. No se me ocurre ninguna razón por la que el orden ify la elseimportancia sean importantes. Es decir, if(a){b();}{c();}debería ser equivalente a if(!a){c();{b();}. En JavaScript, por otro lado, debes tener en cuenta que passwordCheckpodría ser undefined, etc.
Tim Goodman
57

No, no hay nada de malo ELSE. ELSENo es lo nuevo GOTO. De hecho, usar dos IFs en lugar de ELSEpuede generar varios problemas.

Ejemplo uno:

if (this(is(a(very())==complex(check())))) {
   doSomething();
}

if (!this(is(a(very())==complex(check())))) {
   doTheOtherThing();
}

¿Ves el copiar y pegar? Solo espera el día en que cambies uno y olvides el otro.

Ejemplo dos:

foreach(x in y) {
  if (first_time) {
    first_time = false;
    doSomething();
  }

  if (!first_time) {
    doTheOtherThing();
  }
}

Como puede ver, el segundo IFtambién se ejecutará para el primer elemento, porque la condición ya ha cambiado. En los programas del mundo real, estos errores son más difíciles de detectar.

usuario281377
fuente
12
Estoy de acuerdo con ésto. Copiar y pegar una ifdeclaración e invertir su expresión booleana solo para evitar una else, ¡ eso es una mala programación! No sólo está duplicando código ( mala programación), que está también ralentizar el rendimiento (si el cheque es realmente complicado, que está ahora haciendo dos veces - ba-- eh, bueno, ya sabes lo que dicen de optimizaciones prematuros hoy en día ... ¡ no tan buena programación!).
gablin
Para ser honesto, si el cheque debe estar en un método propio para devolver verdadero / falso
billy.bob
Buenos ejemplos, no olvidemos el rendimiento de usar 2 si se comprueba versus uno si se combina con otro. Supongo que, en la mayoría de los idiomas, usar un if / else funcionará mejor que usar dos if si las declaraciones con uno no están en la condición.
Chris
1
dave.b: claro, pero podría comenzar de manera simple y crecer lentamente; La comprobación compleja en mi ejemplo está aquí para hacer que el problema sea más obvio.
user281377
3
Sí, y no olvide que las condiciones pueden tener efectos secundarios en la mayoría de los idiomas, y si hay funciones en las condiciones que realmente no puede distinguir al mirar.
David Thornley
18

Siempre hay un ELSE. Si tú escribes

if(foo)
  bar();

en realidad escribes

if(foo)
{
  bar();
}
else
{
   // do nothing
}

Cualquier cosa que ponga en el camino de ELSE es su responsabilidad.

LennyProgrammers
fuente
77
-1. Esta respuesta es demasiado ridícula. No digo que no sea cierto. Simplemente se pierde el punto. Lo que la compilación genera a partir de su código no tiene nada que ver con las prácticas de codificación y los insectos que escribe
3
La pregunta era "¿Está usando ELSE una mala programación?". Mi respuesta es: puedes fingir que no está allí, pero no evitarlo.
LennyProgrammers
55
que lenguaje ? en Java, el resto no está en el
código de bytes
@ acidzombie24: es una muy buena práctica considerar cuál es el 'otro' de cualquier pregunta. A veces es solo - haz todo lo demás en la función, pero muestra que has pensado en ello
Martin Beckett
14

Personalmente, tiendo a evitar elsetodo lo que pueda, pero no es por ningún problema de seguridad .

Al leer el código, las declaraciones anidadas hacen que sea más difícil seguir la lógica, porque debe recordar qué conjunto de condiciones conducirá allí. Por esta razón, soy un gran fanático de la salida anticipada:

if (checkPassword != OK) { displayMessage(); return; }

letThemIn();

Esto también se aplica a las fory whilelos bucles en el que voy a utilizar continue, y breakcada vez que se evita un nivel de sangría.

Chris Lattner lo dice mejor que yo en los estándares de codificación LLVM .

Matthieu M.
fuente
Estoy de acuerdo. "else" crea un "fork" mental y los humanos somos criaturas secuenciales.
user187291
Fyi, se llama condición de guardia
CaffGeek
@ Chad: ah, gracias, siempre es bueno tener nombre para las cosas :)
Matthieu M.
1
+1. Esta es la única respuesta que puedo soportar que tiene una puntuación de> 1. *writes an answer*
No trato de evitar otra cosa como tal, pero creo que estoy de acuerdo con lo que escribes. El punto es que lo que está probando debe ser la excepción y no el caso normal ( stackoverflow.com/questions/114342/… ). Aquí la falla de autenticación es la excepción y (solo) que debe probarse.
hlovdal
5

Entonces solo reemplácelos,

If (passwordCheck == true)
{
     letThemIn();
}
else
{
     displayMessage();
}
Ahmet Kakıcı
fuente
21
¿Qué tal If ((passwordCheck == true) == true)? :-)
Hippo
1
Bueno, es mi estilo, mejorar la legibilidad. Puede escribir if (!
Value
77
No mejora la legibilidad. Solo agrega ruido. Peor aún son los programadores que escriben construcciones anidadas solo porque tienen miedo de los operadores booleanos.
ak2
3
Si el nombre de la variable es descriptivo, no hay razón para ello: if (someBooleanValue == true) debería ser necesario. Esto sería mejor: if (validPassword) {...
Mark Freedman
Bueno, acabo de reemplazar los bloques de código dentro de la pregunta. Si leyó mi primer comentario, dije que prefiero usar if (value! = True) en lugar de if (! Value). Espero que pueda ver la diferencia entre if (value) y if (! Value). Quise decir que no uso el operador de complemento. De lo contrario tienes razón, verdadero significa verdadero.
Ahmet Kakıcı
3

Al igual que Matthieu M., prefiero la salida temprana a los bloques más anidados ... Ilustra una buena programación defensiva (si hay malas condiciones, no tiene sentido continuar). Mucha gente no estará de acuerdo con nosotros, prefiriendo un punto de salida único; No es el punto del debate (creo).

Ahora, ciertamente lo uso elsecuando tiene sentido, particularmente para alternativas simples y cortas. Como se dijo, duplicar la prueba es una pérdida de tiempo (programador y CPU), una fuente de confusión y, más tarde, de errores (cuando se cambia uno, no el otro).
En algún momento, agrego un comentario en la elseparte, recordando cuál era la condición (particularmente si la ifparte es larga, por ejemplo, en código heredado) o cuál es la alternativa.

Tenga en cuenta que algunos defensores extremos de la programación funcional proponen deshacerse por completo if, a favor de la coincidencia de patrones ... Un poco demasiado extremo para mi gusto. :-)

PhiLho
fuente
1
como nota al margen: si tiene una construcción if en su lenguaje funcional puro, entonces realmente necesita tener otra cosa. ¡Cada expresión tiene que devolver algo!
tokland
2

No hay nada de malo en usar ELSE. Sin embargo, puede conducir a un código demasiado complejo que es difícil de leer y comprender. Puede indicar un mal diseño. Ciertamente indica casos de uso adicionales que deberán ser probados.

Intenta eliminar ELSEs si puedes, pero no seas paranoico al respecto. Steve McConnell llama a este código de línea recta en Code Complete. Es decir, hay un camino simple y claro a través de su código.

Enfoques para tratar su problema particular:

  • usar polimorfismo En el límite de su sistema, valide las credenciales de seguridad del usuario. Si son legítimos, devuelva un objeto de sesión, con acceso a las partes relevantes del sistema o arroje una excepción. Sin embargo, esto podría hacer que su sistema sea más complejo. Entonces usted decide qué es más fácil de entender y mantener.

En general, lo siguiente puede ayudar a reducir ELSE en su código:

  • Los buenos requisitos pueden reducir la necesidad de tales decisiones en el código. Es posible que no necesite implementar el caso de uso (de lo contrario) en absoluto.
  • Diseño más claro. Máxima cohesión y minimizar el acoplamiento. Esto garantiza que los componentes no estén duplicando decisiones tomadas en otros componentes.
  • manejo de excepciones para gestionar casos de error.
  • polimorfismo (ver ejemplo arriba).
  • declaraciones de cambio: estos son ELSE glorificados pero son mejores en ciertas situaciones.
Conor
fuente
2
También incluiría "Mover cheques booleanos complejos a una función separada".
gablin
2

Su suposición acerca de que el código es una fuga de seguridad puede o no ser cierta dependiendo del idioma que esté utilizando. En el código C podría ser un problema (particularmente porque en C un booleano es solo un int que no es cero o cero), pero en la mayoría de los lenguajes fuertemente tipados (es decir, la verificación del tipo de tiempo de ejecución) si la passwordCheckvariable se declara como booleana, no hay forma de asignarle algo más. De hecho, todo en un ifpredicado debe resolverse en un valor booleano, ya sea que use los operadores booleanos o simplemente use el valor. Si logras tener otro tipo de objeto vinculado al passwordChecktiempo de ejecución, arrojarás algún tipo de excepción de lanzamiento ilegal.

Las construcciones if / else simples son mucho más fáciles de leer que las construcciones if / if, y menos propensas a problemas inadvertidos si alguien intenta voltear la construcción. Tomemos el mismo ejemplo por un segundo:

if(passwordCheck == false) {
    denyAccess();
}

if(passwordCheck) {
    letThemIn();
}

Se pierde el significado de las cláusulas mutuamente excluyentes que desea ejecutar arriba. Eso es lo que transmite la construcción if / else. Dos ramas de ejecución mutuamente excluyentes, donde una de ellas siempre se ejecutará. Esta es una parte importante de la seguridad: garantizar que no haya forma de hacerlo letThemIndespués de haber llamado denyAccess.

A los efectos de la claridad del código y para garantizar que las secciones críticas estén más protegidas, deben estar dentro de la cláusula primaria (la ifparte). El comportamiento no conforme predeterminado debe estar en la cláusula alternativa (la elseparte). Por ejemplo:

if(passwordCheck) {
    letThemIn();
} else {
    denyAccess();
}

NOTA: al trabajar con diferentes idiomas, he desarrollado un habbit de codificación que ayuda a evitar la pregunta "¿y si es una cadena?" Esencialmente, es poner la constante primero en la expresión booleana. Por ejemplo, en lugar de verificar passwordCheck == false, estoy revisando false == passwordCheck. Esto también evita el posible problema de asignación accidental en C ++. Usando este enfoque, el compilador se quejará si escribo en =lugar de ==. En lenguajes como Java y C #, el compilador trataría la asignación en la cláusula if como un error, pero C ++ lo aceptará con gusto. Es por eso que también tiendo a hacer una verificación nula con el nullprimero.

Si cambia rutinariamente los idiomas, colocar la constante primero es muy útil. Sin embargo, en mi equipo es opuesto al estándar de codificación y el compilador detecta esos problemas de todos modos. Puede ser un hábitat difícil de romper.

Berin Loritsch
fuente
1

Decir que usar elsecuando programar es malo es como decir que usar otherwisecuando hablar es malo.

Claro, ambos se pueden usar de mala manera, pero eso no significa que se deben evitar solo porque cometió un error que los incluyó. No me sorprendería si muchos errores dependieran de un defaultcaso perdido en una switchdeclaración.

gablin
fuente
1

Piense Elseen una lista blanca del flujo de su aplicación. Verifica las condiciones que DEBEN permitir que el flujo de la aplicación continúe, y si no se cumplen, entonces Elsese ejecuta para resolver el problema, suspender la ejecución de la aplicación o algo similar.

Else en sí mismo no es malo, pero si lo usa mal, puede ver efectos no deseados.

Además, con respecto a su declaración sobre

"Sé que es probable que PasswordCheck sea booleano, pero no pondría la seguridad de mis aplicaciones en él".

Para los métodos que desarrolle, SIEMPRE devuelva un tipo de datos. Aunque PHP Core está lleno de código que devuelve dos o más tipos de datos, esta es una mala práctica ya que hace conjeturas de las llamadas a funciones. Si tiene que devolver más de un tipo de datos, considere lanzar una excepción (creo que esta es a menudo la razón por la que me gustaría devolver otro tipo de datos, algo salió horrible, terriblemente mal), o considere reestructurar su código para que pueda devuelve solo un tipo de datos.

Craige
fuente
¡No sabía que había idiomas que devolvían más de un tipo de datos! ¿Te refieres a funciones polimórficas? Creo que cada vez que sobrecargo una función siempre ha devuelto el mismo tipo de datos, aunque puede tomar diferentes argumentos.
Michael K
1
En lenguajes poco escritos, y especialmente en PHP, las funciones pueden devolver más de un tipo de datos. por ejemplo: stristr- "Devuelve la subcadena coincidente. Si no se encuentra la aguja, devuelve FALSO"
Craige
@ Michael, una función PHP puede devolver cualquier cosa que desee. No hay restricción en el tipo de datos. Mi función más complicada devuelve verdadero / falso / nulo, pero no hay nada (excepto el sentido común) que le impida escribir una función que devuelva verdadero / entero / nulo / cadena / flotante / matriz.
TRiG
Interesante. Nunca he trabajado con PHP. Sin embargo, gracias por la explicación, ¡probablemente me ayuden en el futuro! algo así como Javascript entonces?
Michael K
@Michael: bastante similar a Javascript de hecho.
Craige
1

Ante todo. Jajaja NO HAY RAZÓN para evitar otra cosa, en absoluto. NO es una mala práctica de ninguna manera o forma.

En todo caso, el código debería ser

if(!IsLoggedIn) { ShowBadLoginOrNoAccessPage(); return }

No hay dos si hay y no tiene más. Esto es lo que hago en todas mis aplicaciones, excepto en una en la que lanzo una excepción. La excepción está atrapada en mi función que verifica la url para que se muestre la página adecuada (o, alternativamente, puedo poner la función de error catch / check en asp.net). Imprime una página genérica que dice no autorizar o cualquier mensaje que use en la excepción (siempre verifico el tipo de excepción y establezco el código de estado http).

-Editar- como se muestra en el ejemplo de ammoQ dos ifs es ridículo. Realmente lo demás es tan bueno o mejor que un if. Si hay algo que se debe evitar (aunque personalmente no lo hago. Pero uso return and break much), como se ha dicho, más rutas de código aumentan la probabilidad de errores. Ver complejidad ciclomática

-Editar 2- Si te preocupa el uso de if / else. También notaré que mi preferencia es poner el bloque de código más corto en la parte superior, como

if(cond == false) {
    a()
    b()
    c()
    onetwothree()
}
else
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}

En lugar

if(cond) 
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}
else
{
    a()
    b()
    c()
    onetwothree()
}

fuente
0

Me gusta establecer un valor predeterminado antes de condicionales cuando puedo. Siento que es un poco más fácil de leer y un poco más explícito, pero esto es solo una preferencia. Tengo una tendencia a tratar de evitar condiciones negativas en mi código. No soy un gran admirador de buscar! Foo o false == foo y siento que más es el equivalente condicional de un negativo.

foo = bar;

if ('fubar' == baz) {
    foo = baz;
}

en lugar de ...

if ('fubar' == baz) {
    foo = baz;
} else {
    foo = bar;
}

El bloque de código anterior me parece un poco más fácil de leer. Me parece más natural tener una especie de paranoia escéptica sobre mi código. Establecer un valor predeterminado independientemente de cualquier condición me hace sentir cómodo: P


fuente
0

Yo diría que el uso de la lógica de ramificación de cualquier tipo debe evitarse en la mayor medida posible. Si bien no hay nada malo con ELSE o IF, hay muchas formas de escribir código para minimizar la necesidad de usar cualquier lógica de ramificación. No digo que la lógica de ramificación se pueda eliminar por completo, será necesaria en algunos lugares, pero es posible refactorizar el código para eliminar una buena parte de él. En la mayoría de los casos, esto mejorará la inteligibilidad y precisión de su código.

Como ejemplo, los operadores ternarios también suelen ser buenos candidatos:

If VIP Then 
  Discount = .25
Else
  Discount = 0
End If
Total = (1 - Discount) * Total

Usando un enfoque ternario:

Discount = VIP ? .25 : 0
Total = (1 - Discount) * Total

Los operadores ternarios desplazan la ramificación a la derecha de una buena manera.

Mario T. Lanza
fuente