java - practice - ¿Lanzando excepciones para controlar el olor del código de flujo?
java exceptions list (16)
Considere este código (Java, específicamente):
public int doSomething()
{
doA();
try {
doB();
} catch (MyException e) {
return ERROR;
}
doC();
return SUCCESS;
}
Donde doB()
se define como:
private void doB() throws MyException
Básicamente, MyException
existe solo en caso de que doB()
cumpla con alguna condición (que no sea catastrófica, pero sí necesita plantear de alguna manera esta condición) para que doSomething()
sepa salir con un error.
¿Considera que el uso de una excepción, en este caso para controlar el flujo, es aceptable? ¿O es un olor a código? Si es así, ¿cómo podría refactorizar esto?
¿Es realmente importante que doC () se ejecute cuando doB () falla? Si no es así, ¿por qué no dejar que la Excepción se propague por la pila hasta donde pueda manejarse de manera efectiva? Personalmente, considero usar códigos de error con un código malicioso.
Editar: En su comentario, usted ha descrito exactamente el escenario donde debería simplemente declarar
public void doSomething() throws MyException
Así es como lo manejaría:
public void doSomething()
throws MyException{
doA();
try {
doB();
} finally {
doC();
}
}
Si se produce un error en doB (), la excepción se propagará por la pila.
Como otros han indicado, realmente depende de la funcionalidad prevista de los métodos involucrados aquí. Si el rol de doSomething()
es verificar algo, devolver un código que permita que la ejecución normal de sus métodos de llamada continúe (aunque en diferentes rutas dependiendo del código devuelto) entonces tener que devolver ese int puede estar bien. Sin embargo, en ese caso, doB()
probablemente sea consistente y devuelva un booleano o algo que doSomething()
pueda verificar en lugar de lanzar una excepción que realmente no hará otra cosa.
Si doB()
fuera un método público de otra clase que pudiera tener otros usos fuera de doSomething()
(a diferencia de un método privado en la misma clase que usted indicó) y arroje una excepción y desee usarlo en doSomething()
(que tiene el mismo rol indicado anteriormente), entonces su ejemplo estaría bien.
Sin embargo, el hecho de que el código que devuelve cuando doB()
arroja una excepción se llama ERROR
, indica que probablemente sea una falla que evitaría cualquier operación de la que doSomething()
forme parte. En ese caso, probablemente no deberías atrapar la excepción lanzada por doB()
y simplemente dejar que se lance a la pila hasta que llegue a un punto donde pueda manejarse o hasta que llegue a tu controlador general de errores que puede informar / registrarlo o lo que sea.
Creo que es un mal uso de las excepciones, simple y llanamente. Crearía funciones normales que devuelvan valores normales y dejen excepciones para lo que están diseñados.
Depende completamente de qué es esa condición de error, y cuál es el trabajo del método. Si devolver ERROR
es una forma válida de manejar ese error para la función de llamada, ¿por qué sería malo?
A menudo, sin embargo, es un olor. Considera esto:
bool isDouble(string someString) {
try {
double d = Convert.ParseInt32(someString);
} catch(FormatException e) {
return false;
}
return true;
}
Ese es un olor a código muy grande, porque no esperas un doble valor. Solo quieres saber si una cadena contiene un doble.
A veces, el marco que utiliza no tiene otras formas de hacer lo que desea. Para lo anterior, hay una mejor manera:
bool isDouble(string someString) {
bool success;
Convert.TryParseInt32(someString, ref success);
return success;
}
Ese tipo de excepciones tienen un nombre especial, acuñado por algún amigo cuyo blog he leído recientemente. Pero lamentablemente, olvidé su nombre. Por favor comenta si lo sabes. Por último, pero no menos importante, lo anterior es pseudo código. No soy desarrollador de ac #, por lo que lo anterior no compila, estoy seguro, pero TryParseInt32 / ParseInt32 demuestra que bueno, creo que sí, voy con C #.
Ahora, a tu código. Examinemos dos funciones. Uno huele y el otro no:
1. Olor
public int setupSystem() {
doA();
try { doB(); }
catch (MyException e)
{ return ERROR; }
doC();
return SUCCESS;
}
Es un olor a código , porque cuando quieres configurar un sistema, no quieres que falle. Si no se configura un sistema, significa que no puede continuar sin manejar ese error.
2. Ok
public int pingWorkstation() {
doA();
try { doB(); }
catch (MyException e)
{ return ERROR; }
doC();
return SUCCESS;
}
Eso está bien, porque el propósito de ese método es probar si la estación de trabajo todavía es alcanzable. Si no es así, entonces eso es parte del resultado de ese método, y no es un caso excepcional que necesita una ruta de retorno alternativa.
Dependiendo de la lógica de doB, podría tener algunos valores de retorno apuntando si estaba bien o no, y luego algo podría usar el valor devuelto para manejar la situación de una manera apropiada.
En C #, arrojar excepciones es relativamente costoso, en cuanto al rendimiento, por lo que es otra razón para evitar su uso para controlar el flujo. Es ese el caso en Java? Sí, hay un argumento que se debe hacer para no analizar en exceso el rendimiento antes de tiempo. Pero si sabes que algo tomará tiempo extra, y es fácil de evitar, ¿no deberías hacerlo? ¡A menos que realmente no te importe!
Está bien lanzar Exception en caso de error, como en doB (). Pero el problema es la función doSomething ().
No debe usar declaraciones de devolución para indicar el éxito o el fracaso. Deberías hacer:
try {
doB();
} catch (MyException e) {
throw MyException2(e);
}
Las excepciones se deben usar cuando:
- una función no puede completarse normalmente, y
- cuando no hay un valor de retorno que pueda usarse para indicar el error, o
- cuando la excepción lanzada comunica más información que la
return FAILURE;
puede, por ejemplo, excepciones anidadas o similares.
Recuerde que todas las excepciones, como los valores de devolución o los parámetros del método, son simplemente mensajes enviados entre diferentes partes del código. Esfuércese por optimizar el equilibrio entre la información que se comunica a través de estos métodos y la simplicidad de la API. Cuando un indicador simple de SUCCESS / FAILURE es todo lo que se necesita (y el método no necesita devolver otros valores), úselo. Si el método ya debe devolver un valor, generalmente necesita usar una excepción (que es, para una forma de verlo, simplemente un valor de retorno "excepcional" del método). Si la información de falla que debe comunicarse es demasiado abundante para ser comunicada en un valor de retorno (por ejemplo, el motivo de una falla de E / S), use una excepción.
Finalmente, el manejo de errores es una decisión de diseño, y no existe un conjunto de reglas que cubra todos los casos.
Las excepciones son para un comportamiento no estándar. Entonces sí , está bien usarlos para controlar el flujo.
Como dice Sun:
En términos generales, no arroje una RuntimeException ni cree una subclase de RuntimeException simplemente porque no desea preocuparse por especificar las excepciones que pueden arrojar sus métodos.
Aquí está la pauta de la línea inferior: si se puede esperar razonablemente que un cliente se recupere de una excepción, conviértalo en una excepción marcada. Si un cliente no puede hacer nada para recuperarse de la excepción, conviértalo en una excepción no verificada.
Solo recuerda no subclasificar RuntimeException pero Exception ya que estas son más rápidas.
Mi único problema con el código del OP es que estás mezclando paradigmas: doB muestra un error lanzando una excepción, mientras que algo Algo muestra un error al devolver un código. Idealmente, elegirías uno o el otro. Por supuesto, en el mantenimiento tradicional, es posible que no tenga ese lujo.
Si eliges devolver códigos de error, está bien, pero no me gusta porque te anima a usar canales laterales (como variables globales) para comunicar el estado en caso de falla, en lugar de agrupar ese estado en una excepción y dejar que suba la pila hasta que puede hacer algo al respecto.
Nunca me ha gustado usar excepciones para el flujo de control (en algunos idiomas, como el CLR, son caros).
Si puede modificar doB () , lo mejor que puede hacer es cambiarlo para devolver un valor booleano que indique éxito o error, para que su ejemplo se vea así:
public int doSomething()
{
doA();
if (!doB()) {
return ERROR;
}
doC();
return SUCCESS;
}
Usar excepciones para controlar el flujo , definitivamente es malo. Las excepciones deben arrojarse solo en condiciones excepcionales.
Sin embargo, tener un método de utilidad que simplemente arroje una excepción o no haga nada no es particularmente malo. Por ejemplo, es posible que desee validar todos los argumentos al ingresar a un método, o verificar que el estado interno aún sea válido entre cálculos complejos (que por alguna razón no puede forzar internamente, tal vez acepte cierres y los ejecute). En estos casos, no sería una mala idea extraer la verificación de coherencia en su propio método, para mejorar la legibilidad de la implementación del método "real".
La línea divisoria es en realidad que debe lanzar una excepción cuando se encuentra algo fuera de los parámetros operativos normales, y las cosas son tan malas que no hay una forma real para que el método proceda con lo que hace.
Como ejemplo de lo que no se debe hacer, esto sería usar excepciones para el control de flujo:
public int calculateArraySize(Object[] array)
{
int i = 0;
try
{
array[i++];
}
catch (ArrayIndexOutOfBoundsException ignore) {}
return i;
}
Creo que devolverá el resultado correcto, pero será terriblemente lento e ineficiente, además de difícil de leer para las personas que están acostumbradas a que las excepciones se usen correctamente. ;-)
Por otro lado, algo como esto estaría bien en mi opinión:
public void myMethod(int count, Foobar foo) throws MyPackageException
{
validateArgs(count, foo);
// Stuff
}
private void validateArgs(int count, Foobar foo) throws MyPackageException
{
if (m_isClosed)
{
throw new IllegalStateException("Cannot call methods on a closed widget");
}
if (count < 0)
{
throw new IllegalArgumentException("count must be greater than zero");
}
foo.normalise(); // throws MyPackageException if foo is not fully initialised at this point
}
A pesar de que todo lo que hace el segundo método es potencialmente lanzar una excepción, no está haciendo esto para controlar el flujo del programa, sino que está elevándolo en respuesta a condiciones excepcionales.
Ustedes me están matando aquí. NUNCA quiere usar excepciones para controlar el flujo. ¡No sería muy diferente a usar declaraciones de goto en todas partes!
Las excepciones son excepciones, lo que significa que algo inesperado ha sucedido. Algo así que estamos deteniendo la ejecución de la aplicación. Cualquier persona que haya trabajado en una aplicación en la que este tipo de control de flujo de excepción salga querrá buscar al desarrollador que lo hizo y estrangularlo (o ella).
Usar la excepción como control de flujo hace que el mantenimiento y el desarrollo de nuevas funciones sean una pesadilla.
Las excepciones no deben usarse solo para controlar el flujo. Las excepciones controladas deben usarse para indicarle al código de llamadas que no se cumplieron ciertas condiciones del contrato de la API. No diseñaría doSomething
para manejar casos donde las llamadas a doB
fallarían a menudo usando un bloque try/catch
. Si doB
que tratar un fallo frecuente, diseñaría doB
para devolver un boolean
éxito o error para indicar a sus métodos de llamada si continuar con sus propios métodos de tipo doC
:
public int doSomething() {
doA();
if ( doB() )
doC();
return SUCCESS;
} else {
return ERROR;
}
}
Ver la respuesta superior a esta pregunta:
¿Qué tan lentas son las excepciones de Java?
Resulta que son al menos 50 veces más lento que el flujo de código normal. Así que diría que usar excepciones en la ejecución habitual de código definitivamente es una mala idea.