¿Qué es mejor IllegalStateException o ejecución de método silencioso? [cerrado]

16

Digamos que tengo una clase MediaPlayer que tiene métodos play () y stop (). ¿Cuál es la mejor estrategia para usar al implementar el método de detención en caso de que el método de reproducción no haya sido llamado antes? Veo dos opciones: lanzar una excepción porque el jugador no está en el estado apropiado o ignorar silenciosamente las llamadas al método de detención.

¿Cuál debería ser la regla general cuando no se debe llamar a un método en alguna situación, pero su ejecución no daña el programa en general?

x2bool
fuente
21
No use excepciones para modelar comportamientos que no sean excepcionales.
Alexander - Restablece a Mónica
1
¡Oye! No creo que sea un duplicado. Mi pregunta es más sobre IllegalStateException y la pregunta anterior es sobre el uso de excepciones en general.
x2bool
1
En lugar de simplemente fallar silenciosamente, ¿por qué NO TAMBIÉN registrar la anomalía como una advertencia? Seguramente, el Java que está utilizando admite diferentes niveles de registro (ERROR, WARN, INFO, DEBUG).
Eric Seastrand
33
Tenga en cuenta que Set.add no produce una excepción si el elemento ya está en el conjunto. Su propósito es "garantizar que el elemento esté en el conjunto", en lugar de "agregar el elemento al conjunto", de modo que logre su propósito al no hacer nada si el elemento ya está en el conjunto.
user253751
2
Palabra del día: idempotencia
Julio

Respuestas:

37

No hay regla Depende totalmente de cómo desea que su API "se sienta".

Personalmente, en un reproductor de música, creo que una transición del estado Stoppeda Stoppedtravés del método Stop()es una transición de estado perfectamente válida. No es muy significativo, pero es válido. Con esto en mente, lanzar una excepción parecería pedante e injusto. Haría que la API parezca el equivalente social de hablar con el niño molesto en el autobús escolar. Estás atrapado con la molesta situación, pero puedes lidiar con ella.

Un enfoque más "sociable" es reconocer que la transición es, en el peor de los casos, inofensiva y ser amable con sus desarrolladores consumidores permitiéndola.

Entonces, si fuera por mí, me saltaría la excepción.

MetaFight
fuente
15
Esta respuesta nos recuerda que a veces es una buena idea esbozar las transiciones de estado incluso para interacciones simples.
66
Con esto en mente, lanzar una excepción parecería pedante e injusto. Haría que la API parezca el equivalente social de hablar con el niño molesto en el autobús escolar. > Las excepciones no están diseñadas para castigarte: están diseñadas para decirte que algo inesperado ha sucedido. Personalmente, estaría muy agradecido por un MediaPlayer.stop()método que arrojó un IllegalStateExceptioncódigo de depuración en lugar de pasar horas y me pregunto por qué demonios "no pasa nada" (es decir, "simplemente no funciona").
errantlinguist
33
Por supuesto, si su diseño dice que la transición de bucle invertido no es válida, entonces SI debería lanzar una excepción. Sin embargo, mi respuesta fue sobre un diseño donde esa transición está perfectamente bien.
MetaFight
1
StopOrThrow()suena horrible Si vas por ese callejón, ¿por qué no usar el patrón estándar de TryStop()? Además, en general, cuando el comportamiento de una API no está claro (como la mayoría de ellos), no se espera que los desarrolladores simplemente adivinen o experimenten, se espera que vean la documentación. Por eso me gusta tanto MSDN.
MetaFight
1
Personalmente, prefiero la opción de falla rápida, por lo que elegiría IllegalStateException para indicarle al usuario de la API que hay algo mal en su lógica, incluso si es inofensivo. A menudo obtengo esas excepciones de mi propio código, realmente útil para detectar que mi trabajo no terminado ya está mal
Walfrat
17

Hay dos tipos distintos de acciones que uno puede desear realizar:

  1. Simultáneamente pruebe que algo está en un estado y cámbielo a otro.

  2. Establezca algo en un estado particular, sin tener en cuenta el estado anterior.

Algunos contextos requieren una acción, y algunos requieren la otra. Si un reproductor multimedia que alcanza el final del contenido permanecerá en un estado de "reproducción", pero con la posición congelada al final, entonces un método que afirma simultáneamente que el reproductor estaba en un estado de reproducción mientras lo establece en "detener" El estado podría ser útil si el código quisiera asegurar que nada haya causado que la reproducción falle antes de la solicitud de detención (lo que podría ser importante si, por ejemplo, algo estaba grabando el contenido que se está reproduciendo como un medio para convertir los medios de un formato a otro) . Sin embargo, en la mayoría de los casos, lo que importa es que, después de la operación, el reproductor multimedia esté en el estado esperado (es decir, detenido).

Si un reproductor multimedia se detiene automáticamente cuando llega al final de los medios, una función que afirme que el reproductor estaba funcionando probablemente sería más molesto que útil, pero en algunos casos saber si el reproductor estaba funcionando en el momento en que se detuvo sé útil. Quizás el mejor enfoque para satisfacer ambas necesidades sería hacer que la función devuelva un valor que indique el estado anterior del jugador. El código que se preocupa por ese estado podría examinar el valor de retorno; código que no le importa simplemente podría ignorarlo.

Super gato
fuente
2
+1 para la nota sobre cómo devolver el estado anterior: eso permite implementarlo de una manera que hace que toda la operación (consultar el estado y hacer la transición a un estado definido) sea atómica, que es al menos una buena característica. Sería más fácil escribir un código de usuario robusto que no falle esporádicamente debido a alguna condición de carrera extraña.
cmaster - reinstalar a monica
Un ejemplo de código bool TryStop()y A void Stop()haría que esta sea una respuesta realmente excelente.
RubberDuck
2
@RubberDuck: Ese no sería un buen patrón. El nombre TryStopimplicaría que no se debe lanzar una excepción si el jugador no puede ser forzado a un estado detenido. Tratando de detener el reproductor cuando ya está parado no es una condición excepcional, pero es una condición una persona que llama puede interesarle.
supercat
1
Entonces he entendido mal su respuesta @supercat
RubberDuck
2
Me gustaría señalar que probar y configurar de esta manera NO es una violación de la ley de demeter siempre que la API también proporcione una forma de probar el estado de reproducción sin cambiarlo. Este podría ser un método completamente diferente, por ejemplo isPlaying().
candied_orange
11

No hay una regla general. En este caso específico, la intención del usuario de su API es evitar que el reproductor reproduzca los medios. Si el reproductor no reproduce los medios, es MediaPlayer.stop()posible que no haga nada y el objetivo de la persona que llama al método aún se alcanzará: los medios no se reproducen.

Lanzar una excepción requeriría que el usuario de la API verifique si el jugador está jugando actualmente o atrape y maneje la excepción. Esto haría que la API sea más una tarea difícil de usar.

kmaczuga
fuente
11

El propósito de las excepciones no es indicar que algo malo ha sucedido; es para indicar que

  1. Algo malo ha sucedido
  2. que no sé arreglar aquí
  3. que la persona que llama, o algo que está en la pila de llamadas, debe saber cómo tratar
  4. así que estoy rescatando rápidamente, deteniendo la ejecución de la ruta del código actual para evitar daños o corrupción de datos, y dejo que la persona que llama limpie

Si la persona que llama intenta Stopun estado MediaPlayerque ya está en un estado detenido, este no es un problema que MediaPlayerno puede resolver (simplemente no puede hacer nada). No es un problema que, si continúa, provocará daños o datos corrupción, ya que puede tener éxito simplemente haciendo nada. Y no es realmente un problema que sea más razonable esperar que la persona que llama pueda resolver que el MediaPlayer.

Por lo tanto, no debe lanzar una excepción en esta situación.

Mason Wheeler
fuente
+1. Esta es la primera respuesta que muestra por qué una excepción no es apropiada aquí. Las excepciones indican que la persona que llama probablemente necesite saber sobre el caso excepcional. En este caso, es casi seguro que al cliente de la clase en cuestión no le importará si llamar a 'stop ()' realmente causó una transición de estado, por lo que probablemente no quiera saber acerca de una excepción.
Julio
5

Míralo de esta manera:

Si el cliente llama a Stop () cuando el jugador no está jugando, entonces Stop () tiene éxito automáticamente porque el jugador está actualmente en el estado detenido.

17 de 26
fuente
3

La regla es que haga lo que requiere el contrato de su método.

Puedo ver múltiples formas de definir contratos significativos para tal stopmétodo. Podría ser perfectamente válido para el stopmétodo no hacer absolutamente nada si el jugador no está jugando. En ese caso, usted define su API por sus objetivos. Desea realizar la transición al stoppedestado, y el stopmétodo lo hace, simplemente al pasar del stoppedestado a sí mismo sin error.

¿Hay alguna razón por la que quieras lanzar una Excepción? Si el código de usuario se verá así al final:

try {
    player.stop();
} catch(IllegalStateException e) {
    // do nothing, player was already stopped
}

entonces no hay beneficio en tener esa excepción. Entonces, la pregunta es, ¿el usuario se preocupará por el hecho de que el jugador ya haya sido detenido? ¿Tiene otro wa para consultarlo?

El jugador puede ser observable y ya puede notificar a los observadores sobre ciertas cosas, por ejemplo, notificar a los observadores cuando transita de playingun stoppinga otro stopped. En este caso, hacer que el método arroje una excepción no es necesario. Llamar stopno hará nada si el jugador ya está detenido, y llamarlo cuando no esté parado notificará a los observadores sobre la transición.

en general, depende de dónde terminará su lógica. Pero creo que hay mejores opciones de diseño que lanzar una excepción.

Debe lanzar una excepción si la persona que llama tiene que intervenir. En este caso, no tiene que hacerlo, simplemente puede dejar que el jugador esté como está y continúa funcionando.

Poligoma
fuente
3

Existe una estrategia general simple que lo ayuda a tomar esta decisión.

Considere cómo se manejaría la excepción si fuera lanzada.

Imagine su reproductor de música, los clics del usuario se detienen y luego se detienen nuevamente. ¿Desea mostrar un mensaje de error en ese escenario? Todavía no he visto un jugador que lo haga. ¿Desea que el comportamiento de la aplicación sea diferente a simplemente hacer clic en detener una vez (como registrar el evento o enviar un informe de error en segundo plano)? Probablemente no.

Eso significa que la excepción debería ser atrapada y tragada en alguna parte. Y eso significa que es mejor no lanzar la excepción en primer lugar porque no desea tomar una acción diferente .

Esto no solo se aplica a las excepciones, sino a cualquier tipo de ramificación: básicamente, si no necesita haber una diferencia observable en el comportamiento, no es necesario ramificar.

Dicho esto, no hay mucho daño en definir su stop()método para devolver un booleanvalor enum que indique si la detención fue "exitosa". Probablemente nunca lo use, pero un valor de retorno puede ignorarse de manera más fácil y natural que una excepción.

biziclop
fuente
2

Así es como lo hace Android: MediaPlayer

En pocas palabras, stopcuando startno se ha llamado no es un problema, el sistema permanece en estado detenido, pero si se llama a un jugador que ni siquiera sabe lo que está jugando, se lanza una excepción, porque no hay una buena razón para llamar detente ahí.

njzk2
fuente
1

¿Cuál debería ser la regla general cuando no se debe llamar a un método en alguna situación, pero su ejecución no daña el programa en general?

Veo lo que podría ser una pequeña contradicción en su declaración que puede aclararle la respuesta. ¿Por qué se supone que el método no debe llamarse y, sin embargo, su ejecución no hace daño ?

¿Por qué se supone que el método "no debe llamarse"? Eso parece una restricción autoimpuesta. Si no hay "daño" o impacto en su API, entonces no hay una excepción. Si realmente no se debe invocar el método porque puede crear estados no válidos o impredecibles, se debe lanzar una excepción.

Por ejemplo, si me acerqué a un reproductor de DVD y presioné "detener" antes de presionar "iniciar" y se estrelló, eso no tendría sentido para mí. Debería simplemente sentarse allí o, en el peor de los casos, reconocer "parar" en la pantalla. Un error sería molesto y de ninguna manera útil.

Sin embargo, ¿puedo poner un código de alarma para "apagarlo" cuando ya está apagado (llamar al método), lo que puede provocar que entre en un estado que le impida armarlo correctamente más tarde? Si es así, arroje un error aunque, técnicamente, "no hubo daño" porque puede haber un problema más adelante. Me gustaría saber sobre esto, incluso si en realidad no entró en ese estado. Solo la posibilidad es suficiente. Esto sería un IllegalStateException.

En su caso, si su máquina de estado puede manejar la llamada no válida / innecesaria, ignórela, de lo contrario, arroje un error.

EDITAR:

Tenga en cuenta que un compilador no invoca un error si establece una variable dos veces sin inspeccionar el valor. Tampoco le impide reinicializar una variable. Hay muchas acciones que podrían considerarse un "error" desde una perspectiva, pero son simplemente "ineficientes", "innecesarias", "inútiles", etc. Intenté proporcionar esa perspectiva en mi respuesta: hacer estas cosas generalmente no se considera un "error" porque no produce nada inesperado. Probablemente debería abordar su problema de manera similar.

Jim
fuente
No se debe llamar porque llamarlo indica un error en la persona que llama.
user253751
@immibis: no necesariamente. Por ejemplo, esa persona que llama podría ser un oyente al hacer clic en algún botón de la interfaz de usuario. (Como usuario, probablemente no le gustaría un mensaje de error si accidentalmente hace clic en el botón de detener si el medio ya está detenido)
Meriton
@meriton Si fuera el oyente al hacer clic en un botón de la interfaz de usuario, la respuesta sería obvia: "haga lo que quiera que suceda si se presiona el botón". Claramente no lo es, es algo interno de la aplicación.
user253751
@immibis: edité mi respuesta. Espero que ayude.
Jim
1

¿Qué hacen MediaPlayer.play()y MediaPlayer.stop()qué hacen exactamente ? ¿Son oyentes de eventos para la entrada del usuario o son realmente métodos que inician algún tipo de flujo de medios en el sistema? Si todos ellos son oyentes para la entrada del usuario, entonces es perfectamente razonable que no hagan nada (aunque sería una buena idea al menos registrarlos en algún lugar). Sin embargo, si afectan a la modelo de la interfaz de usuario es el control, entonces se podría lanzar una IllegalStateExceptionporque el jugador debe tener algún tipo de isPlaying=trueo isPlaying=falseestado (que no tiene por qué ser un indicador booleano real como escrito aquí), y por lo tanto cuando se llama MediaPlayer.stop()cuando isPlaying=false, el el método no puede realmente "detener" el MediaPlayerporque el objeto no está en el estado apropiado para detenerse - vea la descripción deljava.lang.IllegalStateException clase:

Señala que se ha invocado un método en un momento ilegal o inapropiado. En otras palabras, el entorno Java o la aplicación Java no está en un estado apropiado para la operación solicitada.

Por qué lanzar excepciones puede ser bueno

Mucha gente parece pensar que las excepciones son malas en general, ya que nunca deberían lanzarse (cf. Joel en Software ). Sin embargo, digamos que tiene un MediaPlayerGUI.notifyPlayButton()que llama MediaPlayer.play()e MediaPlayer.play()invoca un montón de otro código y en algún punto de la línea con la que interactúa, por ejemplo , PulseAudio , pero yo (el desarrollador) no sé dónde porque no escribí todo ese código.

Luego, un día, mientras trabajaba en el código, hago clic en "reproducir" y no sucede nada. Si MediaPlayerGUIController.notifyPlayButton()registra algo, al menos puedo mirar en los registros y comprobar que el clic del botón estaba registrado ... pero ¿por qué no se reproduce? Bueno, digamos que de hecho hay algo mal con los envoltorios PulseAudio que MediaPlayer.play()invoca. Vuelvo a hacer clic en el botón "reproducir" y esta vez obtengo un IllegalStateException:

 Exception in thread "main" java.lang.IllegalStateException: MediaPlayer already in play state.

     at org.superdupermediaplayer.MediaPlayer.play(MediaPlayer.java:16)
     at org.superdupermediaplayer.gui.MediaPlayerGUIController.notifyPlayButton(MediaPlayerGUIController.java:25)
     at org.superdupermediaplayer.gui.MediaPlayerGUI.main(MediaPlayerGUI.java:14)

Al observar ese seguimiento de la pila, puedo ignorar todo antes MediaPlayer.play()cuando depuro y pasar mi tiempo descubriendo por qué, por ejemplo, no se pasa ningún mensaje a PulseAudio para iniciar una transmisión de audio.

Por otro lado, si no lanzas una excepción, no tendré nada con lo que comenzar a trabajar aparte de: "El estúpido programa no reproduce música"; Aunque no es tan malo como ocultar un error explícito , como tragar realmente una excepción que , de hecho , se produjo , todavía está potencialmente perdiendo el tiempo de otros cuando algo sale mal ... así que sea amable y proponles una excepción.

errante
fuente
0

Prefiero diseñar la API de una manera que haga que sea más difícil o imposible para el consumidor cometer errores. Por ejemplo, en lugar de tener MediaPlayer.play()y MediaPlayer.stop(), podría proporcionar MediaPlayer.playToggle()qué alterna entre "detenido" y "jugando". De esta manera, siempre es seguro llamar al método: no hay riesgo de entrar en un estado ilegal.

Por supuesto, esto no siempre es posible o fácil de hacer. El ejemplo que dio es comparable a intentar eliminar un elemento que ya se eliminó de la lista. Puedes ser

  • pragmático al respecto y no arrojar ningún error. Esto ciertamente simplifica mucho código, por ejemplo, en lugar de tener que escribir if (list.contains(x)) { list.remove(x) }, solo necesita list.remove(x)). Pero, también puede ocultar errores.
  • o puede ser pedante y señalar un error de que la lista no contiene ese elemento. El código puede volverse más complicado a veces, ya que constantemente debe verificar si se cumplen las condiciones previas antes de llamar al método, pero la ventaja es que los errores eventuales son más fáciles de rastrear.

Si llamar MediaPlayer.stop()cuando ya está detenido no tiene ningún daño en su aplicación, entonces dejaría que se ejecute en silencio porque simplifica el código y me encantan los métodos idempotentes. Pero si está absolutamente seguro de que MediaPlayer.stop()nunca se llamaría en esas condiciones, arrojaría un error porque podría haber un error en otra parte del código y la excepción ayudaría a rastrearlo.

Pedro Rodrigues
fuente
33
No estoy de acuerdo con que playToggle()sea ​​más fácil de usar: para lograr el efecto deseado (jugador jugando o no jugando), se le requerirá saber su estado actual. Por lo tanto, si recibe una entrada del usuario que le solicita que detenga el reproductor, primero deberá preguntar si el jugador está jugando actualmente, luego alternar su estado o no, dependiendo de la respuesta. Eso es mucho más engorroso que simplemente llamar a un stop()método y terminarlo.
cmaster - reinstalar a monica
Bueno, nunca dije que fuera más fácil de usar, mi afirmación es que es más seguro. Además, su ejemplo asume que al usuario se le darían botones separados para detener y reproducir. Si ese fuera el caso, entonces sí, a playTogglesería muy engorroso de usar. Pero si el usuario también recibió un botón de alternar, entonces playTogglesería más fácil de usar que reproducir / detener.
Pedro Rodrigues