En caso contrario, se supone que la escalera debe capturar todas las condiciones: ¿se debe agregar una cláusula final redundante?

10

Esto es algo que estoy haciendo mucho últimamente.

Ejemplo:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else if (i > current) {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

A menudo, la escalera if / else es significativamente más complicada que esto ...

Ver la cláusula final? Es redundante Se supone que la escalera finalmente capturará todas las condiciones posibles. Por lo tanto, podría reescribirse así:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

Así es como solía escribir código, pero no me gusta este estilo. Mi queja es que la condición bajo la cual se ejecutará la última parte del código no es obvia por el código. Así comencé a escribir esta condición explícitamente para hacerla más evidente.

Sin embargo:

  • Escribir explícitamente la condición exhaustiva final es mi propia idea, y tengo malas experiencias con mis propias ideas, por lo general la gente me grita sobre lo horrible que estoy haciendo, y (a veces mucho) más tarde descubro que realmente fue subóptimo;
  • Una pista de por qué puede ser una mala idea: no es aplicable a Javascript, pero en otros idiomas, los compiladores tienden a emitir advertencias o incluso errores sobre el control que llega al final de la función. Dar a entender que hacer algo así podría no ser muy popular o lo estoy haciendo mal.
    • Las quejas del compilador me hicieron a veces escribir la condición final en un comentario, pero supongo que hacerlo es horrible ya que los comentarios, a diferencia del código, no tienen efecto en la semántica del programa real:
    } else { // i > current
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }

¿Me estoy perdiendo de algo? ¿O está bien hacer lo que describí o es una mala idea?

gaazkam
fuente
+1 para la excelente respuesta de @ Christophe de que la redundancia en el código aumenta las posibilidades de defectos. También hay un problema de eficiencia mucho menor . Para expandir eso, con respecto al ejemplo de código en cuestión, podríamos escribir la serie de if-else-if como si solo por separado sin ningún otro, pero generalmente no lo haríamos ya que los if-else le dan al lector la noción de alternativas exclusivas en lugar de una serie de condiciones independientes (que también serían menos eficientes, ya que eso dice que todas las condiciones deben evaluarse incluso después de una coincidencia).
Erik Eidt
@ErikEidt> ya que eso dice que todas las condiciones deben evaluarse incluso después de que uno coincida con +1, a menos que regrese de una función o se "rompa" de un bucle (que no es el caso en el ejemplo anterior).
Darek Nędza
1
+1, buena pregunta. Como comentario aparte, puede interesarle que en presencia de NaNs el ejemplo no sea exhaustivo.
monocell

Respuestas:

6

Ambos enfoques son válidos. Pero echemos un vistazo más de cerca a los pros y los contras.

Para una ifcadena con condiciones triviales como aquí, realmente no importa:

  • con un final else, es obvio para el lector descubrir en qué condición se activa el otro;
  • con un final else if, es tan obvio para el lector que no elsese necesitan más, ya que lo cubrió todo.

Sin embargo, hay muchas ifcadenas que dependen de condiciones más complejas, combinando estados de varias variables, tal vez con una expresión lógica compleja. En este caso es menos obvio. Y aquí la consecuencia de cada estilo:

  • final else: estás seguro de que una de las ramas está ocupada. Si sucede que ha olvidado un caso, pasará por esa última rama, por lo que durante la depuración, si se eligió la última rama y esperaba algo más, se dará cuenta rápidamente.
  • final else if: necesita derivar la condición redundante al código, y esto crea una fuente de error potencial con el riesgo de no cubrir todos los casos. Además, si se ha perdido un caso, no se realizará nada y podría ser más difícil descubrir que faltaba algo (por ejemplo, si algunas variables que esperaba que se establecieran mantienen valores de iteraciones anteriores).

Entonces, la condición redundante final es una fuente de riesgo. Es por eso que prefiero sugerir ir a una final else.

Editar: codificación de alta fiabilidad

Si está desarrollando con alta confiabilidad en mente, puede estar interesado en otra variante: completar su final explícita redundante else ifcon una final elsepara detectar situaciones inesperadas.

Esto es codificación defensiva. Es recomendado por algunas especificaciones de seguridad como SEI CERT o MISRA . Algunas herramientas de análisis estático incluso implementan esto como una regla que se verifica sistemáticamente (esto podría explicar las advertencias del compilador).

Christophe
fuente
77
¿Qué pasaría si después de la condición final de redundancia agrego otra cosa que siempre arroja una excepción que dice "Se suponía que no debías contactarme!" - ¿Aliviará algunos de los problemas de mi enfoque?
gaazkam
3
@gaazkam sí, este es un estilo de codificación muy defensivo. Por lo tanto, aún necesita calcular la condición final, pero para cadenas complejas propensas a errores, al menos lo descubrirá rápidamente. El único problema que veo con esta variante es que es un poco excesivo para un caso obvio.
Christophe
@gaazkam He editado mi respuesta para abordar también su idea adicional mencionada en su comentario
Christophe
1
@gaazkam Lanzar una excepción es bueno. Si lo hace, no olvide incluir el valor inesperado en el mensaje. Puede ser difícil de reproducir y el valor inesperado puede proporcionar una pista sobre el origen del problema.
Martin Maat
1
Otra opción es poner un assertfinal else if. Sin embargo, su guía de estilo puede variar si es una buena idea. (La condición de un assertnunca debe ser falsa, a menos que el programador de la pata Así que esto es por lo menos usando la función para la finalidad expuesta Pero tanta gente mal uso de él que muchas tiendas han prohibido por completo...)
Kevin
5

Algo que falta en las respuestas hasta ahora es una cuestión de qué tipo de falla es menos dañina.

Si su lógica es buena, realmente no importa lo que haga, el caso importante es lo que sucede si tiene un error.

Omite el condicional final: la opción final se ejecuta incluso si eso no es lo correcto.

Simplemente agregue el condicional final: no ejecuta ninguna opción, dependiendo de la situación, esto podría significar simplemente que algo no se muestra (daño bajo), o podría significar una excepción de referencia nula en algún momento posterior (lo que podría ser una depuración dolor.)

Agregas el condicional final y una excepción: arroja.

Tienes que decidir cuál de estas opciones es la mejor. En el código de desarrollo, considero que esto es obvio: tome el tercer caso. Sin embargo, probablemente establecería circle.src en una imagen de error y circle.alt en un mensaje de error antes de lanzarlo; en caso de que alguien decida desactivar las afirmaciones más tarde, esto hace que falle sin causar daño.

Otra cosa a considerar: ¿cuáles son sus opciones de recuperación? A veces no tienes una ruta de recuperación. Lo que creo que es el mejor ejemplo de esto es el primer lanzamiento del cohete Ariane V. Hubo un error no detectado / 0 (en realidad un desbordamiento de división) que resultó en la destrucción del refuerzo. En realidad, el código que fallaba no tenía ningún propósito en ese momento, se había vuelto discutible en el instante en que se encendieron los amplificadores. Una vez que iluminan su órbita o auge, haces lo mejor que puedes, no se pueden permitir errores. (Si el cohete se desvía debido a esto, el chico de seguridad de rango gira su llave).

Loren Pechtel
fuente
4

Lo que recomiendo es usar una assertdeclaración en su final más, en cualquiera de estos dos estilos:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else {
        assert i > current
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

O una afirmación de código muerto:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else if (i > current) {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    } else {
        assert False, "Unreachable code"
    }
}

La herramienta de cobertura de código a menudo se puede configurar para ignorar código como "afirmar falso" del informe de cobertura.


Al poner la condición en una aserción, documenta efectivamente la condición de una rama explícitamente, pero a diferencia de un comentario, la condición de aserción en realidad se puede verificar y fallará si mantiene las aserciones habilitadas durante el desarrollo o en la producción (generalmente recomiendo mantener las aserciones habilitadas en producción si no afectan demasiado el rendimiento).

Lie Ryan
fuente
1
No me gusta su primera opción, la segunda es mucho más clara de que es un caso que debería ser imposible.
Loren Pechtel el
0

Definí una macro "afirmada" que evalúa una condición, y en una construcción de depuración cae en el depurador.

Entonces, si estoy 100 por ciento seguro de que una de las tres condiciones debe ser cierta, escribo

If condition 1 ...
Else if condition 2 .,,
Else if asserted (condition3) ...

Eso deja en claro que una condición será verdadera y que no se necesita ninguna rama adicional para una afirmación.

gnasher729
fuente
-2

Recomiendo evitarlo por completo . Use ifpara declarar lo que se supone que debe manejar el bloque de código y finalice el bloque al salir de la función.

Esto da como resultado un código que es muy claro:

setCircle(circle, i, { current })
{
    if (i == current)
    {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
        return
    }
    if (i < current)
    {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
        return
    }
    if (i > current)
    {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
        return
    }
    throw new Exception("Condition not handled.");
}

La final ifes, por supuesto, redundante ... hoy. Puede volverse un pensamiento muy importante si / cuando algún desarrollador futuro reorganiza los bloques. Por lo tanto, es útil dejarlo allí.

John Wu
fuente
1
En realidad no está muy claro porque ahora necesito considerar si la ejecución de la primera rama podría cambiar el resultado de la segunda prueba. Más retornos múltiples sin sentido. Más posibilidad de que ninguna condición sea verdadera.
gnasher729
De hecho, escribo si declaraciones como esta cuando estoy esperando intencionalmente múltiples casos pueden ser tomadas. Es especialmente útil en idiomas con sentencias de cambio que no permiten fallar. Creo que si las opciones son mutuamente excluyentes, debería escribirse de modo que sea obvio que los casos son mutuamente excluyentes (usando else). Y si no está escrito así, entonces la implicación es que los casos no son mutuamente excluyentes (es posible que se caiga).
Jerry Jeremiah
@ gnasher729 Entiendo totalmente tu reacción. Conozco algunas personas muy inteligentes que tuvieron problemas con "evitar más" y "regreso temprano" ... al principio. Le animo a que se tome un tiempo para acostumbrarse a la idea y analizarla, porque estas ideas reducen la complejidad de manera objetiva y medible. El retorno temprano en realidad aborda su primer punto (necesita considerar si una rama puede cambiar otra prueba). Y la "posibilidad de que ninguna condición sea verdadera" todavía existe independientemente; Con este estilo, obtienes una excepción obvia en lugar de una falla lógica oculta.
John Wu
Pero, ¿no es la complejidad ciclomática de ambos estilos exactamente la misma? es decir, igual al número de condiciones
gaazkam