¿Deberían favorecerse los tipos de excepciones reutilizados sobre los de un solo uso?

8

Digamos que tengo Doors que son administrados por a DoorService. El DoorServicese encarga de abrir, cerrar y bloquear las puertas que están almacenadas en la base de datos.

public interface DoorService
{
    void open(Door door) throws DoorLockedException, DoorAlreadyOpenedException;

    void close(Door door) throws DoorAlreadyClosedException;

    /**
     * Closes the door if open
     */
    void lock(Door door) throws DoorAlreadyLockedException;
}

Para el método de bloqueo, hay un DoorAlreadyLockedExceptiony para el método abierto hay un DoorLockedException. Esta es una opción, pero hay otras opciones posibles:

1) Usar DoorLockedExceptionpara todo, es un poco incómodo al detectar la excepción en una lock()llamada

try
{
    doorService.lock(myDoor);
}
catch(DoorLockedException ex) // door ALREADY locked
{
    //error handling...
}

2) Tener los 2 tipos de excepciones DoorLockedExceptionyDoorAlreadyLockedException

3) Tener los 2 tipos de excepciones pero dejar DoorAlreadyLockedException extends DoorLockedException

¿Cuál es la mejor solución y por qué?

Invierno
fuente
1
¿Cómo se define "mejor"? En C # ni siquiera tenemos excepciones marcadas. Creemos que esto hace que nuestro desarrollo sea más simple y más confiable.
Robert Harvey
2
¿Qué tan diferentes son las resoluciones de AlreadyLocked and Locked?
whatsisname
@RobertHarvey Considero que las excepciones son parte de la lógica del método. La mejor solución es la más explícita y lógica. A diferencia de C #, estamos promoviendo la explicitación sobre la conveniencia al forzar la captura de casos excepcionales y asegurarnos de que nadie llame a un método sin tener una comprensión completa de lo que está sucediendo.
Invierno
1
Estos métodos simplemente deberían devolver un código de estado. Puede informar a sus usuarios sobre la base del código que obtiene y obtener un aumento de rendimiento de forma gratuita (las excepciones son costosas, entre 1000 y 10000 veces más caras que devolver un código de estado).
Robert Harvey
1
@RobertHarvey "las excepciones son caras, entre 1000 y 10000 veces más caras que devolver un código de estado" He hecho un análisis sobre esto en Java y el costo de las excepciones a menudo se ha exagerado. "Pueden" ser caros si tienes pilas muy profundas. IIRC, tuve que aumentar significativamente el tamaño máximo de la pila para poder crear pilas lo suficientemente grandes como para hacer excepciones lo suficientemente lentas como para preocuparme. Incluso los cerdos apilados como Spring no hacen eso. Siempre y cuando no esté lanzando y atrapando bucles estrechos, es una optimización prematura para evitar excepciones.
JimmyJames

Respuestas:

21

Sé que las personas hacen un gran problema al usar excepciones para el control de flujo, pero ese no es el mayor problema aquí. Veo una violación de separación de consulta de comando enorme . Pero incluso eso no es lo peor.

No, lo peor está aquí:

/**
 * Closes the door if open
 */

¿Por qué demonios explota todo lo demás si su suposición sobre el estado de las puertas es incorrecta, pero lock()solo la arregla por usted? Olvídese de que esto hace que sea imposible cerrar la puerta, lo cual es absolutamente posible y ocasionalmente útil. No, el problema aquí es que has mezclado dos filosofías diferentes para tratar con suposiciones incorrectas. Eso es confuso No hagas eso. No al mismo nivel de abstracción con el mismo estilo de denominación. ¡Ay! Tome una de esas ideas afuera. Los métodos de servicio de la puerta deberían funcionar de la misma manera.

En cuanto a la violación de Separación de consulta de comando, no debería tener que intentar cerrar una puerta para averiguar si está cerrada o abierta. Debería poder preguntar. El servicio de puerta no proporciona una manera de hacerlo sin posiblemente cambiar el estado de la puerta. Eso hace que esto sea mucho peor que los comandos que también devuelven valores (un malentendido común de lo que se trata CQS). ¡Aquí, los comandos de cambio de estado son la única forma de hacer consultas! ¡Ay!

En cuanto a que las excepciones son más caras que los códigos de estado, eso es hablar de optimización. Lo suficientemente rápido es lo suficientemente rápido. No, el verdadero problema es que los humanos no esperan excepciones para casos típicos. Puedes discutir sobre lo que es típico todo lo que te gusta. Para mí, la gran pregunta es qué tan legible está haciendo el código de uso.

ensureClosed(DoorService service, Door door){
  // Need door closed and unlocked. No idea of its state. What to do?
  try {
    service.open(door)
    service.close(door)
  } 
  catch( DoorLockedException e ){
    //Have no way to unlock the door so give up and die
    log(e);
    throw new NoOneGaveMeAKeyException(e);      
  }
  catch( DoorAlreadyOpenedException e ){
    try { 
      service.close(door);
    }
    catch( DoorAlreadyClosedException e ){
      //Some multithreaded goof has been messing with our door.
      //Oh well, this is what we wanted anyway.
      //Hope they didn't lock it.
    }
  }
}

Por favor, no me hagas escribir código como este. Por favor dénos isLocked()y isClosed()métodos. Con ellos puedo escribir los míos ensureClosed()y los ensureUnlocked()métodos que son fáciles de leer. Los que solo tiran si se violan sus condiciones de publicación. Prefiero descubrir que ya los ha escrito y probado, por supuesto. Simplemente no los mezcle con los que arrojan cuando no pueden cambiar el estado. Como mínimo, deles nombres distintivos.

Hagas lo que hagas, no llames a nada tryClose(). Ese es un nombre terrible.

En cuanto a DoorLockedExceptionsolo vs también tener DoorAlreadyLockedExceptionmal decir esto: se trata de usar el código. No diseñe servicios como este sin escribir el código de uso y mirar el desorden que está creando. Refactorice y rediseñe hasta que el código de uso sea al menos legible. De hecho, considere escribir primero el código de uso.

ensureClosed(DoorService service, Door door){
  if( !service.isClosed(door) ){
    try{
      service.close(door);
    }
    catch( DoorAlreadyClosedException e ){
      //Some multithreaded goof has been messing with our door.
      //Oh well, this is what we wanted anyway.
      //Hope they didn't lock it.
    }
  } else {
    //This is what you wanted, so quietly do nothing. 
    //Why are you even here? Who bothers to write empty else conditions?
  }
}

ensureUnlocked(DoorService service, Door door){
  if( service.islocked(door) ){
    throw new NoOneGaveMeAKeyException(); 
  }
}
naranja confitada
fuente
El lock()método que close()me llamaba era que yo fuera flojo cuando escribía la pregunta, ¡pero gracias por hacerme consciente de eso! No caeré en ello cuando escriba código real. En mi caso, los 3 métodos nunca se llaman sucesivamente. Es solo: llame a uno, si no hay una buena excepción, de lo contrario, muestre un diálogo traducido y posiblemente cierre la sesión (si InvalidTokenException). Tengo isLocked()y isOpen()métodos, y se usan en el flujo de control del servidor, pero no se deben usar como una forma de verificar si se debe mostrar un cuadro de diálogo de error, ese es un trabajo para una excepción.
Invierno
Para su último punto, sí, el código de uso se ve un poco mejor, DoorAlreadyLockedExceptionpero hace clases gemelas.
Invierno
2
Si por clases individuales que quiere decir las excepciones sí deja que te enseñe mi uso favorito de la herencia: public class DoorAlreadyLockedException inherits DoorLockedException {}. Defina una clase en una línea solo por darle un buen nombre. Elige lo que heredas sabiamente. Prefiero heredar lo más alto que puedo, sin ser innecesariamente redundante, para evitar una larga cadena de herencia.
candied_orange
Tengo ganas de cambiarle el nombre NoOneGaveMeAKeya NoOneGaveMeAKeyExceptionsolo para ser pendantic. De lo contrario, estoy de acuerdo con el punto principal, que es que antes del procesamiento, debemos poder conocer el estado del objeto.
Walfrat
2
Abogado del diablo: solo tener isOpen/ isClosedno es lo suficientemente bueno. Piensa en el sistema de archivos. Puede verificar que el archivo exista / no exista, pero eso aún puede cambiar para cuando intente leerlo o escribirlo, y se IOExceptionobtendrá un resultado. Tal vez en este contexto realmente existe la posibilidad de que el estado no sea válido al abrir o cerrar la puerta.
Tom G
8

Hacer una distinción DoorLockedExceptiony DoorAlreadyLockedExceptionsugiere que se están utilizando diferentes tipos de excepción para el control de flujo, una práctica que ya se considera ampliamente un antipatrón.

Tener múltiples tipos de excepción para algo tan benigno es gratuito. La complejidad adicional no se ve compensada por los beneficios adicionales.

Solo úsalo DoorLockedExceptionpara todo.

Mejor aún, simplemente no hagas nada. Si la puerta ya está cerrada, seguirá estando en el estado correcto cuando lock()se complete el método. No arroje excepciones para condiciones que no sean notables.

Si todavía te sientes incómodo con eso, cambia el nombre de tu método ensureDoorLocked().

Robert Harvey
fuente
44
Por favor, no use en exceso el meme "excepciones para el control de flujo", si las dos situaciones, por alguna razón, justifican drásticamente diferentes cursos de acción para resolver, entonces tener diferentes excepciones tiene sentido. Sin embargo, no está claro en el ejemplo si ese es el caso.
cuál es el
1
@whatsisname: si se trata de un meme que no tiene ninguna base, publique su propia respuesta a tal efecto.
Robert Harvey
1
@RobertHarvey Las excepciones se utilizan para el "flujo de control" de los cuadros de diálogo y el registro de errores. La aplicación lógica principal obviamente no se ejecuta a través de las cláusulas catch. No creo que hacer una distinción entre DoorLockedExceptiony DoorAlreadyLockedExceptionsugiera que se trata de tener un controlador de errores mucho más significativo para el lock()método a costa de tener una clase casi duplicada.
Invierno
1
Si llama a lock () y la puerta está cerrada, ya sabe que la puerta ya estaba cerrada. No necesita que le digan lo que ya sabe.
Robert Harvey
2
@ewan: Oh, ffs. Llámalo como quieras; una enumeración, un resultado, lo que sea. Es el enfoque correcto.
Robert Harvey
3

El tema de esta pregunta "¿Deberían favorecerse los tipos de excepciones reciclados sobre los de un solo uso?" parece haber sido ignorado, no solo en las respuestas, sino también en los detalles de las preguntas (las respuestas fueron a los detalles de la pregunta).

Estoy de acuerdo con la mayoría de las críticas al código que los encuestados han ofrecido.

Pero como respuesta a la pregunta principal, diría que debería preferir la reutilización de las 'excepciones recicladas' sobre las 'de un solo uso'.

En el uso más típico del manejo de excepciones, tiene una gran DISTANCIA en el código entre el lugar donde se detecta y se lanza la excepción, y el lugar donde se captura y maneja la excepción. Eso significa que el uso de tipos privados (modulares) en el manejo de excepciones generalmente no funcionará bien. Un programa típico con manejo de excepciones: tendrá un puñado de ubicaciones de nivel superior en el código donde se manejan las excepciones. Y como se trata de ubicaciones de nivel superior, es difícil para ellos tener un conocimiento detallado sobre aspectos estrechos de módulos particulares.

En cambio, es probable que tengan controladores de alto nivel para algunos problemas de alto nivel.

La única razón por la que el uso de clases de excepción personalizadas detalladas parece tener sentido en su muestra es porque (y aquí estoy parroteando a los otros encuestados), no debería usar el manejo de excepciones para el flujo de control ordinario.

Lewis Pringle
fuente
1
"En el uso más típico del manejo de excepciones, tienes una gran DISTANCIA" No estamos analizando lo que es típico. Estamos diseñando una forma para que funcione. Muchos buenos diseños capturan sus excepciones en el siguiente nivel en la pila de llamadas y en un alcance extremadamente limitado. Eso no debe descartarse como control de flujo cuando todo lo que hacen es recuperarse del problema. Los mejores diseños de excepción comienzan por planificar cómo recuperarse antes de definir las excepciones. Entonces, los nombres de excepción no tienen que ser sobre informar el problema. Se trata de encontrar la solución. Puede usar la cadena msg para informar el problema.
candied_orange
"No estamos analizando lo que es típico. Estamos diseñando una forma para que funcione", acordó, eso es lo que estaba haciendo. No es lo que estaba haciendo. Si nos fijamos en el titular, hizo una pregunta. Si observa el cuerpo del mensaje, hizo tres preguntas diferentes (relacionadas). Respondiste las preguntas del cuerpo (hasta cierto punto, realmente, más criticaste el código de muestra). Respondí la pregunta del titular.
Lewis Pringle
Por cierto, usted afirma "Muchos buenos diseños capturan sus excepciones en el siguiente nivel en la pila de llamadas y en un alcance extremadamente limitado". No estoy seguro de estar totalmente en desacuerdo con eso. Pero debo decir que, después de 40 años de programación, me está costando encontrar un ejemplo que respalde esa afirmación. Me doy cuenta de que es un poco derivado (y tal vez no esté manejando esto correctamente, nuevo en stackoverflow), pero ¿le importaría ofrecer un ejemplo o dos donde tal manejo localizado de prueba / captura sea la mejor solución? Ciertamente no es un patrón típico / común.
Lewis Pringle
Un ejemplo ¿eh? Permíteme presentarte a Jon Skeet
candied_orange
OK, eso no es en absoluto un ejemplo de lo que reclamaste. Usted dijo "Muchos buenos diseños capturan sus excepciones en el siguiente nivel en la pila de llamadas y en un alcance extremadamente limitado". Su ejemplo fue simplemente traducir una API que solo produjo excepciones por error en una que produjo algún valor centinela. Tal vez esto es solo una diferencia de redacción entre nosotros dos. No lo describiría como un "buen diseño" sino un "truco localizado para evitar una elección de diseño inconveniente en una herramienta que está utilizando".
Lewis Pringle
0

Una alternativa inexplorada. Es posible que esté modelando lo incorrecto con sus clases, ¿y si tuviera estos en su lugar?

public interface OpenDoor
{
    public ClosedDoor close();
    public LockedDoor lock();
}

public interface ClosedDoor
{
    public OpenDoor open();
    public LockedDoor lock();
}

public interface LockedDoor
{
    // ? unlock? open?
}

Ahora es imposible intentar algo que sea un error. No necesitas ninguna excepción.

Caleth
fuente
Pero en la mayoría de las aplicaciones no puede mantener la referencia a esos objetos, ya que la base de datos se puede cambiar en cualquier momento. Sin embargo, buen esfuerzo creativo
Invierno
0

Contestaré la pregunta original, en lugar de criticar el ejemplo.

En pocas palabras, si espera que el cliente deba reaccionar de manera diferente para cada caso, garantiza un nuevo tipo de excepción.

Si decide usar varios tipos de excepción y, sin embargo, espera que el cliente en algunos casos quiera crear una cláusula catch común para ellos, probablemente debería crear una superclase abstracta que ambos extiendan.

Gudmundur Orn
fuente
0

Este es un seudocódigo, pero creo que entenderás lo que quiero decir:

public interface DoorService
{
    // ensures the door is open, uses key if locked, returns false if lock without a key or key does not fit
    bool Open(Door door, optional Key key) throws DoorStuckException, HouseOnFireException;

    // closes the door if open. already closed doors stay closed. Locked status does not change.
    void Close(Door door) throws throws DoorStuckException, HouseOnFireException;

    // closes the door if open and locks it
    void CloseAndLock(Door door, Key key) throws DoorStuckException, HouseOnFireException;
}

Sí, la reutilización del tipo de excepción tiene sentido, si usa excepciones para cosas verdaderamente excepcionales . Porque las cosas verdaderamente excepcionales son en su mayoría preocupaciones transversales. Estaba utilizando Excepciones básicamente para flujos de control y eso conduce a esos problemas.

nvoigt
fuente
Pero esos tipos de excepción son realmente no descriptivos. ¿Qué significa siquiera tener una puerta "atascada" cuando solo puede abrir, cerrar o cerrar una puerta?
Invierno
Esto es solo tragar las excepciones y no hace nada con ellas. Esto no es apropiado en un contexto en el que no quieres saber qué está pasando. Esto se siente como la forma de C # de hacer mi ejemplo de Java. Hay una razón por la que estoy trabajando con Java.
Invierno
Bueno, eso está atascado. No se puede mover. Por lo tanto, no puede abrirlo y no puede cerrarlo. No es un estado lógico que necesita corregir llamando a otro método, es un estado excepcional que no puede resolver a través del servicio de puerta.
nvoigt
Y no está tragando nada , no está produciendo una excepción para un estado de programa no excepcional. Si mi esposa me pide que cierre la puerta del balcón y me levanto y descubro que ya está cerrada, no corro y grito y me asusto. Solo regreso y digo "está cerrado ahora".
nvoigt
Exactamente, con las excepciones marcadas, la persona que llama debe atrapar la excepción al inicio del programa. En la llamada, simplemente ignora la excepción o registra una advertencia y está bien. También lo que llama "excepciones excepcionales" son solo RuntimeExceptions. DoorStuckExceptiony HouseOnFireExceptionno deben verificarse, no deben capturarse en todas las llamadas.
Invierno