java - practices - ¿Está volviendo una excepción un anti-patrón?
java rethrow exception best practice (11)
Ambos estan equivocados
Está bien, ya que necesitamos tratar el objeto más adelante en los dos primeros métodos de manera diferente y si el objeto Exception o boolean devuelto no cambia el flujo de trabajo en absoluto
No está bien. El concepto de excepciones significa que se lanzan, bueno, casos excepcionales. Están destinados a ser atrapados en el lugar donde serán manejados (o al menos se volverán a tirar después de una limpieza / registro local, etc.). No deben entregarse así (es decir, en el código de "dominio").
La gente se confundirá. Los errores reales pueden arrastrarlo fácilmente, por ejemplo, qué sucede si hay alguna Exception de alguna otra fuente que no sea la red aquí; ¿Una que no anticipó, que es realmente mala (como una excepción fuera de los límites que creó por algún error de programación)?
Y los nuevos programadores se confundirán sin cesar, y / o copiarán ese antipatrón en lugares a los que simplemente no pertenecen.
Por ejemplo, un colega recientemente implementó una interfaz bastante complicada (como en la interfaz de máquina a máquina, no la interface Java), e hizo un manejo de excepciones similar; conversión de excepciones a variaciones ignoradas silenciosamente (módulo algunos mensajes de registro). No hace falta decir que cualquier excepción que no esperaba realmente rompió todo el desorden de la peor manera imaginable; lo contrario de fallar rápido .
Es un antipatrón usar Excepciones para controlar el flujo de trabajo y solo debemos devolver el valor booleano desde serviceUp () y nunca el objeto Excepción en sí. El argumento es que el uso de excepciones para controlar el flujo de trabajo es un antipatrón.
Las excepciones ciertamente controlan el flujo de trabajo en el aspecto que a menudo abortan, o redirigen a un mensaje de error que se muestra al usuario. Es absolutamente posible tener alguna parte del código "deshabilitada" debido a una excepción; es decir, el manejo de excepciones seguramente se permite en otro lugar que no sea el nivel superior de control.
Pero las excepciones recurrentes son, de hecho, un anti-patrón; nadie espera que, es raro, conduce a errores falsos (es fácil ignorar el valor de retorno), etc., etc.
Por lo tanto, en el caso de su serviceUp() , serviceUp() void : funciona el 99% del tiempo o lanza una excepción; o boolean en boolean en el sentido de que acepta completamente que fallará en alguna parte. Si necesita pasar el mensaje de error, hágalo como una String , o guárdelo en algún lugar, o algo así, pero no lo use como valor de retorno, especialmente si desea volver a throw más tarde.
Solución fácil y estándar.
Esta solución es más corta (menos líneas, menos variables, menos if ), más simple, estándar de bog y hace exactamente lo que usted quería. Fácil de mantener, fácil de entender.
public void proceedWhenError() {
try {
serviceUp();
// do stuff (only when no exception)
}
catch (Exception exception) {
logger.debug("Exception happened, but it''s alright.", exception)
// do stuff (only when exception)
}
}
public void doNotProceedWhenError() {
try {
serviceUp();
// do stuff (only when no exception)
}
catch (Exception exception) {
// do stuff (only when exception)
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
private void serviceUp() {
service.connect();
}
Tengo dos métodos simples:
public void proceedWhenError() {
Throwable exception = serviceUp();
if (exception == null) {
// do stuff
} else {
logger.debug("Exception happened, but it''s alright.", exception)
// do stuff
}
}
public void doNotProceedWhenError() {
Throwable exception = serviceUp();
if (exception == null) {
// do stuff
} else {
// do stuff
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
El tercer método es un método auxiliar privado:
private Throwable serviceUp() {
try {
service.connect();
return null;
catch(Exception e) {
return e;
}
}
Tuvimos una pequeña charla con un colega mío sobre el patrón utilizado aquí:
devolviendo un objeto Exception (o Throwable ) del método serviceUp() .
La primera opinión:
Es un antipatrón usar Excepciones para controlar el flujo de trabajo y solo debemos devolver el valor booleano desde
serviceUp()y nunca el objeto Excepción en sí. El argumento es que el uso de excepciones para controlar el flujo de trabajo es un antipatrón.
La segunda opinión:
Está bien, ya que necesitamos tratar el objeto más adelante en los dos primeros métodos de manera diferente y si el objeto Exception o boolean devuelto no cambia el flujo de trabajo en absoluto
¿Crees que 1) o 2) es correcto y sobre todo, por qué? Tenga en cuenta que la pregunta SÓLO es sobre el método serviceUp() y su tipo de devolución: boolean vs Exception object.
Nota: no estoy cuestionando si usar objetos Throwable o Exception.
Como se mencionó en comentarios anteriores "La excepción es un evento", el objeto de excepción que obtenemos es solo un detalle del evento. Una vez que se atrapa una Excepción en el bloque "catch" y no se vuelve a lanzar, el evento termina. Publica que solo tienes un objeto de detalle, no una excepción, aunque el objeto es de clase excepción / lanzable.
Devolver el objeto de excepción puede ser útil debido a los detalles que tiene, pero agregará ambigüedad / confusión ya que el método de llamada no es "manejar la excepción y controlar el flujo basado en la excepción". Se trata simplemente de utilizar los detalles de un objeto devuelto para tomar decisiones.
Entonces, en mi opinión, una forma más lógica y menos confusa será devolver un boolean / enum basado en la excepción en lugar de simplemente devolver un objeto de excepción.
Es un antipatrón usar excepciones para dirigir el flujo solo cuando la excepción se produce en una situación no excepcional * . Por ejemplo, terminar un ciclo lanzando una excepción cuando llega al final de una colección es un antipatrón.
Controlar el flujo con excepciones reales, por otro lado, es una buena aplicación de excepciones. Si su método encuentra una situación excepcional que no puede manejar, debería lanzar una excepción, por lo tanto, redirigir el flujo en el llamador al bloque de control de excepciones.
Devolver un objeto Exception "desnudo" desde un método, en lugar de lanzarlo, es ciertamente contraintuitivo. Si necesita comunicar los resultados de una operación a la persona que llama, un mejor enfoque es utilizar un objeto de estado que contenga toda la información relevante, incluida la excepción:
public class CallStatus {
private final Exception serviceException;
private final boolean isSuccess;
public static final CallStatus SUCCESS = new CallStatus(null, true);
private CallStatus(Exception e, boolean s) {
serviceException = e;
isSuccess = s;
}
public boolean isSuccess() { return isSuccess; }
public Exception getServiceException() { return serviceException; }
public static CallStatus error(Exception e) {
return new CallStatus(e, false);
}
}
Ahora el llamante recibiría CallStatus de CallStatus :
CallStatus status = serviceUp();
if (status.isSuccess()) {
... // Do something
} else {
... // Do something else
Exception ex = status.getException();
}
Tenga en cuenta que el constructor es privado, por lo que CallStatus.SUCCESS devolverá CallStatus.SUCCESS o llamará a CallStatus.error(myException) .
* Lo que es excepcional y lo que no es excepcional depende en gran medida del contexto. Por ejemplo, los datos no numéricos causan una excepción en nextInt del nextInt , ya que considera que dichos datos no son válidos. Sin embargo, los mismos datos exactos no causan una excepción en el método hasNextInt , porque es perfectamente válido.
Este es un anti-patrón:
catch(Exception e) {
return e;
}
La única excusa razonable para devolver Throwable es proporcionar información detallada sobre fallas a una ruta rápida donde se espera una falla, sin pagar el precio del manejo de excepciones. Como beneficio adicional, facilita la conversión a una excepción lanzada si la persona que llama no sabe cómo manejar esta situación en particular.
Pero en su escenario, ese costo ya ha sido pagado. La captura de la excepción en nombre de su interlocutor no hace ningún favor a nadie.
1 Pedante, el costo directo de capturar una excepción (encontrar un manejador que coincida, desenrollar la pila) es bastante bajo o es necesario hacerlo de todos modos. La mayor parte del costo de try / catch comparado con un valor de retorno se debe a acciones incidentales, como la construcción de un seguimiento de pila. Por lo tanto, si los objetos de excepción no desplegados hacen un buen almacenamiento para datos de error eficientes depende de si este trabajo incidental se realiza en el momento de la construcción o en el momento del lanzamiento. Por lo tanto, si la devolución de un objeto de excepción es razonable puede diferir entre diferentes plataformas administradas.
Hay tres (idiomáticas) para manejar funciones que pueden fallar durante el tiempo de ejecución.
1. Devolver booleano
El primero es devolver el boolean . Este estilo se usa ampliamente en las API de estilo C y C, como PHP. Esto lleva, por ejemplo en PHP, a menudo a un código como este:
if (xyz_parse($data) === FALSE)
$error = xyz_last_error();
Las desventajas de esto son obvias, es por eso que ha pasado de moda cada vez más, especialmente en idiomas OOP e idiomas con manejo de excepciones.
2. Usa un objeto intermediario / estado / resultado
Si no usa excepciones, todavía tiene la oportunidad en los idiomas OOP para devolver objetos que describen el estado .
Por ejemplo, si recibe una entrada del usuario y desea validar la entrada, sabe de antemano que podría obtener basura y que el resultado no se validará con bastante frecuencia, y podría escribir un código como este:
ValidationResult result = parser.validate(data);
if (result.isValid())
// business logic
else
error = result.getvalidationError();
3. Utilizar excepciones.
Java hace esto como lo has mostrado con sockets. El razonamiento es que la creación de una conexión debería tener éxito y solo falla en circunstancias excepcionales que requieren un manejo especial.
Aplicándolo
En su caso, solo lanzaría la excepción directamente, e incluso descartaría el método auxiliar. Su método auxiliar tiene un mal nombre. En realidad, no consulta si el servicio está activo (como sugiere su nombre), sino que simplemente se conecta .
Utilizar excepciones (marcadas)
Asumamos que conectar es lo que realmente hace tu método. En este caso, lo llamamos más adecuadamente connectToService y hacemos que lance la excepción directamente:
public void connectToService() thows IOException {
// yada yada some init stuff, whatever
socket.connect();
}
public void proceedWhenError() {
try {
connectToService();
} else {
logger.debug("Exception happened, but it''s alright.", exception)
// do stuff
}
}
public void doNotProceedWhenError() throws IllegalStateException {
try {
connectToService();
// do stuff
}
catch(IOException e) {
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
Usar boolean
Por otro lado, podría ser posible que su serviceUp (que todavía tiene un nombre isServiceUp y que mejor se llame isServiceUp ) realmente consulte si el servicio se está ejecutando o no (lo que podría haber sido iniciado por otra persona). En este caso, usar un booleano sería la manera correcta
public boolean isServiceUp {
return ...; // query the service however you need
}
Esto, por supuesto, se refiere a la primera solución del uso de booleanos y no es realmente útil cuando también necesita saber por qué el servicio no está funcionando.
Usar objetos intermedios / resultados / estado
Así que descartemos este enfoque limitado y lo modifiquemos utilizando un objeto intermediario / resultado / estado:
class ServiceState {
enum State { CONNECTED, WAITING, CLOSED; }
State state;
IOException exception;
String additionalMessage;
public ServiceState (State state, IOException ex, String msg) {
[...] // ommitted for brevity
}
// getters ommitted for brevity
}
La función auxiliar se convertiría en getServiceState :
public ServiceState getServiceState() {
// query the service state however you need & return
}
public void proceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() == State.CONNECTED) {
// do stuff
} else {
logger.debug("Exception happened, but it''s alright.", state.getException())
// do stuff
}
}
public void doNotProceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() == State.CONNECTED) {
// do stuff
} else {
// do stuff
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", exception)
}
}
Tenga en cuenta que también se está repitiendo en el proceso Si el proceedWhenError , todo el código podría simplificarse para:
public void proceedWhenError() {
ServiceState state = getServiceState();
if (state.getState() != State.CONNECTED) {
logger.debug("Exception happened, but it''s alright.", state.getException())
}
// do stuff
}
Hay un poco de debate sobre cuándo usar el segundo caso y cuándo usar el tercero. Algunas personas creen que las excepciones deben ser excepcionales y que usted no debe diseñar teniendo en cuenta la posibilidad de las excepciones, y casi siempre usará la segunda opción. Eso está bien. Pero hemos comprobado las excepciones en Java, por lo que no veo ninguna razón para no usarlas. Uso excepciones comprobadas cuando se supone que la llamada debería tener éxito (como usar un socket), pero es posible que se produzcan fallos, y uso la segunda opción cuando no está claro si la llamada debería tener éxito (como la validación de datos). Pero hay diferentes opiniones sobre esto.
También depende de lo que realmente haga su método de ayuda. isServiceUp implica que consulta algún estado , pero no lo cambia. En ese caso, devolver un objeto de estado es obviamente la forma idiomática de manejar esto.
Pero su implementación muestra que el método auxiliar se conecta al servicio. En cuyo caso, al menos en java, la forma idiomática de manejarlo sería lanzar una excepción (marcada), pero también podría justificar el uso de un objeto de resultado.
Sin embargo, no es aconsejable devolver boolean , ya que es demasiado limitado. Si todo lo que necesita saber si el servicio se está ejecutando o no (y no está interesado en la causa), tal método podría seguir siendo útil (y podría implementarse como un método auxiliar que simplemente return getServiceState() == State.SUCCESS ).
Es un antipatrón usar Excepciones para controlar el flujo de trabajo
Entonces, veamos, ¿qué es una excepción?
Definición: Una excepción es un evento, que ocurre durante la ejecución de un programa, que interrumpe el flujo normal de las instrucciones del programa.
Fuente: https://docs.oracle.com/javase/tutorial/essential/exceptions/definition.html
La definición misma de una excepción es que es un evento que desactiva (y por lo tanto cambia) el flujo de trabajo del programa. Si usarlo como se define es un anti-patrón, entonces toda la característica del lenguaje en sí misma debe considerarse un anti-patrón. Hay personas que creen que las excepciones son malas, y algunas tienen algunos argumentos válidos para ello, pero en general, las excepciones se consideran construcciones útiles y se consideran herramientas útiles para manejar flujos de trabajo excepcionales.
Lanzar una excepción ciertamente no es un anti-patrón. Devolviéndolo , por otro lado, es. Por lo tanto, sí, su código, tal como se presenta, no es idiomático.
La disonancia cognitiva obvia es el anti-patrón aquí. Un lector lo verá usando excepciones para el control de flujo y un desarrollador intentará de inmediato recodificarlo para que no lo haga.
Mi instinto sugiere un enfoque como:
// Use an action name instead of a question here because it IS an action.
private void bringServiceUp() throws Exception {
}
// Encapsulate both a success and a failure into the result.
class Result {
final boolean success;
final Exception failure;
private Result(boolean success, Exception failure) {
this.success = success;
this.failure = failure;
}
Result(boolean success) {
this(success, null);
}
Result(Exception exception) {
this(false, exception);
}
public boolean wasSuccessful() {
return success;
}
public Exception getFailure() {
return failure;
}
}
// No more cognitive dissonance.
private Result tryToBringServiceUp() {
try {
bringServiceUp();
} catch (Exception e) {
return new Result(e);
}
return new Result(true);
}
// Now these two are fine.
public void proceedWhenError() {
Result result = tryToBringServiceUp();
if (result.wasSuccessful()) {
// do stuff
} else {
logger.debug("Exception happened, but it''s alright.", result.getFailure());
// do stuff
}
}
public void doNotProceedWhenError() throws IllegalStateException {
Result result = tryToBringServiceUp();
if (result.wasSuccessful()) {
// do stuff
} else {
// do stuff
throw new IllegalStateException("Oh, we cannot proceed. The service is not up.", result.getFailure());
}
}
La segunda opinión ("está bien") no es válida. El código no está bien porque devolver excepciones en lugar de lanzarlas no es realmente idiomático.
Tampoco compro la primera opinión ("el uso de Excepciones para controlar el flujo de trabajo es antipatrónico"). service.connect() está lanzando una excepción y tiene que reaccionar a esta excepción, por lo que este es efectivamente un control de flujo. Devolver un objeto boolean u otro objeto de estado y procesarlo en lugar de manejar una excepción, y pensar que no es un flujo de control basado en una excepción es ingenuo. Otra desventaja es que si decide volver a emitir una excepción ( IllegalArgumentException en IllegalArgumentException o lo que sea), ya no tendrá la excepción original. Y esto es extremadamente malo cuando intentas analizar lo que realmente sucedió.
Así que haría el clásico:
- Lanzar la excepción en
serviceUp. - En los métodos que invocan
serviceUp:-
try-catch, log debug y tragar la excepción si desea continuar con la excepción. -
try-catchy rethrow la excepción envuelta en otra excepción que proporciona más información sobre lo que sucedió. Alternativamente, simplemente deje que la excepción original se propague a través dethrowssi no puede agregar nada sustancial.
-
Es muy importante no perder la excepción original.
No puedes decir que 2) la opinión es falsa, porque el flujo de trabajo se ejecutará como quieres que se ejecute. Desde un punto de vista lógico, se ejecutará como se desee, por lo que es correcto.
Pero es una forma realmente extraña y no recomendable de hacerlo. Primero, porque las Excepciones no están diseñadas para hacer eso (lo que significa que estás haciendo un anti-patrón). Es un objeto específico diseñado para ser lanzado y atrapado. Así que es extraño, más bien usarlo tal como está diseñado, elegir para devolverlo y atraparlo usar un "if" en él. Además, (quizás sea despreciable) se enfrentará a un problema de rendimiento en lugar de utilizar un simple valor lógico booleano como indicador, se crea una instancia de un objeto completo.
Finalmente, tampoco se recomienda que la función de causa deba devolver algo si el objetivo de la función es obtener algo (que no es su caso). Debería rediseñarlo para que sea una función que inicie un servicio, luego no devolverá nada porque no se llamará para obtener algo y generará una excepción si se produce un error. Y si desea saber si su trabajo de servicio, cree una función diseñada para proporcionarle esta información, como public boolean isServiceStarted() .
Si los llamadores de un método esperan que una operación pueda tener éxito o falle, y están preparados para manejar cualquiera de los casos, entonces el método debe indicar, generalmente a través del valor de retorno, si la operación falló. Si las personas que llaman no están preparadas para manejar la falla de manera útil, entonces el método por el cual ocurrió la falla debe lanzar la excepción en lugar de requerir que las personas que llaman agreguen código para hacerlo.
La única complicación es que algunos métodos tendrán algunos llamadores que están preparados para manejar las fallas con gracia y otros que no lo están. Mi método preferido sería que tales métodos acepten una devolución de llamada opcional que se invoca en caso de falla; Si no se proporciona ninguna devolución de llamada, el comportamiento predeterminado debe ser lanzar una excepción. Un enfoque de este tipo ahorraría el costo de construir una excepción en los casos en que una persona que llama esté preparada para manejar una falla, mientras minimiza la carga para las personas que llaman que no lo son. La mayor dificultad con un diseño de este tipo es decidir qué parámetros debería tomar tal devolución de llamada, ya que es más difícil cambiar esos parámetros más adelante.
devolver una excepción es de hecho un patrón porque las excepciones deben reservarse para errores en la ejecución, no para describir la condición del servicio.
imagine si hay un error en el código de serviceUp() que hace que se lance NullPointerException . Ahora imagine que el error está en el servicio y se NullPointerException la misma NullPointerException desde connect() .
¿Ves mi punto?
Otra razón es el cambio de requisitos.
Actualmente, el servicio tiene dos condiciones: hacia arriba o hacia abajo.
Actualmente.
Mañana, tendrá tres condiciones para el servicio: arriba, abajo. o funcionando con advertencias. El día después, también querrá que el método devuelva detalles sobre el servicio en json .....
ServiceState un ServiceState que puede ser, por ejemplo, WAITING RUNNING , WAITING , CLOSED . El método se llamaría getServiceState .
enum ServiceState { RUNNING, WAITING, CLOSED; }
Nunca he visto los métodos que devuelven una excepción como resultado de la ejecución. Para mí, cuando un método devuelve el valor, significa que el método terminó su trabajo sin problemas . No quiero recuperar el resultado y analizarlo si contiene algún error. El resultado en sí mismo significa que no se produjeron fallos , todo salió como estaba previsto.
Por otro lado, cuando el método lanza una excepción, necesito analizar un objeto especial para averiguar qué salió mal. No analizo el resultado, porque no hay resultado .
Un ejemplo:
public void proceedWhenError() {
final ServiceState state = getServiceState();
if (state != ServiceState.RUNNING) {
logger.debug("The service is not running, but it''s alright.");
}
// do stuff
}
public void doNotProceedWhenError() {
final ServiceState state = getServiceState();
if (state != ServiceState.RUNNING) {
throw new IllegalStateException("The service is not running...");
}
// do stuff
}
private ServiceState getServiceState() {
try {
service.connect();
return ServiceState.RUNNING;
catch(Exception e) {
// determine the state by parsing the exception
// and return it
return getStateFromException(e);
}
}
Si las excepciones generadas por el servicio son importantes y / o se procesan en otro lugar, junto con el estado podrían guardarse en un objeto ServiceResponse :
class ServiceResponse {
private final ServiceState state;
private final Exception exception;
public ServiceResponse(ServiceState state, Exception exception) {
this.state = state;
this.exception = exception;
}
public static ServiceResponse of(ServiceState state) {
return new ServiceResponse(state, null);
}
public static ServiceResponse of(Exception exception) {
return new ServiceResponse(null, exception);
}
public ServiceState getState() {
return state;
}
public Exception getException() {
return exception;
}
}
Ahora, con ServiceResponse , estos métodos pueden parecer:
public void proceedWhenError() {
final ServiceResponse response = getServiceResponse();
final ServiceState state = response.getState();
final Exception exception = response.getException();
if (state != ServiceState.RUNNING) {
logger.debug("The service is not running, but it''s alright.", exception);
}
// do stuff
}
public void doNotProceedWhenError() {
final ServiceResponse response = getServiceResponse();
final ServiceState state = response.getState();
final Exception exception = response.getException();
if (state != ServiceState.RUNNING) {
throw new IllegalStateException("The service is not running...", exception);
}
// do stuff
}
private ServiceResponse getServiceResponse() {
try {
service.connect();
return ServiceResponse.of(ServiceState.RUNNING);
catch(Exception e) {
// or -> return ServiceResponse.of(e);
return new ServiceResponse(getStateFromException(e), e);
}
}