Use un otro después de la excepción (o no)

9

Considere este bit de código:

if (x == 1)
{
  throw "no good; aborting" ;
}

[... more code ...]

Ahora considere este código:

if (x == 1)
{
  throw "no good; aborting" ;
}
else
{
  [... more code ...]
}

Los dos casos funcionan exactamente de la misma manera. El primer caso tiene la ventaja de que no tiene que "encapsular" el resto del código en un else. El segundo tiene la ventaja de seguir la práctica de tener explícitamente un elsepara cada if.

¿Alguien puede proporcionar argumentos sólidos a favor de uno sobre el otro?

rlandster
fuente
55
La práctica que usted cita a favor de lo explícito elseparece falsa. Muy a menudo, simplemente no hay nada que poner en el elsebloque a menos que se doble hacia atrás.
¿Consistencia? ¿Permitir robustez frente al cambio de código? ¿Legibilidad?
Thomas Eding
1
Ehhhh, no soy tan fanático de la idea de que todo ifnecesita un else. El último programador que trabajó en nuestra base de código lo siguió rígidamente ( bueno, a veces ... es un poco esquizofrénico ). Como resultado, tenemos muchos else { /* do nothing */ }bloques completamente sin sentido ensuciando el código ...
KChaloux
44
"Otra cosa para cada si" parece una extraña proclamación emitida por un arquitecto de la nube en nombre de una coherencia (tonta). No veo ninguna ventaja en seguir esa práctica y nunca he oído hablar de ella en ningún lado.
Erik Dietrich
Es un redundante más. Si está trabajando con .NET stack, instale ReSharper y le recordará que elimine todas las declaraciones redundantes.
CodeART

Respuestas:

16

No debe agregar elsedespués de las iframas que rompen el flujo de control incondicionalmente , como las que contienen a throwo a return. Esto mejoraría la legibilidad de su programa al eliminar el nivel innecesario de anidamiento introducido por la elserama.

Lo que se ve más o menos bien con un solo se throwvuelve realmente feo cuando se involucran varios lanzamientos seguidos:

void myMethod(int arg1, int arg2, int arg3) {
    // This is demonstrably ugly - do not code like that!
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    } else {
        if (!isValid(arg2)) {
            throw new ArgumentException("arg2 is invalid");
        } else {
            if (!isValid(arg3)) {
                throw new ArgumentException("arg3 is invalid");
            } else {
                // The useful code starts here
            }
        }
    }
}

Este fragmento hace lo mismo, pero se ve mucho mejor:

void myMethod(int arg1, int arg2, int arg3) {
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    }
    if (!isValid(arg2)) {
        throw new ArgumentException("arg2 is invalid");
    }
    if (!isValid(arg3)) {
        throw new ArgumentException("arg3 is invalid");
    }
    // The useful code starts here
}
dasblinkenlight
fuente
+1 verdadero. El segundo caso de OP te obliga a leer con cuidado, luego te deja con un WTF. Pero ... siempre trate de hacer los métodos cortos. Un retorno en el medio de un método de 200 líneas también es malo.
Tulains Córdova
1
Para ser justos, si solo está usando repetidos if, puede hacerlo else if.
Guvante
2
@Guvante: cada ifprueba busca una sola condición y la trata si la condición es verdadera y, a menos que haya algo alternativo que tenga que suceder si la condición es falsa, no else ifes necesario. Tenemos un término en mi oficina para el código como el primer fragmento de dasblinkenlight: " máquinas pachinko ".
Blrfl
@Blrfl máquinas pachinko ja, analogía perfecta +1
Jimmy Hoffa
@Blrfl: Estaba haciendo referencia a que los if repetidos son un mal ejemplo de demasiado anidamiento. No deberías anidar ifs repetidos de todos modos. Estoy de acuerdo en que, en general, a menos que esté hablando de una cantidad menor de código, no hay razón para incluirlo else.
Guvante
5

Llamaría a la práctica "explícita de lo contrario" a la que se refiere como un antipatrón, ya que oscurece el hecho de que no hay un código de caso especial como otra cosa para su if.

La legibilidad / mantenibilidad generalmente mejora cuando en su mayoría no tiene más que construcciones de flujo de código necesarias, y las minimiza. Esto significa elses redundantes y los if que agregarán un alcance a una función completa hacen que seguirlo y mantenerlo sea más difícil.

Digamos, por ejemplo, que tiene esta función:

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
    }
    else
    {
        oblogonToConfigure.Color = _validColors[0];
        oblogonToConfigure.ColorIndex = 0;
    }
}

Ahora, el requisito es que durante la configuración también debe especificar el tipo / índice de tipo del oblogon, hay varios ámbitos en los que alguien podría colocar ese código y terminar con un código no válido, es decir

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validOblogons.Contains(oblogonToConfigure.Type))
    {
        oblogonToConfigure.Type = _validOblogons[0];
        oblogonToConfigure.TypeIndex = 0;
        if (_validColors.Contains(oblogonToConfigure.Color))
        {
            oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
        }
        else
        {
            oblogonToConfigure.Color = _validColors[0];
            oblogonToConfigure.ColorIndex = 0;
        }
    }
    else
    {
        oblogonToConfigure.TypeIndex = _validOblogons.IndexOf(oblogonToConfigure.Type);
    }
}

Compare esto con si el código original se escribió con construcciones de flujo de control mínimas necesarias y minimizadas.

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.Color = _validColors[0];
    }

    oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
}

Ahora sería mucho más difícil poner accidentalmente algo en el alcance incorrecto o terminar con los ámbitos de hinchazón que causan duplicación en el crecimiento y el mantenimiento a largo plazo de esta función. Además, es obvio cuáles son los posibles flujos a través de esta función, por lo que se mejora la legibilidad.

Lo sé, el ejemplo es un poco artificial, pero he visto muchas veces

SomeFunction()
{
    if (isvalid)
    {
        /* ENTIRE FUNCTION */
    }
    /* Nothing should go here but something does on accident, and an invalid scenario is created. */
}

Entonces, formalizar esas reglas sobre construcciones de flujo de control creo que puede ayudar a la gente a desarrollar la intuición necesaria para oler algo cuando comienzan a escribir código como ese. Entonces comenzarán a escribir ...

SomeFunction()
{
    if (!isvalid)
    {
        /* Nothing should go here, and it's so small no one will likely accidentally put something here */
        return;
    }

    /* ENTIRE FUNCTION */
}
Jimmy Hoffa
fuente