¿Usar bloques anidados try-catch es un antipatrón?

95

¿Es esto un antipatrón? ¿Es una práctica aceptable?

    try {
        //do something
    } catch (Exception e) { 
        try {
            //do something in the same line, but being less ambitious
        } catch (Exception ex) {
            try {
                //Do the minimum acceptable
            } catch (Exception e1) {
                //More try catches?
            }
        }
    }
Señor Smith
fuente
¿Puedes darnos un caso para esto? ¿Por qué no puede manejar cada tipo de error en la captura de nivel superior?
Morons
2
Recientemente he visto este tipo de código, realizado por programadores sin experiencia que realmente no saben lo que están llamando dentro de los bloques de prueba, y no quieren molestarse en probar el código. En el ejemplo de código que he visto, fue la misma operación pero realizada cada vez con parámetros de reserva.
Señor Smith, el
@LokiAstari: su ejemplo es un intento en la sección Finalmente ... donde no hay captura. Esto está anidado en la sección Intentar. Es diferente.
Morons
44
¿Por qué debería ser un antipatrón?
2
+1 para "¿más capturas de prueba?"
JoelFan

Respuestas:

85

Esto a veces es inevitable, especialmente si su código de recuperación puede arrojar una excepción.

No es bonito, pero a veces no hay alternativas.

Oded
fuente
17
@MisterSmith: no siempre.
Oded
44
Sí, esto es algo a lo que estaba tratando de llegar. Por supuesto, hay un punto en sus declaraciones de prueba / captura anidadas donde solo tiene que decir que ya es suficiente. Estaba defendiendo la anidación en lugar de los intentos / capturas secuenciales, diciendo que hay situaciones en las que solo desea que el código dentro del segundo intento se ejecute si el primer intento se cae.
AndrewC
55
@MisterSmith: Prefiero las capturas de prueba anidadas a las capturas de prueba secuenciales que están parcialmente controladas con variables de marca (si fueran funcionalmente iguales).
FrustratedWithFormsDesigner
31
intente {transacción.commit (); } catch {try {transacción.rollback (); } catch {seriouslogging ()} notsoseriouslogging (); } es un ejemplo de un intento de captura anidado necesario
Thanos Papathanasiou
3
¡Al menos extraigan el bloque catch a un método, muchachos! Al menos hagamos esto legible.
Sr. Cochese
43

No creo que sea un antipatrón, simplemente mal utilizado.

La mayoría de las capturas de prueba anidadas son de hecho evitables y desagradables, generalmente el producto de desarrolladores junior.

Pero hay veces que no puedes evitarlo.

try{
     transaction.commit();
   }catch{
     logerror();
     try{
         transaction.rollback(); 
        }catch{
         seriousLogging();
        }
   }

Además, necesitará un bool extra en alguna parte para indicar el retroceso fallido ...

Thanos Papathanasiou
fuente
19

La lógica está bien: puede tener mucho sentido en algunas situaciones probar un enfoque alternativo, que podría experimentar eventos excepcionales ... por lo tanto, este patrón es casi inevitable.

Sin embargo, sugeriría lo siguiente para mejorar el código:

  • Refactorice el intento interno ... capture bloques en funciones separadas, por ejemplo, attemptFallbackMethody attemptMinimalRecovery.
  • Sea más específico sobre los tipos de excepciones particulares que se están detectando. ¿Realmente espera alguna subclase de Excepción y, si es así, realmente desea manejarlas de la misma manera?
  • Considere si un finallybloque podría tener más sentido: este suele ser el caso de cualquier cosa que parezca "código de limpieza de recursos"
mikera
fuente
14

Está bien. Una refactorización a tener en cuenta es empujar el código a su propio método y usar salidas tempranas para tener éxito, lo que le permite escribir los diferentes intentos de hacer algo al mismo nivel:

try {
    // do something
    return;
} catch (Exception e) {
    // fall through; you probably want to log this
}
try {
    // do something in the same line, but being less ambitious
    return;
} catch (Exception e) {
    // fall through again; you probably want to log this too
}
try {
    // Do the minimum acceptable
    return;
} catch (Exception e) {
    // if you don't have any more fallbacks, then throw an exception here
}
//More try catches?

Una vez que lo haya desglosado así, podría pensar en envolverlo en un patrón de Estrategia.

interface DoSomethingStrategy {
    public void doSomething() throws Exception;
}

class NormalStrategy implements DoSomethingStrategy {
    public void doSomething() throws Exception {
        // do something
    }
}

class FirstFallbackStrategy implements DoSomethingStrategy {
    public void doSomething() throws Exception {
        // do something in the same line, but being less ambitious
    }
}

class TrySeveralThingsStrategy implements DoSomethingStrategy {
    private DoSomethingStrategy[] strategies = {new NormalStrategy(), new FirstFallbackStrategy()};
    public void doSomething() throws Exception {
        for (DoSomethingStrategy strategy: strategies) {
            try {
                strategy.doSomething();
                return;
            }
            catch (Exception e) {
                // log and continue
            }
        }
        throw new Exception("all strategies failed");
    }
}

Luego, use el TrySeveralThingsStrategy, que es una especie de estrategia compuesta (¡dos patrones por el precio de uno!).

Una gran advertencia: no hagas esto a menos que tus estrategias sean lo suficientemente complejas, o si quieres poder usarlas de manera flexible. De lo contrario, está cargando una línea de código simple con una gran cantidad de orientación innecesaria a los objetos.

Tom Anderson
fuente
7

No creo que sea automáticamente un antipatrón, pero lo evitaría si puedo encontrar una manera más fácil y limpia de hacer lo mismo. Si el lenguaje de programación en el que está trabajando tiene una finallyconstrucción, eso podría ayudar a limpiar esto, en algunos casos.

FrustratedWithFormsDesigner
fuente
6

No es un antipatrón per se, sino un patrón de código que le indica que necesita refactorizar.

Y es bastante fácil, solo tiene que conocer una regla general que no está escribiendo más que un bloque de prueba en el mismo método. Si sabe bien escribir código relacionado, generalmente solo está copiando y pegando cada bloque try con sus bloques catch y pegándolo dentro de un nuevo método, y luego reemplace el bloque original con una llamada a este método.

Esta regla general se basa en la sugerencia de Robert C. Martin de su libro 'Clean Code':

si la palabra clave 'try' existe en una función, debería ser la primera palabra en la función y que no debería haber nada después de los bloques catch / finally.

Un ejemplo rápido de "pseudo-java". Supongamos que tenemos algo como esto:

try {
    FileInputStream is = new FileInputStream(PATH_ONE);
    String configData = InputStreamUtils.readString(is);
    return configData;
} catch (FileNotFoundException e) {
    try {
        FileInputStream is = new FileInputStream(PATH_TWO);
        String configData = InputStreamUtils.readString(is);
        return configData;
    } catch (FileNotFoundException e) {
        try {
            FileInputStream is = new FileInputStream(PATH_THREE);
            String configData = InputStreamUtils.readString(is);
            return configData;
        } catch (FileNotFoundException e) {
            return null;
        }
    }
}

Entonces podríamos refactorizar cada try try y, en este caso, cada bloque try-catch intenta lo mismo pero en diferentes ubicaciones (qué conveniente: D), solo tenemos que copiar y pegar uno de los bloques try-catch y hacer un método. .

public String loadConfigFile(String path) {
    try {
        FileInputStream is = new FileInputStream(path);
        String configData = InputStreamUtils.readString(is);
        return configData;
    } catch (FileNotFoundException e) {
        return null;
    }
}

Ahora usamos esto con el mismo propósito que antes.

String[] paths = new String[] {PATH_ONE, PATH_TWO, PATH_THREE};

String configData;
for(String path : paths) {
    configData = loadConfigFile(path);
    if (configData != null) {
        break;
    }
}

Espero que eso ayude :)

Adrián Pérez
fuente
buen ejemplo. Este ejemplo es realmente el tipo de código que tenemos que refactorizar. sin embargo, en otras ocasiones, es necesario un try-catch anidado.
linehrr
4

Seguramente está disminuyendo la legibilidad del código. Yo diría que si tienes oportunidad , entonces evita anidar trampas.

Si tiene que anidar capturas de prueba, deténgase siempre por un minuto y piense:

  • ¿Tengo la oportunidad de combinarlos?

    try {  
      ... code  
    } catch (FirstKindOfException e) {  
      ... do something  
    } catch (SecondKindOfException e) {  
      ... do something else    
    }
    
  • ¿Debería simplemente extraer la parte anidada en un nuevo método? El código será mucho más limpio.

    ...  
    try {  
      ... code  
    } catch (FirstKindOfException e) {  
       panicMethod();  
    }   
    ...
    
    private void panicMethod(){   
    try{  
    ... do the nested things  
    catch (SecondKindOfException e) {  
      ... do something else    
      }  
    }
    

Es obvio si tiene que anidar tres o más niveles de capturas de prueba, en un solo método, que es una señal segura de tiempo para la refactorización.

CsBalazsHungary
fuente
3

He visto este patrón en el código de red, y en realidad tiene sentido. Aquí está la idea básica, en pseudocódigo:

try
   connect;
catch (ConnectionFailure)
   try
      sleep(500);
      connect;
   catch(ConnectionFailure)
      return CANT_CONNECT;
   end try;
end try;

Básicamente es una heurística. Un intento fallido de conectarse podría ser solo un problema de red, pero si ocurre dos veces, eso probablemente significa que la máquina a la que está tratando de conectarse realmente no es accesible. Probablemente hay otras formas de implementar este concepto, pero lo más probable es que sean aún más feas que los intentos anidados.

Mason Wheeler
fuente
2

Resolví esta situación así (try-catch con fallback):

$variableForWhichINeedFallback = null;
$fallbackOptions = array('Option1', 'Option2', 'Option3');
while (!$variableForWhichINeedFallback && $fallbackOptions){
    $fallbackOption = array_pop($fallbackOptions);
    try{
        $variableForWhichINeedFallback = doSomethingExceptionalWith($fallbackOption);
    }
    catch{
        continue;
    }
}
if (!$variableForWhichINeedFallback)
    raise new ExceptionalException();
Adrian
fuente
2

He "tenido" que hacer esto en una clase de prueba por coincidencia (JUnit), donde el método setUp () tuvo que crear objetos con parámetros de constructor no válidos en un constructor que arrojó una excepción.

Si tuviera que hacer que fallara la construcción de 3 objetos no válidos, por ejemplo, necesitaría 3 bloques try-catch, anidados. En su lugar, creé un nuevo método, donde se detectaron las excepciones, y el valor de retorno fue una nueva instancia de la clase que estaba probando cuando tuvo éxito.

Por supuesto, solo necesitaba 1 método porque hice lo mismo 3 veces. Puede que no sea una buena solución para bloques anidados que hacen cosas totalmente diferentes, pero al menos su código sería más legible en la mayoría de los casos.

MarioDS
fuente
0

De hecho, creo que es un antipatrón.

En algunos casos, es posible que desee múltiples capturas de prueba, pero solo si NO SABE qué tipo de error está buscando, por ejemplo:

public class Test
{
    public static void Test()
    {            
        try
        {
           DoOp1();
        }
        catch(Exception ex)
        {
            // treat
        }

        try
        {
           DoOp2();
        }
        catch(Exception ex)
        {
            // treat
        }

        try
        {
           DoOp3();
        }
        catch(Exception ex)
        {
            // treat
        }
    }

    public static void Test()
    {
        try
        {
            DoOp1();
            DoOp2();
            DoOp3();
        }
        catch (DoOp1Exception ex1)
        {
        }
        catch (DoOp2Exception ex2)
        {
        }
        catch (DoOp3Exception ex3)
        {
        }
    }
}

Si no sabe lo que está buscando, DEBE usar la primera manera, que es IMHO, fea y no funcional. Supongo que este último es mucho mejor.

Entonces, si sabe qué TIPO de error está buscando, sea ​​específico . No es necesario realizar capturas de prueba anidadas o múltiples dentro del mismo método.

George Silva
fuente
2
Un código como el que mostró no tiene ningún sentido en la mayoría, si no en todos los casos. Sin embargo, el OP se refería a try-catches anidados , que es una pregunta muy diferente a la de múltiples declaraciones consecutivas.
JimmyB
0

En algunos casos, un Try-Catch anidado es inevitable. Por ejemplo, cuando el código de recuperación de error en sí mismo puede arrojar una excepción. Pero para mejorar la legibilidad del código, siempre puede extraer el bloque anidado en un método propio. Consulte esta publicación de blog para obtener más ejemplos sobre bloques anidados Try-Catch-Finalmente.

codelion
fuente
0

No hay nada mencionado como Anti Pattern en Java en ningún lado. Sí, a pocas cosas llamamos buenas prácticas y malas prácticas.

Si se requiere un bloque try / catch dentro de un bloque catch, no puede evitarlo. Y no hay alternativa. Como un bloque catch no puede funcionar como parte de prueba si se produce una excepción.

Por ejemplo :

String str=null;
try{
   str = method(a);
}
catch(Exception)
{
try{
   str = doMethod(a);
}
catch(Exception ex)
{
  throw ex;
}

Aquí, en el ejemplo anterior, el método arroja una excepción, pero el método doMethod (utilizado para manejar la excepción del método) incluso arroja una excepción. En este caso tenemos que usar el try catch dentro de try catch.

algo que se sugiere no hacer es ...

try 
{
  .....1
}
catch(Exception ex)
{
}
try 
{
  .....2
}
catch(Exception ex)
{
}
try 
{
  .....3
}
catch(Exception ex)
{
}
try 
{
  .....3
}
catch(Exception ex)
{
}
try 
{
  .....4
}
catch(Exception ex)
{
}
gauravprasad
fuente
esto no parece ofrecer nada sustancial sobre las 12 respuestas anteriores
mosquito