El desarrollador insiste en que las declaraciones no deben tener condiciones negadas, y siempre debe tener un bloque más

161

Tengo un conocido, un desarrollador más experimentado que yo. Estábamos hablando de prácticas de programación y me sorprendió su enfoque sobre las afirmaciones "si". Él insiste en algunas prácticas con respecto a si las declaraciones que encuentro bastante extrañas.

En primer lugar , una sentencia if debe ir seguida de una sentencia else, ya sea que haya algo que poner en ella o no. Lo que lleva a un código que se ve así:

if(condition) 
{
    doStuff();
    return whatever;
}
else
{
}

En segundo lugar , es mejor probar los valores verdaderos en lugar de los falsos. Eso significa que es mejor probar una variable 'doorClosed' en lugar de una variable '! DoorOpened'

Su argumento es que hace más claro lo que está haciendo el código.

Lo que me confunde bastante, ya que una combinación de esas dos reglas puede llevarlo a escribir este tipo de código si quiere hacer algo cuando no se cumple la condición.

if(condition)
{
}
else
{
    doStuff();
    return whatever;
}

Mi opinión acerca de esto es que, de hecho, es muy fea y / o que la mejora de la calidad, si existe, es insignificante. Pero como junior, soy propenso a dudar de mi instinto.

Así que mis preguntas son: ¿Es una práctica buena / mala / "no importa"? ¿Es una práctica común?

    
pregunta Patsuan 08.06.2017 - 15:35
fuente

15 respuestas

176

Bloque else explícito

La primera regla simplemente contamina el código y lo hace ni más legible, ni menos propenso a errores. El objetivo de su colega, supongo, es ser explícito, demostrando que el desarrollador era plenamente consciente de que la condición puede evaluar a false . Si bien es bueno ser explícito, dicha explicación no debería tener un costo de tres líneas adicionales de código .

Ni siquiera menciono el hecho de que una declaración if no sea seguida necesariamente por un else o nada: también podría ir seguida por uno o más elif s.

La presencia de la declaración return empeora las cosas. Incluso si realmente tuvieras código para ejecutar dentro del bloque else , sería más legible hacerlo así:

if (something)
{
    doStuff();
    return whatever;
}

doOtherThings();
return somethingElse;

Esto hace que el código tome dos líneas menos y desentraña el bloque else . Las cláusulas de guardia tienen que ver con eso.

Sin embargo, tenga en cuenta que la técnica de su colega podría resolver parcialmente un patrón muy desagradable de bloques condicionales apilados sin espacios:

if (something)
{
}
if (other)
{
}
else
{
}

En el código anterior, la falta de un salto de línea sano después del primer bloque if hace que sea muy fácil malinterpretar el código. Sin embargo, aunque la regla de su colega dificultaría más la mala interpretación del código, una solución más sencilla sería simplemente agregar una nueva línea.

Prueba para true , no para false

La segunda regla puede tener sentido, pero no en su forma actual.

No es falso que probar una puerta cerrada sea más intuitivo que probar una puerta no abierta . Las negaciones, y especialmente las negaciones anidadas, suelen ser difíciles de entender:

if (!this.IsMaster || (!this.Ready && !this.CanWrite))

Para resolver eso, en lugar de agregar bloques vacíos, cree propiedades adicionales, cuando sean relevantes, o variables locales .

La condición anterior podría hacerse fácil de leer:

if (this.IsSlave || (this.Synchronizing && this.ReadOnly))
    
respondido por el Arseni Mourzenko 08.06.2017 - 15:46
fuente
62

Con respecto a la primera regla, este es un ejemplo de escritura inútil. No solo lleva más tiempo escribir, sino que causará una gran confusión a cualquiera que lea el código. Si el código no es necesario, no lo escriba. Esto incluso se extendería a no tener un else poblado en su caso ya que el código regresa del bloque if :

if(condition) 
{
    doStuff();
    return whatever;
}

doSomethingElse(); // no else needed
return somethingelse;

Con respecto al segundo punto, es bueno evitar los nombres booleanos que contienen un negativo:

bool doorNotOpen = false; // a double negative is hard to reason
bool doorClosed = false; // this is much clearer

Sin embargo, extender esto a no probar un negativo nuevamente, como señala, conduce a una escritura más inútil. Lo siguiente es mucho más claro que tener un bloque if vacío:

if(!condition)
{
    doStuff();
    return whatever;
}
    
respondido por el David Arno 08.06.2017 - 15:54
fuente
44

1. Un argumento a favor de las declaraciones else vacías.

Muchas veces uso (y defiendo) algo parecido a esa primera construcción, una cosa vacía. Señala a los lectores del código (herramientas de análisis tanto humanas como automatizadas) que el programador ha pensado un poco en la situación. Las declaraciones faltantes de else que deberían haber estado presentes han matado a personas, se han estrellado vehículos y han costado millones de dólares. MISRA-C, por ejemplo, exige al menos un comentario que dice que el final faltante es intencional en una secuencia if (condition_1) {do_this;} else if (condition_2) {do_that;} ... else if (condition_n) {do_something_else;} . Otros estándares de alta confiabilidad van aún más lejos: con algunas excepciones, las declaraciones que faltan están prohibidas.

Una excepción es un comentario simple, algo como las líneas de /* Else not required */ . Esto señala la misma intención que las tres líneas vacías. Otra excepción en la que no se necesita esa otra cosa vacía es donde es evidente para los lectores del código y para las herramientas de análisis automatizadas que esa otra cosa es superflua. Por ejemplo, if (condition) { do_stuff; return; } Del mismo modo, no se necesita un elemento vacío en el caso de throw something o goto some_label 1 en lugar de return .

2. Un argumento para preferir if (condition) sobre if (!condition) .

Este es un ítem de factores humanos. La lógica booleana compleja hace tropezar a mucha gente. Incluso un programador experimentado tendrá que pensar en if (!(complex || (boolean && condition))) do_that; else do_this; . Como mínimo, vuelva a escribir esto como if (complex || (boolean && condition)) do_this; else do_that; .

3. Esto no significa que uno deba preferir las declaraciones then vacías.

La segunda sección dice "prefiere" en lugar de "tú debes". Es una pauta en lugar de una regla. La razón por la que la guía prefiere condiciones if positivas es que el código debe ser claro y obvio. Una cláusula entonces vacía (por ejemplo, if (condition) ; else do_something; ) viola esto. Está confusa en la programación, lo que hace que incluso los programadores más experimentados realicen copias de seguridad y vuelvan a leer la condición if en su forma negada. Así que escríbalo en la forma negada en primer lugar y omita la declaración else (o tenga un else más vacío o comente a tal efecto si tiene el mandato de hacerlo).


1 Escribí que las cláusulas que terminan con return , throw o goto no requieren un espacio vacío. Es obvio que la cláusula de lo contrario no es necesaria. Pero ¿qué pasa con goto ? Además, las reglas de programación críticas para la seguridad a veces no permiten el retorno anticipado y casi siempre rechazan las excepciones de lanzamiento. Sin embargo, sí permiten goto en una forma restringida (por ejemplo, goto cleanup1; ). Este uso restringido de goto es la práctica preferida en algunos lugares. El kernel de Linux, por ejemplo, está lleno de tales declaraciones goto .

    
respondido por el David Hammen 09.06.2017 - 12:55
fuente
30

Utilizo una rama else vacía (y algunas veces una rama if vacía) en casos muy raros: cuando es obvio que tanto la parte if como la otra parte deben manejarse de alguna manera, pero por alguna razón no trivial el caso puede ser manejado por no hacer nada. Y, por lo tanto, cualquier persona que lea el código con la acción de lo que sea necesario sospechará inmediatamente que falta algo y perderá su tiempo.

if (condition) {
    // No action needed because ...
} else {
    do_else_action()
}

if (condition) {
    do_if_action()
} else {
    // No action needed because ...
}

Pero no:

if (condition) {
    do_if_action()
} else {
    // I was told that an if always should have an else ...
}
    
respondido por el gnasher729 09.06.2017 - 21:06
fuente
23

Todo lo demás es igual, prefiero brevedad.

Lo que no escribas, nadie tiene que leer y entender.

Aunque ser explícito puede ser útil, ese es solo el caso si se hace evidente, sin una verbosidad excesiva, que lo que escribiste es realmente lo que querías escribir.

Por lo tanto, evite las ramas vacías, no solo son inútiles sino también poco comunes y, por lo tanto, generan confusión.

Además, evite escribir una rama else si sale directamente de la rama if.

Una aplicación útil de explícita sería poner un comentario cada vez que caiga en los casos de cambio // FALLTHRU , y usar un comentario o un bloque vacío donde necesite una declaración vacía for(a;b;c) /**/; .

    
respondido por el Deduplicator 08.06.2017 - 20:35
fuente
6

No hay una regla estricta y rápida sobre las condiciones positivas o negativas para una declaración IF, no que yo sepa. Personalmente, prefiero codificar para un caso positivo en lugar de negativo, cuando corresponda. Sin embargo, ciertamente no lo haré si me lleva a hacer un bloque de FI vacío, seguido de un ELSE lleno de lógica. Si surgiera una situación de este tipo, de todos modos tomaría como 3 segundos refactorizarla para probar un caso positivo.

Lo que realmente no me gusta de tus ejemplos es el espacio vertical completamente innecesario que ocupa el ELSE en blanco. Simplemente no hay razón alguna para hacer esto. No agrega nada a la lógica, no ayuda a documentar lo que hace el código y no aumenta la legibilidad en absoluto. De hecho, argumentaría que el espacio vertical agregado podría posiblemente disminuir la legibilidad.

    
respondido por el Langecrew 08.06.2017 - 15:50
fuente
5

Bloque else explícito

No estoy de acuerdo con esto como una declaración general que cubre todas las declaraciones if pero hay ocasiones en que agregar un bloque else por costumbre es una buena cosa.

En mi opinión, una declaración if cubre dos funciones distintas.

Si se supone que debemos hacer algo, hazlo aquí.

Cosas como esta obviamente no necesitan una parte else .

    if (customer.hasCataracts()) {
        appointmentSuggestions.add(new CataractAppointment(customer));
    }
    if (customer.isDiabetic()) {
        customer.assignNurse(DiabeticNurses.pickBestFor(customer));
    }

y en algunos casos, insistir en agregar un else podría inducir a error.

    if (k > n) {
        return BigInteger.ZERO;
    }
    if (k <= 0 || k == n) {
        return BigInteger.ONE;
    }

es no lo mismo que

    if (k > n) {
        return BigInteger.ZERO;
    } else {
        if (k <= 0 || k == n) {
            return BigInteger.ONE;
        }
    }

aunque sea funcionalmente igual. Escribir el primer if con un else vacío puede llevarte al segundo resultado que es innecesariamente feo.

Si estamos comprobando un estado específico, a menudo es una buena idea agregar un else vacío solo para recordarle que cubra esa eventualidad

        // Count wins/losses.
        if (doors[firstChoice] == Prize.Car) {
            // We would have won without switching!
            winWhenNotSwitched += 1;
        } else {
            // We win if we switched to the car!
            if (doors[secondChoice] == Prize.Car) {
                // We picked right!
                winWhenSwitched += 1;
            } else {
                // Bad choice.
                lost += 1;
            }
        }

Recuerde que estas reglas se aplican solo cuando está escribiendo un nuevo código . En mi humilde opinión, las cláusulas else vacías deben eliminarse antes del registro.

Prueba para true , no para false

Nuevamente, este es un buen consejo a nivel general, pero en muchos casos hace que el código sea innecesariamente complejo y menos legible.

A pesar de que el código como

    if(!customer.canBuyAlcohol()) {
        // ...
    }

es discordante para el lector, pero al hacerlo

    if(customer.canBuyAlcohol()) {
        // Do nothing.
    } else {
        // ...
    }

es al menos tan malo, si no peor.

Codifiqué en BCPL hace muchos años y en ese idioma hay una cláusula IF y una cláusula UNLESS para que puedas codificar mucho más fácilmente que:

    unless(customer.canBuyAlcohol()) {
        // ...
    }

que es significativamente mejor, pero aún no es perfecto.

Mi proceso personal

En general, cuando escribo el código nuevo a menudo agrego un bloque else vacío a una declaración if solo para recordarme que aún no he cubierto esa eventualidad. Esto me ayuda a evitar la captura DFS y asegura que cuando reviso el código, me doy cuenta de que hay más por hacer. Sin embargo, generalmente agrego un comentario TODO para realizar un seguimiento.

  if (returnVal == JFileChooser.APPROVE_OPTION) {
    handleFileChosen();
  } else {
    // TODO: Handle case where they pressed Cancel.
  }

Encuentro que generalmente uso else rara vez en mi código ya que a menudo puede indicar un olor de código.

    
respondido por el OldCurmudgeon 09.06.2017 - 11:31
fuente
1

Para el primer punto, he usado un lenguaje que forzó que las declaraciones de IF se usaran de esta manera (en Opal, el lenguaje detrás de un raspador de la pantalla del mainframe para colocar una interfaz gráfica de usuario en los sistemas del mainframe), y solo con una línea para el IF y el ELSE. ¡No fue una experiencia agradable!

Espero que cualquier compilador optimice esas cláusulas ELSE adicionales. Pero para el código en vivo no se agrega nada (en desarrollo puede ser un marcador útil para el código adicional).

Una vez que uso algo así como estas cláusulas adicionales es cuando uso el procesamiento de tipo CASE / WHEN. Siempre agrego una cláusula por defecto incluso si está vacía. Este es un hábito a largo plazo de lenguajes que se equivocarán si no se usa dicha cláusula, y obliga a pensar si las cosas realmente deberían pasar por alto.

Hace mucho tiempo que la práctica de mainframe (por ejemplo, PL / 1 y COBOL) se aceptó que los controles negativos eran menos eficientes. Esto podría explicar el segundo punto, aunque en estos días hay ahorros de eficiencia mucho más importantes que se ignoran como micro optimizaciones.

La lógica negativa tiende a ser menos legible, aunque no tanto en una declaración IF tan simple.

    
respondido por el Kickstart 09.06.2017 - 12:50
fuente
1

Dirigiría la posición de la mayoría de las respuestas que los bloques else vacíos son virtualmente siempre un desperdicio dañino de tinta electrónica. No agregue estos a menos que tenga una buena razón para hacerlo, en cuyo caso el bloque vacío no debe estar vacío, debe contener un comentario que explique por qué está allí.

Sin embargo, el problema de evitar los negativos merece un poco más de atención: normalmente, cuando necesita utilizar un valor booleano, necesita algunos bloques de código para operar cuando está configurado y otros bloques para funcionar cuando no está configurado. Como tal, si hace cumplir una regla de no negativos, aplique las declaraciones if() {} else {...} (con un bloque if vacío), o cree un segundo booleano para cada booleano que contenga su valor negado. Ambas opciones son malas, ya que confunden a sus lectores.

Una política útil es esta: nunca use una forma negada dentro del nombre de un booleano, y exprese la negación como un solo ! . Una declaración como if(!doorLocked) es perfectamente clara, una declaración como if(!doorUnlocked) nudos cerebros. El último tipo de expresión es lo que debe evitar a toda costa, no la presencia de un solo ! en una condición if() .

    
respondido por el cmaster 11.06.2017 - 03:26
fuente
0

Yo diría que es definitivamente una mala práctica. Agregar las instrucciones else agregará un montón de líneas sin sentido a tu código que no hacen nada y hacen que sea aún más difícil de leer.

    
respondido por el ayrton clark 08.06.2017 - 15:49
fuente
0

Hay un punto al considerar el argumento de "siempre tener una cláusula else" que no he visto en ninguna otra respuesta: puede tener sentido en un estilo de programación funcional. Tipo de.

En un estilo de programación funcional, se manejan expresiones en lugar de declaraciones. Por lo tanto, cada bloque de código tiene un valor de retorno, incluida una expresión if-then-else . Sin embargo, eso excluiría un bloque else vacío. Déjame darte un ejemplo:

var even = if (n % 2 == 0) {
  return "even";
} else {
  return "odd";
}

Ahora, en lenguajes con estilo C o sintaxis inspirada en estilo C (como Java, C # y JavaScript, solo para nombrar algunos), esto parece extraño. Sin embargo, parece mucho más familiar cuando se escribe como tal:

var even = (n % 2 == 0) ? "even" : "odd";

Dejar la rama else vacía aquí causaría que un valor no estuviera definido; en la mayoría de los casos, no queremos que sea un caso válido al programar la funcionalidad. Lo mismo con dejarlo completamente fuera. Sin embargo, cuando estás programando de manera iterativa, establezco muy pocas razones para tener siempre un bloque else .

    
respondido por el blalasaadri 11.06.2017 - 22:52
fuente
0
  

... una sentencia if debe ir seguida de una sentencia else, ya sea que haya algo que poner en ella o no.

No estoy de acuerdo if (a) { } else { b(); } debería reescribirse como if (!a) { b(); } y if (a) { b(); } else { } debería reescribirse como if (a) { b(); } .

Sin embargo, vale la pena señalar que rara vez tengo una rama vacía. Esto se debe a que normalmente registro que entré en la rama de lo contrario vacía. De esta manera puedo desarrollar puramente los mensajes de registro. Rara vez utilizo un depurador. En entornos de producción no obtienes un depurador; es bueno poder solucionar problemas de producción con las mismas herramientas que utiliza para desarrollar.

  

En segundo lugar, es mejor probar los valores verdaderos en lugar de los falsos. Eso significa que es mejor probar una variable 'doorClosed' en lugar de una variable '! DoorOpened'

Tengo sentimientos encontrados sobre esto. Una desventaja de tener doorClosed y doorOpened es que potencialmente duplica la cantidad de palabras / términos que necesita conocer. Otra desventaja es que con el tiempo el significado de doorClosed y doorOpened puede cambiar (otros desarrolladores que vienen después de usted) y podría terminar con dos propiedades que ya no son negaciones precisas entre sí. En lugar de evitar las negaciones, valoro la adaptación del idioma del código (nombres de clase, nombres de variables, etc.) al vocabulario de los usuarios de negocios y a los requisitos que me dan. No me gustaría crear un término completamente nuevo para evitar un ! si ese término solo tiene un significado para un desarrollador que los usuarios empresariales no entienden. Quiero que los desarrolladores hablen el mismo idioma que los usuarios y las personas que escriben los requisitos. Ahora, si los nuevos términos simplifican el modelo, eso es importante, pero eso debería manejarse antes de que se finalicen los requisitos, no después. El desarrollo debe manejar este problema antes de que comiencen a codificar.

  

Soy propenso a dudar de mi instinto.

Es bueno seguir haciéndote preguntas.

  

¿Es una práctica buena / mala / "no importa"?

Hasta cierto punto, mucho de esto no importa. Haz que revisen tu código y adáptalo a tu equipo. Eso suele ser lo correcto para hacer. Si todos los miembros de su equipo desean codificar de cierta manera, probablemente sea mejor hacerlo de esa manera (cambiando 1 persona -  usted mismo: toma menos puntos de historia que cambiar un grupo de personas).

  

¿Es una práctica común?

No recuerdo haber visto una rama vacía. Sin embargo, veo personas que agregan propiedades para evitar negaciones.

    
respondido por el Words Like Jared 12.06.2017 - 18:06
fuente
0

Solo abordaré la segunda parte de la pregunta y esto viene desde mi punto de vista de trabajar en sistemas industriales y manipular dispositivos en el mundo real.

Sin embargo, esto es de alguna manera un comentario extendido en lugar de una respuesta.

Cuando esté indicado

  

En segundo lugar, es mejor probar los valores verdaderos en lugar de los falsos. Ese   significa que es mejor probar una variable 'doorClosed' en lugar de una   '! doorOpened' variable

     

Su argumento es que aclara lo que está haciendo el código.

Desde mi punto de vista, aquí se supone que el estado de la puerta es un estado binario cuando en realidad es al menos un estado ternario:

  1. La puerta está abierta.
  2. La puerta no está completamente abierta ni completamente cerrada.
  3. La puerta está cerrada.

Por lo tanto, dependiendo de la ubicación de un solo sensor digital binario, solo puede suponer uno de dos conceptos:

  1. Si el sensor está en el lado abierto de la puerta: la puerta está abierta o no está abierta
  2. Si el sensor está en el lado cerrado de la puerta: la puerta está cerrada o no está cerrada.

Y no puede intercambiar estos conceptos mediante el uso de una negación binaria. Por lo tanto, es probable que doorClosed y !doorOpened no sean sinónimos y cualquier intento de fingir que son sinónimos es un pensamiento erróneo que supone un mayor conocimiento del estado del sistema que el que realmente existe.

Al volver a su pregunta, yo apoyaría la verborrea que coincide con el origen de la información que representa la variable. Por lo tanto, si la información se deriva del lado cerrado de la puerta, vaya con doorClosed , etc. Esto puede implicar el uso de doorClosed o !doorClosed según sea necesario en varias declaraciones según sea necesario, lo que genera una posible confusión con el uso de negación. Pero lo que no hace es propagar implícitamente los supuestos sobre el estado del sistema.

Para fines de discusión del trabajo que hago, la cantidad de información que tengo disponible sobre el estado del sistema depende de la funcionalidad requerida por el sistema en general.

A veces solo necesito saber si la puerta está cerrada o no. En cuyo caso solo bastará un único sensor binario. Pero otras veces necesito saber que la puerta está abierta, cerrada o en transición. En tales casos, habría dos sensores binarios (en cada extremo del movimiento de la puerta, con la comprobación de errores contra la apertura y el cierre simultáneos de la puerta) o un sensor analógico que medirá la apertura de la puerta.

Un ejemplo de lo primero sería la puerta de un horno de microondas. Puede habilitar el funcionamiento del horno basándose en que la puerta esté cerrada o no. No te importa lo abierta que esté la puerta.

Un ejemplo del segundo sería un simple accionador accionado por motor. Usted inhibe el avance del motor cuando el actuador está completamente apagado. E inhibe el accionamiento del motor en marcha atrás cuando el actuador está completamente instalado.

Fundamentalmente, la cantidad y el tipo de sensores se reducen a la ejecución de un análisis de costos de los sensores en comparación con un análisis de los requisitos de lo que se necesita para lograr la funcionalidad requerida.

    
respondido por el Peter M 13.06.2017 - 19:31
fuente
-1

Hay otro patrón que aún no se ha mencionado sobre el manejo del segundo caso que tiene un bloque de caracteres vacío. Una forma de lidiar con esto sería regresar en el bloque if. Así:

void foo()
{
    if (condition) return;

    doStuff();
    ...
}

Este es un patrón común para verificar las condiciones de error. El caso afirmativo todavía se usa, y el interior ya no está vacío, lo que deja a los programadores futuros preguntándose si una no-op fue intencional o no. También puede mejorar la legibilidad, ya que es posible que deba hacer una nueva función (dividiendo así las funciones grandes en funciones individuales más pequeñas).

    
respondido por el Shaz 08.06.2017 - 22:06
fuente
-1

Las reglas de estilo concreto son siempre malas. Las pautas son buenas, pero al final a menudo hay casos en los que puedes aclarar las cosas haciendo algo que no sigue las pautas.

Dicho esto, hay mucho odio por el vacío, ya que "desperdicia líneas", "tipografía adicional", que son simplemente malas. Podría hacer el argumento para mover los corchetes en la misma línea si realmente necesita el espacio vertical, pero si ese es un problema, no debería poner su {en una línea separada de todos modos.

Como se mencionó en otra parte, tener el bloque else es muy útil para mostrar que explícitamente no quieres que pase nada en el otro caso. Después de realizar una gran cantidad de programación funcional (donde más se requiere), aprendí que siempre debería considerar la posibilidad de escribir el if, aunque, como menciona @OldCurmudgeon, en realidad hay dos casos de uso diferentes. Uno debería tener otra cosa, uno no debería. Lamentablemente, no es algo que siempre se pueda decir de un vistazo, y mucho menos con una plantilla, de ahí que el dogmático "siempre ponga un bloque de lo contrario".

En cuanto a los 'no negativos', de nuevo, las reglas absolutas son malas. Tener un vacío puede ser extraño, especialmente si es el tipo de si no se necesita otra cosa, ¡así que escribe todo eso en lugar de un! o un == falso es malo. Dicho esto, hay muchos casos en los que lo negativo tiene sentido. Un ejemplo común sería almacenar un valor en caché:

static var cached = null

func getCached() {
    if !cached {
        cached = (some calculation, etc)
    }

    return cached
}

si la lógica real (mental / inglés) implica un negativo, también debería hacerlo la instrucción if.

    
respondido por el zacaj 13.06.2017 - 15:23
fuente

Lea otras preguntas en las etiquetas