Tenga cuidado con los peligros de las excepciones genéricas

Mientras trabajaba en un proyecto reciente, encontré un fragmento de código que realizaba la limpieza de recursos. Debido a que tenía muchas llamadas diversas, podría generar seis excepciones diferentes. El programador original, en un intento de simplificar el código (o simplemente guardar la escritura), declaró que el método arroja en Exceptionlugar de las seis excepciones diferentes que podrían arrojarse. Esto obligó a que el código de llamada se envolviera en un bloque try / catch que capturara Exception. El programador decidió que debido a que el código era para fines de limpieza, los casos de falla no eran importantes, por lo que el bloque de captura permanecía vacío mientras el sistema se apagaba de todos modos.

Obviamente, estas no son las mejores prácticas de programación, pero nada parece estar terriblemente mal ... excepto por un pequeño problema de lógica en la tercera línea del código original:

Listado 1. Código de limpieza original

cleanupConnections () vacío privado lanza ExceptionOne, ExceptionTwo {for (int i = 0; i <connections.length; i ++) {connection [i] .release (); // Lanza ExceptionOne, ExceptionTwo connection [i] = null; } conexiones = nulo; } cleanupFiles vacío abstracto protegido () lanza ExceptionThree, ExceptionFour; removeListeners () vacío abstracto protegido lanza ExceptionFive, ExceptionSix; public void cleanupEverything () lanza Exception {cleanupConnections (); cleanupFiles (); removeListeners (); } public void done () {try {doStuff (); cleanupEverything (); doMoreStuff (); } captura (excepción e) {}}

En otra parte del código, la connectionsmatriz no se inicializa hasta que se crea la primera conexión. Pero si nunca se crea una conexión, entonces la matriz de conexiones es nula. Entonces, en algunos casos, la llamada a connections[i].release()da como resultado un NullPointerException. Este es un problema relativamente fácil de solucionar. Simplemente agregue un cheque para connections != null.

Sin embargo, nunca se informa de la excepción. Es arrojado cleanupConnections(), arrojado nuevamente cleanupEverything()y finalmente atrapado done(). El done()método no hace nada con la excepción, ni siquiera lo registra. Y debido a cleanupEverything()que solo se llama done(), la excepción nunca se ve. Entonces el código nunca se arregla.

Por lo tanto, en el escenario de falla, los métodos cleanupFiles()y removeListeners()nunca se llaman (por lo que sus recursos nunca se liberan) y doMoreStuff()nunca se llaman, por lo que el procesamiento final done()nunca se completa. Para empeorar las cosas, done()no se llama cuando el sistema se apaga; en su lugar, se llama para completar cada transacción. Entonces, los recursos se filtran en cada transacción.

Este problema es claramente importante: los errores no se informan y los recursos se pierden. Pero el código en sí parece bastante inocente y, por la forma en que se escribió, este problema resulta difícil de rastrear. Sin embargo, aplicando algunas pautas simples, el problema se puede encontrar y solucionar:

  • No ignore las excepciones
  • No coger genéricos Exceptions
  • No arrojes Exceptions genéricos

No ignore las excepciones

El problema más obvio con el código del Listado 1 es que se ignora por completo un error en el programa. Se está lanzando una excepción inesperada (las excepciones, por su naturaleza, son inesperadas) y el código no está preparado para lidiar con esa excepción. La excepción ni siquiera se informa porque el código asume que las excepciones esperadas no tendrán consecuencias.

En la mayoría de los casos, una excepción debería, como mínimo, registrarse. Varios paquetes de registro (consulte la barra lateral "Excepciones de registro") pueden registrar errores y excepciones del sistema sin afectar significativamente el rendimiento del sistema. La mayoría de los sistemas de registro también permiten que se impriman los seguimientos de la pila, lo que proporciona información valiosa sobre dónde y por qué ocurrió la excepción. Finalmente, debido a que los registros generalmente se escriben en archivos, se puede revisar y analizar un registro de excepciones. Consulte el Listado 11 en la barra lateral para ver un ejemplo de seguimiento de la pila de registros.

Las excepciones de registro no son críticas en algunas situaciones específicas. Uno de ellos es la limpieza de recursos en una cláusula final.

Excepciones en finalmente

En el Listado 2, se leen algunos datos de un archivo. El archivo debe cerrarse independientemente de si una excepción lee los datos, por lo que el close()método está envuelto en una cláusula final . Pero si un error cierra el archivo, no se puede hacer mucho al respecto:

Listado 2

public void loadFile (String fileName) lanza IOException {InputStream in = null; try {in = new FileInputStream (fileName); readSomeData (en); } finalmente {if (in! = null) {try {in.close (); } catch (IOException ioe) {// Ignorado}}}}

Tenga en cuenta que loadFile()todavía informa un IOExceptional método de llamada si la carga de datos real falla debido a un problema de E / S (entrada / salida). También tenga en cuenta que, aunque close()se ignora una excepción de , el código lo indica explícitamente en un comentario para dejarlo en claro para cualquiera que esté trabajando en el código. Puede aplicar este mismo procedimiento para limpiar todos los flujos de E / S, cerrar sockets y conexiones JDBC, etc.

Lo importante de ignorar las excepciones es asegurarse de que solo un método esté incluido en el bloque try / catch que ignora (de modo que se sigan llamando a otros métodos del bloque adjunto) y que se capture una excepción específica. Esta circunstancia especial difiere claramente de la captura de un genérico Exception. En todos los demás casos, la excepción debe registrarse (como mínimo), preferiblemente con un seguimiento de pila.

No detecte excepciones genéricas

A menudo, en software complejo, un bloque de código determinado ejecuta métodos que arrojan una variedad de excepciones. Carga dinámica de una clase y una instancia de un objeto puede lanzar varias excepciones diferentes, entre ellos ClassNotFoundException, InstantiationException, IllegalAccessException, y ClassCastException.

En lugar de agregar los cuatro bloques catch diferentes al bloque try, un programador ocupado puede simplemente envolver las llamadas al método en un bloque try / catch que captura Exceptions genéricos (vea el Listado 3 a continuación). Si bien esto parece inofensivo, podrían producirse algunos efectos secundarios no deseados. Por ejemplo, si className()es nulo, Class.forName()arrojará un NullPointerException, que quedará atrapado en el método.

En ese caso, el bloque de captura detecta excepciones que nunca tuvo la intención de detectar porque a NullPointerExceptiones una subclase de RuntimeException, que, a su vez, es una subclase de Exception. Por lo que los genéricos catch (Exception e)capturas de todas las subclases RuntimeException, entre ellos NullPointerException, IndexOutOfBoundsExceptiony ArrayStoreException. Normalmente, un programador no tiene la intención de detectar esas excepciones.

En el Listado 3, el null classNameresultado es a NullPointerException, que indica al método de llamada que el nombre de la clase no es válido:

Listado 3

public SomeInterface buildInstance (String className) {SomeInterface impl = null; intente {Class clazz = Class.forName (className); impl = (SomeInterface) clazz.newInstance (); } catch (Exception e) {log.error ("Error al crear la clase:" + className); } return impl; }

Otra consecuencia de la cláusula de captura genérica es que el registro es limitado porque catchno conoce la excepción específica que se captura. Algunos programadores, cuando se enfrentan a este problema, recurren a agregar un cheque para ver el tipo de excepción (ver Listado 4), lo que contradice el propósito de usar bloques catch:

Listado 4

catch (Exception e) {if (e instanceof ClassNotFoundException) {log.error ("Nombre de clase no válido:" + className + "," + e.toString ()); } else {log.error ("No se puede crear la clase:" + className + "," + e.toString ()); }}

El Listado 5 proporciona un ejemplo completo de la captura de excepciones específicas en las que un programador podría estar interesado. El instanceofoperador no es necesario porque se capturan las excepciones específicas. Cada una de las excepciones controladas ( ClassNotFoundException, InstantiationException, IllegalAccessException) es capturado y tratado. El caso especial que produciría a ClassCastException(la clase se carga correctamente, pero no implementa la SomeInterfaceinterfaz) también se verifica al verificar esa excepción:

Listado 5

public SomeInterface buildInstance(String className) { SomeInterface impl = null; try { Class clazz = Class.forName(className); impl = (SomeInterface)clazz.newInstance(); } catch (ClassNotFoundException e) { log.error("Invalid class name: " + className + ", " + e.toString()); } catch (InstantiationException e) { log.error("Cannot create class: " + className + ", " + e.toString()); } catch (IllegalAccessException e) { log.error("Cannot create class: " + className + ", " + e.toString()); } catch (ClassCastException e) { log.error("Invalid class type, " + className + " does not implement " + SomeInterface.class.getName()); } return impl; } 

In some cases, it is preferable to rethrow a known exception (or perhaps create a new exception) than try to deal with it in the method. This allows the calling method to handle the error condition by putting the exception into a known context.

Listing 6 below provides an alternate version of the buildInterface() method, which throws a ClassNotFoundException if a problem occurs while loading and instantiating the class. In this example, the calling method is assured to receive either a properly instantiated object or an exception. Thus, the calling method does not need to check if the returned object is null.

Note that this example uses the Java 1.4 method of creating a new exception wrapped around another exception to preserve the original stack trace information. Otherwise, the stack trace would indicate the method buildInstance() as the method where the exception originated, instead of the underlying exception thrown by newInstance():

Listing 6

public SomeInterface buildInstance(String className) throws ClassNotFoundException { try { Class clazz = Class.forName(className); return (SomeInterface)clazz.newInstance(); } catch (ClassNotFoundException e) { log.error("Invalid class name: " + className + ", " + e.toString()); throw e; } catch (InstantiationException e) { throw new ClassNotFoundException("Cannot create class: " + className, e); } catch (IllegalAccessException e) { throw new ClassNotFoundException("Cannot create class: " + className, e); } catch (ClassCastException e) { throw new ClassNotFoundException(className + " does not implement " + SomeInterface.class.getName(), e); } } 

In some cases, the code may be able to recover from certain error conditions. In these cases, catching specific exceptions is important so the code can figure out whether a condition is recoverable. Look at the class instantiation example in Listing 6 with this in mind.

In Listing 7, the code returns a default object for an invalid className, but throws an exception for illegal operations, like an invalid cast or a security violation.

Note:IllegalClassException is a domain exception class mentioned here for demonstration purposes.

Listing 7

public SomeInterface buildInstance(String className) throws IllegalClassException { SomeInterface impl = null; try { Class clazz = Class.forName(className); return (SomeInterface)clazz.newInstance(); } catch (ClassNotFoundException e) { log.warn("Invalid class name: " + className + ", using default"); } catch (InstantiationException e) { log.warn("Invalid class name: " + className + ", using default"); } catch (IllegalAccessException e) { throw new IllegalClassException("Cannot create class: " + className, e); } catch (ClassCastException e) { throw new IllegalClassException(className + " does not implement " + SomeInterface.class.getName(), e); } if (impl == null) { impl = new DefaultImplemantation(); } return impl; } 

When generic Exceptions should be caught

Certain cases justify when it is handy, and required, to catch generic Exceptions. These cases are very specific, but important to large, failure-tolerant systems. In Listing 8, requests are read from a queue of requests and processed in order. But if any exceptions occur while the request is being processed (either a BadRequestException or any subclass of RuntimeException, including NullPointerException), then that exception will be caught outside the processing while loop. So any error causes the processing loop to stop, and any remaining requests will not be processed. That represents a poor way of handling an error during request processing:

Listing 8

public void processAllRequests () {Solicitud req = null; intente {while (verdadero) {req = getNextRequest (); if (req! = null) {processRequest (req); // lanza BadRequestException} else {// La cola de solicitudes está vacía, se debe hacer break; }}} catch (BadRequestException e) {log.error ("Solicitud no válida:" + req, e); }}