Es `atrapar (...) {lanzar; } `una mala práctica?

74

Si bien estoy de acuerdo en que atrapar ... sin volver a lanzar es realmente incorrecto, sin embargo, creo que usar construcciones como esta:

try
{
  // Stuff
}
catch (...)
{
  // Some cleanup
  throw;
}

Es aceptable en casos donde RAII no es aplicable . (Por favor, no pregunte ... no a todos en mi empresa les gusta la programación orientada a objetos y RAII a menudo se ve como "material escolar inútil" ...)

Mis compañeros de trabajo dicen que siempre debes saber qué excepciones se deben lanzar y que siempre puedes usar construcciones como:

try
{
  // Stuff
}
catch (exception_type1&)
{
  // Some cleanup
  throw;
}
catch (exception_type2&)
{
  // Some cleanup
  throw;
}
catch (exception_type3&)
{
  // Some cleanup
  throw;
}

¿Existe una buena práctica bien admitida con respecto a estas situaciones?

ereOn
fuente
3
@Pubby: No estoy seguro de que esta sea exactamente la misma pregunta. La pregunta vinculada es más sobre "¿Debo atrapar? ...", Mientras que mi pregunta se centra en "¿Debería atrapar ...o <specific exception>antes de volver a lanzar?"
antes del
53
Lamento decir eso, pero C ++ sin RAII no es C ++.
fredoverflow
46
Entonces, ¿sus trabajadores de vacas descartan la técnica que se inventó para tratar un problema determinado y luego discuten sobre cuál de las alternativas inferiores debería usarse? Lamento decirlo, pero eso parece estúpido , no importa de qué manera lo mire.
sbi
11
"atrapar ... sin volver a tirar está realmente mal", estás equivocado. En main, catch(...) { return EXIT_FAILURE; }podría estar bien en el código que no se ejecuta bajo un depurador. Si no atrapa, entonces la pila podría no ser desenrollada. Solo cuando su depurador detecta excepciones no detectadas quiere que se vayan main.
Steve Jessop
3
... así que incluso si se trata de un "error de programación", no necesariamente se deduce que no desea saber al respecto. De todos modos, sus colegas no son buenos profesionales del software, por lo que, como dice sbi, es muy difícil hablar sobre la mejor manera de lidiar con una situación que es crónicamente débil para empezar.
Steve Jessop

Respuestas:

196

Mis compañeros de trabajo dicen que siempre debes saber qué excepciones se deben lanzar [...]

Su compañero de trabajo, odio decirlo, obviamente nunca ha trabajado en bibliotecas de uso general.

¿Cómo en el mundo puede una clase como std::vectorincluso pretender saber lo que los constructores de copia tirarán, sin dejar de garantizar la seguridad de excepción?

Si siempre supieras lo que haría el llamado en tiempo de compilación, ¡entonces el polimorfismo sería inútil! A veces, el objetivo completo es abstraer lo que sucede en un nivel inferior, por lo que específicamente no desea saber qué está pasando.

Mehrdad
fuente
32
En realidad, incluso si supieran que se lanzarán excepciones. ¿Cuál es el propósito de esta duplicación de código? A menos que el manejo difiera, no veo ningún punto en enumerar las excepciones para mostrar su conocimiento.
Michael Krelin - hacker
3
@ MichaelKrelin-hacker: Eso también. Además, agréguele el hecho de que desaprobaron las especificaciones de excepción porque enumerar todas las excepciones posibles en el código tendió a causar errores más adelante ... es la peor idea de todas.
Mehrdad
44
Y lo que me molesta es cuál podría ser el origen de tal actitud cuando se combina con ver una técnica útil y conveniente como "material escolar inútil". Pero bueno ...
Michael Krelin - hacker
1
+1, la enumeración de todas las opciones posibles es una excelente receta para un futuro fracaso, ¿por qué alguien elegiría hacer eso ...?
littleadv 05 de
2
Buena respuesta. Posiblemente podría beneficiarse mencionar que si un compilador que uno tiene que soportar tiene un error en el área X, entonces usar la funcionalidad del área X no es inteligente, al menos no usarlo directamente. Por ejemplo, dada la información sobre la compañía, no me sorprendería si usaran Visual C ++ 6.0, que tenía algunos chinches en esta área (como los destructores de objetos de excepción llamados dos veces): algunos descendientes más pequeños de esos primeros errores sobrevivieron a este día, pero requieren arreglos cuidadosos para manifestarse.
Alf P. Steinbach
44

En lo que parece estar atrapado es en el infierno específico de alguien que trata de tener su pastel y comérselo también.

RAII y las excepciones están diseñadas para ir de la mano. RAII es el medio por el cual no tiene que escribir muchas catch(...)declaraciones para realizar la limpieza. Sucederá automáticamente, por supuesto. Y las excepciones son la única forma de trabajar con objetos RAII, porque los constructores solo pueden tener éxito o lanzar (o poner el objeto en un estado de error, pero ¿quién quiere eso?).

Una catchdeclaración puede hacer una de dos cosas: manejar un error o una circunstancia excepcional, o hacer un trabajo de limpieza. A veces hace ambas cosas, pero cada catchdeclaración existe para hacer al menos una de estas.

catch(...)es incapaz de hacer el manejo adecuado de excepciones. No sabes cuál es la excepción; No puede obtener información sobre la excepción. No tiene absolutamente ninguna otra información que no sea el hecho de que una excepción fue lanzada por algo dentro de un determinado bloque de código. Lo único legítimo que puedes hacer en ese bloque es hacer la limpieza. Y eso significa volver a lanzar la excepción al final de la limpieza.

Lo que RAII le brinda con respecto al manejo de excepciones es la limpieza gratuita. Si todo está encapsulado RAII correctamente, entonces todo se limpiará correctamente. Ya no necesita que las catchdeclaraciones hagan la limpieza. En cuyo caso, no hay razón para escribir una catch(...)declaración.

Así que estaría de acuerdo en que catch(...)es mayormente malvado ... provisionalmente .

Esa disposición es el uso adecuado de RAII. Porque sin ella, necesita ser capaz de hacer cierta limpieza. No hay forma de evitarlo; tienes que poder hacer el trabajo de limpieza. Debes poder asegurarte de que lanzar una excepción dejará el código en un estado razonable. Y catch(...)es una herramienta vital para hacerlo.

No puedes tener uno sin el otro. No se puede decir que tanto RAII como catch(...) son malos. Necesita al menos uno de estos; de lo contrario, no estás a salvo de excepciones.

Por supuesto, hay un uso válido, aunque raro, catch(...)que ni siquiera RAII puede desterrar: exception_ptrenviar un mensaje a otra persona. Normalmente a través de una promise/futureinterfaz o similar.

Mis compañeros de trabajo dicen que siempre debes saber qué excepciones se deben lanzar y que siempre puedes usar construcciones como:

Tu compañero de trabajo es un idiota (o simplemente terriblemente ignorante). Esto debería ser inmediatamente obvio debido a la cantidad de código de copiar y pegar que sugiere que escriba. La limpieza para cada una de esas declaraciones de captura será exactamente la misma . Esa es una pesadilla de mantenimiento, sin mencionar la legibilidad.

En resumen: este es el problema que RAII se creó para resolver (no es que no resuelva otros problemas).

Lo que me confunde acerca de esta noción es que generalmente es al revés de cómo la mayoría de las personas argumentan que RAII es malo. En general, el argumento dice "RAII es malo porque tienes que usar excepciones para señalar la falla del constructor. Pero no puedes lanzar excepciones, porque no es seguro y tendrás que tener muchas catchdeclaraciones para limpiar todo". Lo cual es un argumento roto porque RAII resuelve el problema que crea la falta de RAII.

Lo más probable es que esté en contra de RAII porque oculta detalles. Las llamadas de destructor no son visibles inmediatamente en las variables automáticas. Entonces obtienes código que se llama implícitamente. Algunos programadores realmente odian eso. Aparentemente, hasta el punto en que piensan que tener 3 catchdeclaraciones, todas las cuales hacen lo mismo con el código de copiar y pegar, es una mejor idea.

Nicol Bolas
fuente
2
Parece que no escribe código que ofrezca una fuerte garantía de seguridad de excepción. RAII ayuda a proporcionar una garantía básica . Pero para proporcionar una garantía sólida, debe deshacer algunas acciones para revertir el sistema al estado que tenía antes de que se llamara a la función. La garantía básica es la limpieza , la garantía sólida es la reversión . Hacer rollback es función específica. Entonces no puedes ponerlo en "RAII". Y es que cuando cajón de sastre bloque convierte en la mano. Si escribes código con una garantía sólida, usas catch-all mucho.
anton_rh
@anton_rh: Quizás, pero incluso en esos casos, las declaraciones generales son la herramienta de último recurso . La herramienta preferida es hacer todo lo que arroje antes de cambiar cualquier estado que tendría que revertir en una excepción. Obviamente, no puede implementar todo de esa manera en todos los casos, pero esa es la forma ideal de obtener la garantía de excepción fuerte.
Nicol Bolas
14

Dos comentarios, de verdad. La primera es que, si bien en un mundo ideal, siempre debe saber qué excepciones se pueden lanzar, en la práctica, si está tratando con bibliotecas de terceros o compilando con un compilador de Microsoft, no lo sabe. Más al punto, sin embargo; incluso si conoce exactamente todas las posibles excepciones, ¿eso es relevante aquí? catch (...)expresa la intención mucho mejor que catch ( std::exception const& ), incluso suponiendo que todas las posibles excepciones derivan de std::exception(que sería el caso en un mundo ideal). En cuanto al uso de varios bloques de captura, si no hay una base común para todas las excepciones: eso es francamente ofuscación y una pesadilla de mantenimiento. ¿Cómo reconoce que todos los comportamientos son idénticos? ¿Y esa era la intención? ¿Y qué sucede si tiene que cambiar el comportamiento (corrección de errores, por ejemplo)? Es muy fácil perderse uno.


fuente
3
En realidad, mi compañero de trabajo diseñó su propia clase de excepción que no se deriva std::exceptione intenta todos los días para imponer su uso entre nuestra base de código. Supongo que intenta castigarme por usar código y bibliotecas externas que no ha escrito él mismo.
antes del
17
@ereOn Me parece que su compañero de trabajo necesita urgentemente capacitación. En cualquier caso, probablemente evitaría usar bibliotecas escritas por él.
2
Las plantillas y saber qué excepciones se lanzarán van juntas como la mantequilla de maní y los geckos muertos. Algo tan simple como std::vector<>puede arrojar cualquier tipo de excepción por casi cualquier motivo.
David Thornley
3
Díganos, ¿cómo sabe exactamente qué excepción generará la corrección de errores de mañana más abajo en el árbol de llamadas?
mattnz
11

Creo que su compañero de trabajo ha mezclado algunos buenos consejos: solo debe manejar las excepciones conocidas en un catchbloque cuando no las vuelva a lanzar.

Esto significa:

try
{
  // Stuff
}
catch (...)
{
  // General stuff
}

Es malo porque oculta silenciosamente cualquier error.

Sin embargo:

try
{
  // Stuff
}
catch (exception_type_we_can_handle&)
{
  // Deal with the known exception
}

Está bien: sabemos a qué nos enfrentamos y no necesitamos exponerlo al código de llamada.

Igualmente:

try
{
  // Stuff
}
catch (...)
{
  // Rollback transactions, log errors, etc
  throw;
}

Está bien, incluso las mejores prácticas, el código para tratar los errores generales debe estar con el código que los causa. Es mejor que confiar en la persona que llama para saber que una transacción necesita revertirse o lo que sea.

Keith
fuente
9

Cualquier respuesta de o no debe ir acompañada de una justificación de por qué esto es así.

Decir que está mal simplemente porque me han enseñado de esa manera es solo un fanatismo ciego.

Escribir lo mismo //Some cleanup; throwvarias veces, como en su ejemplo, está mal porque es una duplicación de código y eso es una carga de mantenimiento. Escribirlo solo una vez es mejor.

Escribir un catch(...)para silenciar todas las excepciones es incorrecto porque solo debe manejar las excepciones que sabe cómo manejar, y con ese comodín puede catear más de lo que espera, y al hacerlo puede silenciar errores importantes.

Pero si vuelve a tirar después de un catch(...), entonces la última razón ya no se aplica, ya que en realidad no está manejando la excepción, por lo que no hay razón para desalentar esto.

En realidad, he hecho esto para iniciar sesión en funciones sensibles sin ningún problema:

void DoSomethingImportant()
{
    try
    {
        Log("Going to do something important");
        DoIt();
    }
    catch (std::exception &e)
    {
        Log("Error doing something important: %s", e.what());
        throw;
    }
    catch (...)
    {
        Log("Unexpected error doing something important");
        throw;
    }
    Log("Success doing something important");
}
pesófago
fuente
2
Esperemos que Log(...)no se pueda tirar.
Deduplicador
2

En general, estoy de acuerdo con el estado de ánimo de las publicaciones aquí, realmente no me gusta el patrón de capturar excepciones específicas: creo que la sintaxis de esto todavía está en su infancia y aún no puede hacer frente al código redundante.

Pero como todo el mundo dice eso, hablaré con el hecho de que, aunque los uso con moderación, a menudo he visto una de mis declaraciones de "captura (Excepción e)" y dije "Maldición, desearía haber llamado las excepciones específicas ese momento "porque cuando vienes más tarde, a menudo es bueno saber cuál era la intención y qué es probable que el cliente arroje de un vistazo.

No estoy justificando la actitud de "Usar siempre x", solo digo que ocasionalmente es bueno verlos en la lista y estoy seguro de que por eso algunas personas piensan que es el camino "correcto".

Bill K
fuente