Mantenibilidad de la lógica booleana: ¿es necesario anidar si se necesitan declaraciones?

11

¿Cuál de estos es mejor para la mantenibilidad?

if (byteArrayVariable != null)
    if (byteArrayVariable .Length != 0)
         //Do something with byteArrayVariable 

O

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

Prefiero leer y escribir el segundo, pero recuerdo leer en código completo que hacer cosas así es malo para la mantenibilidad.

Esto se debe a que confía en el idioma para no evaluar la segunda parte de ifsi la primera parte es falsa y no todos los idiomas lo hacen. (La segunda parte arrojará una excepción si se evalúa con un valor nulo byteArrayVariable).

No sé si eso es realmente algo de qué preocuparse o no, y me gustaría recibir comentarios generales sobre la pregunta.

Gracias.

Vaccano
fuente
1
La primera forma tiene el riesgo de
colgarse de los
Por curiosidad, ¿con qué idioma estás trabajando?
STW
@STW: utilizamos C # y tenemos un código heredado en Delphi.
Vaccano
2
bueno saber. No estoy familiarizado con Delphi pero en C # que puedo tener confianza en las &&y los ||operadores que realizan la evaluación de corto circuito. Pasar de un idioma a otro generalmente tiene algunos inconvenientes, y es bueno ser firme al hacer revisiones de código para ayudar a detectar esos pequeños problemas.
STW
Si bien junto con muchas respuestas aquí, encuentro que la segunda es más legible, la primera es mucho más fácil de depurar. En este caso, todavía usaría el segundo porque está comprobando que es fácil depurar cosas. Pero ten cuidado en general.
Magus

Respuestas:

30

Creo que la segunda forma está bien, y también representa más claramente lo que estás tratando de hacer. Tu dices eso...

Recuerdo haber leído en código completo que hacer cosas así es malo para la mantenibilidad. Esto se debe a que confía en el idioma para no evaluar la segunda parte del if si la primera parte es falsa y no todos los idiomas lo hacen.

No importa si todos los idiomas hacen eso. Estás escribiendo en un idioma en particular, por lo que solo importa si ese idioma lo hace. De lo contrario, básicamente está diciendo que no debe usar las funciones de un idioma en particular porque otros idiomas pueden no ser compatibles con esas funciones, y eso es una tontería.

mipadi
fuente
12
Convenido. ¿Por qué me importaría si mi código se ejecutará igual cuando se copia a otro idioma?
Tim Goodman el
16

Me sorprende que nadie lo haya mencionado (así que espero no haber leído mal la pregunta), ¡ pero ambos son una mierda!

La mejor alternativa sería utilizar salidas anticipadas (colocar una declaración de retorno al principio del código si no se ejecuta ningún otro código). Esto supone que su código está relativamente bien refactorizado y que cada método tiene un propósito conciso.

En ese momento harías algo como:

if ((byteArrayVariable == null) || (byteArrayVariable.Length != 0)) {
  return; // nothing to be done, leaving
}

// Conditions met, do something

Puede parecerse mucho a la validación, y eso es precisamente lo que es. Valida que se hayan cumplido las condiciones para que el método haga lo suyo: ambas opciones que ofreció ocultaron la lógica real dentro de las comprobaciones previas.

Si su código es una gran cantidad de espagueti (métodos muy largos, muchos ifs anidados con lógica dispersa entre ellos, etc.), entonces debe centrarse en el problema más grande de poner su código en forma antes de los problemas estilísticos más pequeños. Dependiendo de su entorno y la gravedad del desorden, existen algunas herramientas excelentes para ayudar (por ejemplo, Visual Studio tiene una función de refactorización de "Método de extracción").


Como Jim menciona, todavía hay espacio para el debate sobre cómo estructurar la salida anticipada. La alternativa a lo anterior sería:

if (byteArrayVariable == null) 
  return; // nothing to be done, leaving

if (byteArrayVariable.Length != 0)
  return;

// Conditions met, do something
STW
fuente
1
Estoy de acuerdo con el retorno en lugar del ajuste de la buena lógica, pero las mismas personas usarán el mismo argumento sobre el uso de || como para &&.
MIA
14

No pierda el tiempo tratando de atrapar por cada no esta condición. Si puede descalificar los datos o la condición, hágalo lo antes posible.

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
// do something

Esto le permite adaptar su código mucho más fácilmente para nuevas condiciones

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
if (NewCondition == false) Exit;
if (NewerCondition == true) Exit;
// do something
Michael Riley - también conocido como Gunny
fuente
2
+1 Sí, sí, sí. Una lógica tan simple como esta no debería resultar en un código difícil. Tu instinto (de la persona que pregunta) debería decirte esto.
Trabajo
11

Su ejemplo es el ejemplo perfecto de por qué los anidados ifsson un mal enfoque para la mayoría de los idiomas que admiten correctamente la comparación booleana. Anidar ifscrea una situación en la que su intención no está del todo clara. ¿Se requiere que el segundo ifsiga inmediatamente al primero? ¿O es solo porque todavía no has puesto otra operación allí?

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

En este caso, estos deben estar juntos. Actúan como guardia para garantizar que no realice una operación que resulte en una excepción. Usando anidado ifs, lo deja a la interpretación de la persona que lo sigue en cuanto a si los dos representan un guardia de dos partes o alguna otra lógica de ramificación. Al combinarlos en un solo if, estoy afirmando eso. Es mucho más difícil esconder una operación dentro de allí donde no debería estar.

Siempre favoreceré una comunicación clara de mi intención sobre la estúpida afirmación de alguien de que "no funciona en todos los idiomas". Adivina qué ... throwtampoco funciona en todos los lenguajes, ¿eso significa que no debería usarlo al codificar en C # o Java y en su lugar debería confiar en los valores de retorno para señalar cuando hubo un problema?

Dicho todo esto, es mucho más fácil de leer para invertirlo y simplemente volverán de método si bien es no satisfecho como dice STW en su respuesta.

desaparecido en combate
fuente
Más de dos condiciones pueden justificar su propia función que devuelve un valor nulo.
Trabajo
3

En este caso, sus dos cláusulas están directamente relacionadas, por lo que es preferible la segunda forma.

Si no estuvieran relacionados (por ejemplo, una serie de cláusulas de protección en diferentes condiciones), preferiría la primera forma (o refactorizaría un método con un nombre que represente lo que realmente representa la condición compuesta).

FinnNk
fuente
1

Si trabaja en Delphi, la primera forma es, en general, más segura. (Para el ejemplo dado, no hay mucho que ganar usando ifs anidados).

Delphi le permite especificar si las expresiones booleanas evalúan o no "cortocircuito", a través de conmutadores de compilación. Hacer la forma # 1 (ifs anidados) le permite asegurarse de que la segunda cláusula se ejecute si y solo si (y estrictamente después ) la primera cláusula se evalúa como verdadera.

Frank Shearar
fuente
1
En 23 años de desarrollo de Delphi, nunca he cambiado la opción "Evaluación booleana completa". Si estuviera enviando el código a otra parte, pondría {$ BOOLEVAL OFF} o {$ B-} en la unidad.
Gerry
Yo tampoco. También siempre uso la verificación de rango también ... pero otras personas no. Y no debería hacer una diferencia, excepto quizás desde una perspectiva de rendimiento, ya que no debería usar efectos secundarios en funciones utilizadas en expresiones booleanas. Sin embargo, estoy seguro de que alguien lo hace.
Frank Shearar
@Gerry: De acuerdo. La única razón por la que podría desear una evaluación completa son los efectos secundarios, ¡y los efectos secundarios en una condición son una mala idea!
Loren Pechtel
¡Solo vuelva a leer mi comentario, 23 años deberían ser 13! No tengo Tardis
Gerry
1

El segundo estilo puede ser contraproducente en VBA, ya que si la primera prueba es si el objeto generado, todavía hará la segunda prueba y se eliminará el error. Acabo de acostumbrarme a VBA cuando trato con la verificación de objetos para usar siempre el primer método.


fuente
2
Eso es porque VBA es un lenguaje terrible
Falmarri
@Falmarri, tiene algunas cosas terribles al respecto (y algunas cosas buenas). Arreglaría muchas cosas si tuviera mis druthers.
2
La falta de evaluación booleana de cortocircuito es más un legado que otra cosa. No tengo idea de por qué no comenzaron con eso. El "arreglo" VB.NET de OrElse y AndAlso también es un poco molesto, pero probablemente la única opción para solucionarlo sin romper la compatibilidad con versiones anteriores.
JohnFx
1

La segunda forma es más legible, especialmente si está utilizando bloques {} alrededor de cada cláusula, porque la primera forma conducirá a bloques anidados {} y múltiples niveles de sangría, lo que puede ser difícil de leer (debe contar los espacios de sangría para averiguar dónde termina cada bloque).

o. nate
fuente
0

Los encuentro legibles, y no acepto el argumento de que debería preocuparse por el mantenimiento del código si cambiara de idioma.

En algunos idiomas, la segunda opción funciona mejor, por lo que sería una opción clara.

Cuenta
fuente
0

Estoy contigo; Lo hago de la segunda manera. Para mí, es lógicamente más claro. La preocupación de que algunos idiomas ejecuten la segunda parte incluso si la primera se evalúa como falsa me parece un poco tonta, ya que nunca en mi vida he escrito un programa que se ejecute en "algunos idiomas"; mi código siempre parece existir en un lenguaje específico, que puede manejar esa construcción o no. (Y si no estoy seguro de si el lenguaje específico puede manejarlo, eso es algo que realmente necesito aprender, ¿no es así?)

En mi opinión, el peligro con la primera forma de hacerlo es que me deja vulnerable a poner accidentalmente un código fuera de ese ifbloque anidado cuando no era mi intención. Lo cual, sin duda, no es una gran preocupación, pero parece más razonable que preocuparse por si mi código es independiente del lenguaje.

BlairHippo
fuente
0

Prefiero la segunda opción, porque es más legible.

Si la lógica que está implementando es más compleja (comprobar que una cadena no es nula y no está vacía es solo un ejemplo), puede realizar una refactorización útil: extraer una función booleana. Estoy con @mipadi en el comentario "Code Complete" ("no importa si todos los idiomas hacen eso ...") Si el lector de su código no está interesado en mirar dentro de la función extraída, entonces el orden en el que se evalúan los operandos booleanos se convierte en una cuestión discutible para ellos.

azheglov
fuente
0
if ((byteArrayVariable != null)
 && (byteArrayVariable.Length != 0)) {
    //Do something with byteArrayVariable 

pero esa es básicamente la segunda opción.

Boicot SE para Monica Cellio
fuente