¿Cuál es el mejor estilo de codificación?

Question

Durante una revisión de código, un desarrollador senior comentó sobre algunos anidamientos que tenía en mi código. Me sugirió que estableciera un valor bool para que nunca tenga más de un nivel de anidamiento. Creo que mi código es más legible, pero quiero obtener la opinión de otros desarrolladores sobre esto. ¿Cuál es mejor estilo? ¿Se funda su aversión a la anidación?

A continuación hay algunos ejemplos de código simplificado.

Anidado:

If(condition1)
{
    If(condition2)
    {
        if(condition3)
        {
            return true;
        }
        else
        {
            log("condition3 failed");
        }
    else
    {
        log("condition2 failed")
    }
}
else
{
    log("condition1 failed")
}

return false;

o

Impulsado por Bool:

bool bRC = false;

bRC = (condition1);
if(brc)
{
    bRC = (condition2);
}
else
{
    log("condition1 failed");
    return false;
}

if(bRC)
{
    bRC = (condition3);
}
else
{
    log("condition2 failed");
    return false;
}

if(bRC)
{
    return true;
}
else
{
    log("condition3 failed");
    return false;
}

Answer 1

Me gusta más el tuyo, pero probablemente haría algo como:

if (condition1 && condition2 && condition3)
{
    return true;
}
else if (!condition1)
{
    log("condition1 failed");
}
else if (!condition2)
{
    log("condition2 failed");
}
else
{
    log("condition3 failed");
}
return false;

Si las condiciones son expresiones complejas, entonces podría asignar las expresiones a variables con nombres apropiados antes de evaluar las declaraciones if para evitar tener que volver a calcular los valores en cada if.

Esto supone que el modo normal es que todas las condiciones son verdaderas y, por lo tanto, desea tener esa verificación primero. Si el modo normal es que una o más condiciones son falsas, entonces lo reordenaría y comprobaría cada negación por turno y simplemente devolvería verdadero si fallaran todas las verificaciones. Eso también eliminaría la necesidad de variables temporales para tomar el lugar de expresiones complejas.

Answer 2

Si no tienes reglas tontas sobre los puntos de retorno múltiples, creo que esto es bastante bueno (y también lo hace alguien más, pero eliminaron su respuesta por razones desconocidas):

if(!condition1)
{
    log("condition1 failed");
    return false;
}

if(!condition2)
{
    log("condition2 failed");
    return false;
}

if(!condition3)
{
    log("condition3 failed");
    return false;
}

return true;

Tal vez esta sea una aversión igual a la súper anidación, pero ciertamente es más limpio que su basura que almacena las condiciones booleanas en ciertos valores. Sin embargo, puede ser menos legible en contexto: considere si una de las condiciones era isreadable () . Es más claro decir if (isreadable ()) porque queremos saber si algo es legible. if (! isreadable ()) sugiere si queremos saber si no es legible, lo cual no es nuestra intención. Ciertamente es discutible que pueda haber situaciones en las que una sea más legible / intuitiva que la otra, pero yo también soy fanático de esta manera. Si alguien queda atrapado en las devoluciones, puede hacer esto:

if(!condition1)
    log("condition1 failed");

else if(!condition2)
    log("condition2 failed");

else if(!condition3)
    log("condition3 failed");

else
    return true;

return false;

Pero eso es bastante sencillo, y menos "claro" en mi opinión.

Answer 3

Personalmente, el código anidado me resulta mucho más fácil de leer.

Answer 4

Generalmente prefiero anidado para mis condiciones; por supuesto, si mis condiciones anidadas se sangran demasiado a la derecha, debo comenzar a preguntarme si hay una mejor manera de hacer lo que sea que esté tratando de hacer (refactorización, rediseño, etc.)

Answer 5

Similar a la versión anidada, pero mucho más limpia para mí:

if not condition1:
    log("condition 1 failed")
else if not condition2:
    log("condition 2 failed")
else if not condition3:
    log("condition 3 failed")
else
    return true;
return false;

Tenga en cuenta que cada condición se evalúa una vez.

Answer 6

El segundo estilo es absurdamente detallado: ¿realmente sugirió exactamente esto? No necesita la mayoría de esas declaraciones if , porque hay un return en la parte else.

Con el ejemplo anidado en el que está confiando, no olvide incluir las posibles cláusulas else .

Ninguno de estos me parece satisfactorio.

Answer 7

La forma impulsada por bool es confusa. La anidación está bien si es necesario, pero puede eliminar parte de la profundidad de anidación combinando las condiciones en una declaración, o llamando a un método donde se realiza parte de la evaluación adicional.

Answer 8

Creo que ambas formas son posibles y tienen sus ventajas y desventajas. Usaría el estilo impulsado por bool en casos en los que tendría un anidamiento realmente profundo (8 o 10 o algo así). En su caso con 3 niveles, elegiría su estilo, pero para la muestra exacta de arriba, diría así:

void YourMethod(...)
{
  if (condition1 && condition2 && consition3)
    return true;

  if (!condition 1)
    log("condition 1 failed");

  if (!condition 2)
    log("condition 2 failed");

  if (!condition 3)
    log("condition 3 failed");

  return result;
}

... o así si prefiere un único punto de salida (como yo) ...

void YourMethod(...)
{
  bool result = false;

  if (condition1 && condition2 && consition3)
  { 
    result = true;
  }
  else
  {
    if (!condition 1)
      log("condition 1 failed");

    if (!condition 2)
      log("condition 2 failed");

    if (!condition 3)
      log("condition 3 failed");
  }
  return result;
}

De esa manera, obtendrá toda la condición fallida registrada en la primera ejecución. En su ejemplo, solo se registrará una condición fallida, incluso si hay más de una condición fallida.

Answer 9

Probablemente iría con

   if (!condition1)      log("condition 1 failed");
   else if (!condition2) log("condition 2 failed");
   else if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

que creo que es más eficiente y mucho más limpio ...

Además, una vez que el cliente decide que todas las condiciones deben evaluarse y registrarse si fallan, no solo la primera que falla, esto es mucho más fácil de modificar para hacer eso:

   if (!condition1) log("condition 1 failed");
   if (!condition2) log("condition 2 failed");
   if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

Answer 10

Así es como lo implementaría, siempre que sus implementaciones realmente reflejen el comportamiento deseado.

if (!condition1) {
    log("condition1 failed");
    return false;
}
if (!condition2) {
    log("condition2 failed");
    return false;
}
if (!condition3) {
    log("condition3 failed");
    return false;
}
return true;

Sin embargo, creo que es más probable que se registre cada condición fallida.

result = true;
if (!condition1) {
    log("condition1 failed");
    result = false;
}
if (!condition2) {
    log("condition2 failed");
    result = false;
}
if (!condition3) {
    log("condition3 failed");
    result = false;
}
return result;

Answer 11

Si el idioma admite el manejo de excepciones, iría con lo siguiente:

try {
    if (!condition1) {
        throw "condition1 failed";
    }

    if (!condition2) {
        throw "condition2 failed";
    }

    if (!condition3) {
        throw "condition3 failed";
    }

    return true;

} catch (e) {
    log(e);
    return false;
}

EDITAR de charles bretana: Consulte Uso de excepciones para el flujo de control

Answer 12

No me gusta de ninguna manera. Cuando tienes tantos nidos, algo está mal. En el caso de una validación de formulario o algo que realmente requiera algo como esto, trate de descubrir algo que sea más modular o compacto.

Un ejemplo sería una matriz que contiene las condiciones, a través de las cuales iterará con un tiempo, e imprimirá / romperá según sea necesario.

Hay demasiadas implementaciones dependiendo de sus necesidades, por lo que crear un código de ejemplo no tendría sentido.

Como regla general, cuando su código parece demasiado complicado, apesta :). Intenta repensarlo. Seguir las prácticas de codificación la mayoría de las veces hace que el código resulte mucho más estético y corto; y obviamente, también más inteligente.

Answer 13

El código volverá a enunciar el problema en un idioma dado. Por lo tanto, mantengo que cualquiera de los fragmentos puede ser "mejor". Depende del problema que se esté modelando. Aunque supongo que ninguna de las soluciones será paralela al problema real. Si pone términos reales en lugar de condición1,2,3, podría cambiar por completo el `` mejor '' código.
Sospecho que hay una mejor manera (3d) de escribir eso todos juntos.

Answer 14

if( condition1 && condition2 && condition3 )
    return true;

log(String.Format("{0} failed", !condition1 ? "condition1" : (!condition2 ? "condition2" : "condition3")));
return false;

De esa manera, no tiene que ver muchas líneas de código solo para iniciar sesión. Y si todas sus condiciones son verdaderas, no pierde el tiempo evaluándolas en caso de que tenga que iniciar sesión.