Was ist besser, wenn IllegalStateException oder eine stille Methode ausgeführt wird? [geschlossen]

16

Angenommen, ich habe eine MediaPlayer-Klasse mit den Methoden play () und stop (). Was ist die beste Strategie für die Implementierung der Stop-Methode, wenn die Play-Methode noch nicht aufgerufen wurde? Ich sehe zwei Möglichkeiten: eine Ausnahme auslösen, weil sich der Player nicht in einem geeigneten Zustand befindet, oder Aufrufe der Stopp-Methode stillschweigend ignorieren.

Was sollte die allgemeine Regel sein, wenn eine Methode in bestimmten Situationen nicht aufgerufen werden soll, ihre Ausführung jedoch dem Programm im Allgemeinen keinen Schaden zufügt?

x2bool
quelle
21
Verwenden Sie keine Ausnahmen, um ein Verhalten zu modellieren, das nicht außergewöhnlich ist.
Alexander - Reinstate Monica
1
Hallo! Ich denke nicht, dass es ein Duplikat ist. Meine Frage bezieht sich mehr auf IllegalStateException und die obige Frage bezieht sich auf die Verwendung von Ausnahmen im Allgemeinen.
x2bool
1
Warum nicht auch die Anomalie als Warnung protokollieren, anstatt im Stillen zu scheitern? Sicherlich unterstützt das von Ihnen verwendete Java verschiedene Protokollebenen (ERROR, WARN, INFO, DEBUG).
Eric Seastrand
3
Beachten Sie, dass Set.add keine Ausnahme auslöst, wenn sich das Element bereits im Set befindet. Der Zweck besteht darin, "sicherzustellen, dass sich das Element in der Menge befindet" und nicht "das Element zur Menge hinzuzufügen". Daher wird der Zweck erreicht, indem nichts unternommen wird, wenn sich das Element bereits in der Menge befindet.
user253751
2
Wort des Tages: Idempotenz
Jules

Antworten:

37

Es gibt keine Regel. Es liegt ganz bei Ihnen, wie Sie Ihre API "fühlen" lassen möchten.

Persönlich in einem Musik - Player, ich glaube , ein Übergang von dem Zustand Stoppedzu Stoppedmittels des Verfahrens Stop()ist ein absolut gültiger Zustandsübergang. Es ist nicht sehr aussagekräftig, aber es ist gültig. In diesem Sinne erscheint das Auslösen einer Ausnahme pedantisch und unfair. Dadurch würde sich die API wie das soziale Äquivalent eines Gesprächs mit dem nervigen Kind im Schulbus anfühlen. Sie stecken in der nervigen Situation fest, aber Sie können damit umgehen.

Ein "geselliger" Ansatz ist es, zu erkennen, dass der Übergang im schlimmsten Fall harmlos ist, und freundlich zu Ihren konsumierenden Entwicklern zu sein, indem Sie ihn zulassen.

Wenn es nach mir ginge, würde ich die Ausnahme auslassen.

MetaFight
quelle
15
Diese Antwort erinnert uns daran, dass es manchmal eine gute Idee ist, die Zustandsübergänge für einfache Interaktionen zu skizzieren.
6
In diesem Sinne erscheint das Auslösen einer Ausnahme pedantisch und unfair. Dadurch würde sich die API wie das soziale Äquivalent eines Gesprächs mit dem nervigen Kind im Schulbus anfühlen. > Ausnahmen sollen Sie nicht bestrafen: Sie sollen Ihnen mitteilen, dass etwas Unerwartetes passiert ist. Ich persönlich wäre sehr dankbar für eine MediaPlayer.stop()Methode, die IllegalStateExceptionstundenlanges Debuggen von Code ersetzte und sich fragte, warum zum Teufel "nichts passiert" (dh "es funktioniert einfach nicht").
Errantlinguist
3
Wenn Ihr Design sagt, dass der Loopback-Übergang ungültig ist, sollten Sie natürlich eine Ausnahme auslösen. Meine Antwort betraf jedoch ein Design, bei dem dieser Übergang vollkommen in Ordnung ist.
MetaFight
1
StopOrThrow()klingt schrecklich Wenn Sie diese Gasse entlang gehen, warum nicht das Standardmuster von verwenden TryStop()? Wenn das Verhalten einer API unklar ist (wie die meisten von ihnen), wird von Entwicklern nicht erwartet, dass sie einfach raten oder experimentieren, sondern dass sie sich die Dokumentation ansehen. Deshalb mag ich MSDN so sehr.
MetaFight
1
Persönlich bevorzuge ich die Fail-Fast-Option, also würde ich IllegalStateException wählen, um dem API-Benutzer anzuzeigen, dass etwas in seiner Logik nicht stimmt, auch wenn es harmlos ist. Ich bekomme diese Ausnahmen oft aus meinem eigenen Code, was sehr hilfreich ist, um zu erkennen, dass meine unerledigte Arbeit bereits falsch ist
Walfrat,
17

Es gibt zwei verschiedene Arten von Aktionen, die Sie ausführen möchten:

  1. Testen Sie gleichzeitig, ob sich etwas in einem Zustand befindet, und ändern Sie ihn in einen anderen.

  2. Legen Sie einen bestimmten Status fest, ohne den vorherigen Status zu berücksichtigen.

Einige Kontexte erfordern eine Aktion und einige erfordern die andere. Wenn ein Mediaplayer, der das Ende des Inhalts erreicht, in einem "Wiedergabe" -Zustand verbleibt, aber die Position am Ende eingefroren ist, dann eine Methode, die gleichzeitig bestätigt, dass sich der Player in einem Wiedergabezustand befand, während er auf "Stopp" gesetzt wurde. state kann nützlich sein, wenn Code sicherstellen möchte, dass die Wiedergabe vor der Stoppanforderung nicht fehlgeschlagen ist (was wichtig sein kann, wenn z. B. der wiedergegebene Inhalt aufgezeichnet wird, um das Medium von einem Format in ein anderes zu konvertieren). . In den meisten Fällen kommt es jedoch darauf an, dass sich der Media Player nach dem Vorgang im erwarteten Zustand befindet (dh angehalten wurde).

Wenn sich ein Mediaplayer am Ende des Mediums automatisch selbst stoppt, ist eine Funktion, die angibt, dass der Player ausgeführt wird, wahrscheinlich eher ärgerlich als hilfreich. In einigen Fällen kann es jedoch hilfreich sein, zu wissen, ob der Player zum Zeitpunkt des Stopps ausgeführt wurde nützlich sein. Möglicherweise besteht die beste Lösung für beide Anforderungen darin, dass die Funktion einen Wert zurückgibt, der den vorherigen Status des Spielers angibt. Code, der sich um diesen Status kümmert, kann den Rückgabewert untersuchen. Code, der sich nicht darum kümmert, könnte ihn einfach ignorieren.

Superkatze
quelle
2
+1 für den Hinweis zur Rückgabe des alten Zustands: Dies ermöglicht eine Implementierung in einer Weise, die die gesamte Operation (Abfrage des Zustands und Übergang zu einem definierten Zustand) atomar macht, was zumindest eine nette Funktion ist. Dies würde es einfacher machen, robusten Benutzercode zu schreiben, der nicht sporadisch aufgrund einer komischen Rennbedingung versagt.
cmaster
Ein bool TryStop()und ein void Stop()Codebeispiel würden dies zu einer wirklich ausgezeichneten Antwort machen.
RubberDuck
2
@RubberDuck: Das wäre kein gutes Muster. Der Name TryStopwürde bedeuten, dass eine Ausnahme nicht geworfen werden sollte, wenn der Spieler nicht in einen gestoppten Zustand gezwungen werden kann. Der Versuch, den Spieler zu stoppen, wenn er bereits gestoppt ist, ist keine Ausnahmebedingung, aber eine Bedingung, an der ein Anrufer interessiert sein könnte.
Supercat
1
Dann habe ich deine Antwort falsch verstanden @supercat
RubberDuck
2
Ich möchte darauf hinweisen, dass das Testen und Einstellen dieser Methode NICHT gegen das Demeter-Gesetz verstößt, vorausgesetzt, die API bietet auch die Möglichkeit, den Wiedergabestatus zu testen, ohne ihn zu ändern. Dies könnte beispielsweise eine völlig andere Methode sein isPlaying().
candied_orange
11

Es gibt keine allgemeine Regel. In diesem speziellen Fall möchte der Benutzer Ihrer API den Player daran hindern, die Medien abzuspielen. Wenn der Player die Medien nicht abspielt, unternimmt der MediaPlayer.stop()möglicherweise nichts und das Ziel des Methodenaufrufers wird weiterhin erreicht - die Medien werden nicht abgespielt.

Beim Auslösen einer Ausnahme muss der Benutzer der API entweder prüfen, ob der Player gerade spielt, oder die Ausnahme abfangen und behandeln. Dies würde die API zu einer mühsameren Aufgabe machen.

kmaczuga
quelle
11

Mit Ausnahmen soll nicht signalisiert werden, dass etwas Schlimmes passiert ist. es soll das signalisieren

  1. Es ist etwas Schlimmes passiert
  2. das kann ich hier nicht beheben
  3. dass der Anrufer oder etwas, das sich im Callstack befindet, wissen sollte, wie man damit umgeht
  4. Ich rette mich also schnell, stoppe die Ausführung des aktuellen Codepfads, um Beschädigungen oder Datenbeschädigungen zu vermeiden, und überlasse es dem Aufrufer, die Daten zu bereinigen

Wenn der Anrufer versucht, Stopeine Verbindung herzustellen MediaPlayer, die sich bereits in einem angehaltenen Zustand befindet, ist dies kein Problem, das MediaPlayernicht gelöst werden kann (es kann einfach nichts tun). Wenn dies fortgesetzt wird, führt dies nicht zu Schäden oder Daten Korruption, da sie einfach durch Nichtstun erfolgreich sein kann. Und es ist nicht wirklich ein Problem, dass es vernünftiger ist, zu erwarten, dass der Anrufer in der Lage ist, eine Lösung zu finden als dieMediaPlayer .

Aus diesem Grund sollten Sie in dieser Situation keine Ausnahme auslösen.

Mason Wheeler
quelle
+1. Dies ist die erste Antwort, die zeigt, warum eine Ausnahme hier nicht angemessen ist. Ausnahmen weisen darauf hin, dass der Anrufer wahrscheinlich über den Ausnahmefall Bescheid wissen muss. In diesem Fall ist es dem Client der betreffenden Klasse mit ziemlicher Sicherheit egal, ob der Aufruf von 'stop ()' tatsächlich einen Zustandsübergang verursacht hat, und er möchte wahrscheinlich keine Ausnahme kennen.
Jules
5

Schau es dir so an:

Wenn der Client Stop () aufruft, während der Player nicht spielt, ist Stop () automatisch erfolgreich, da sich der Player derzeit im gestoppten Zustand befindet.

17 von 26
quelle
3

In der Regel tun Sie, was der Vertrag Ihrer Methode erfordert.

Ich sehe mehrere Möglichkeiten, sinnvolle Verträge für eine solche stopMethode zu definieren . Es kann durchaus sinnvoll sein, dass die stopMethode absolut nichts tut, wenn der Spieler nicht spielt. In diesem Fall definieren Sie Ihre API anhand ihrer Ziele. Sie möchten in den stoppedZustand übergehen , und die stopMethode tut dies, indem Sie einfach stoppedfehlerfrei aus dem Zustand in sich selbst übergehen .

Gibt es einen Grund, warum Sie eine Ausnahme auslösen möchten ? Wenn der Benutzercode am Ende so aussehen wird:

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

dann hat diese Ausnahme keinen Vorteil. Die Frage ist also, ob es dem Benutzer wichtig ist, dass der Player bereits gestoppt wurde. Hat er noch eine Frage?

Der Spieler kann beobachtbar sein und Beobachter bereits über bestimmte Dinge benachrichtigen - z. B. Beobachter benachrichtigen, wenn er von playingnach stoppingnach wechselt stopped. In diesem Fall ist es nicht erforderlich, dass die Methode eine Ausnahme auslöst. Der Aufruf stopwird einfach nichts tun , wenn der Spieler bereits gestoppt ist, und nannte es , wenn es nicht benachrichtigt die Beobachter über den Übergang gestoppt.

Alles in allem hängt es davon ab, wo Ihre Logik landen wird. Aber ich denke, es gibt bessere Entwurfswahlen als das Werfen einer Ausnahme.

Sie sollten eine Ausnahme auslösen, wenn der Anrufer eingreifen muss. In diesem Fall muss er nicht, Sie können den Player einfach so lassen, wie er ist, und es funktioniert weiter.

Polygnom
quelle
3

Es gibt eine einfache allgemeine Strategie, mit der Sie diese Entscheidung treffen können.

Überlegen Sie, wie die Ausnahme behandelt werden soll, wenn sie ausgelöst wird.

Stellen Sie sich also Ihren Musik-Player vor, der Benutzer klickt auf Stopp und dann erneut auf Stopp. Möchten Sie in diesem Szenario eine Fehlermeldung anzeigen? Ich habe noch keinen Spieler gesehen, der das tut. Möchten Sie, dass sich das Anwendungsverhalten von einem einfachen Klicken auf Stopp unterscheidet (z. B. das Protokollieren des Ereignisses oder das Senden eines Fehlerberichts im Hintergrund)? Wahrscheinlich nicht.

Das heißt, die Ausnahme müsste irgendwo gefangen und geschluckt werden. Und das bedeutet, dass Sie besser dran sind, die Ausnahme gar nicht erst auszulösen, weil Sie keine andere Aktion ausführen möchten .

Dies gilt nicht nur für Ausnahmen, sondern für jede Art von Verzweigung: Wenn es keine feststellbaren Verhaltensunterschiede geben soll, ist keine Verzweigung erforderlich.

Das heißt, es schadet nicht, Ihre stop()Methode so zu definieren , dass sie einen booleanoder einen Aufzählungswert zurückgibt, der angibt, ob das Stoppen "erfolgreich" war. Sie werden es wahrscheinlich nie verwenden, aber ein Rückgabewert kann einfacher und natürlicher ignoriert werden als eine Ausnahme.

biziclop
quelle
2

So macht es Android: MediaPlayer

Kurz gesagt, stopwenn startnicht angerufen wurde, ist das kein Problem, das System bleibt im gestoppten Zustand. Wenn jedoch ein Spieler angerufen wird, der nicht einmal weiß, was er spielt, wird eine Ausnahme ausgelöst, da es keinen guten Grund für einen Anruf gibt halte dort an.

njzk2
quelle
1

Was sollte die allgemeine Regel sein, wenn eine Methode in bestimmten Situationen nicht aufgerufen werden soll, ihre Ausführung jedoch dem Programm im Allgemeinen keinen Schaden zufügt?

Ich sehe, was ein kleiner Widerspruch in Ihrer Aussage sein könnte, der Ihnen die Antwort klar machen könnte. Warum soll die Methode nicht aufgerufen werden und dennoch schadet ihre Ausführung nichts ?

Warum soll die Methode "nicht aufgerufen werden"? Das scheint eine selbst auferlegte Einschränkung zu sein. Wenn Ihre API keinen "Schaden" oder Einfluss hat, gibt es keine Ausnahme. Wenn die Methode wirklich nicht aufgerufen werden sollte, weil sie möglicherweise ungültige oder unvorhersehbare Zustände erzeugt, sollte eine Ausnahme ausgelöst werden.

Wenn ich zum Beispiel zu einem DVD-Player gelaufen bin und auf "Stopp" geklickt habe, bevor ich auf "Start" geklickt habe und er abgestürzt ist, würde das für mich keinen Sinn ergeben. Es sollte nur dort sitzen oder im schlimmsten Fall "Stop" auf dem Bildschirm bestätigen. Ein Fehler wäre ärgerlich und in keiner Weise hilfreich.

Kann ich jedoch einen Alarmcode eingeben, um das Gerät auszuschalten, wenn es bereits ausgeschaltet ist (Aufrufen der Methode)? Wenn ja, dann werfen Sie einen Fehler, obwohl technisch "es keinen Schaden angerichtet hat", weil es später Probleme geben kann. Ich würde es gerne wissen, auch wenn es nicht in diesen Zustand ging. Nur die Möglichkeit ist genug. Das wäre ein IllegalStateException.

Wenn in Ihrem Fall Ihr Zustandsautomat den ungültigen / unnötigen Aufruf verarbeiten kann, ignorieren Sie ihn, andernfalls wird ein Fehler ausgegeben.

BEARBEITEN:

Beachten Sie, dass ein Compiler keinen Fehler auslöst, wenn Sie eine Variable zweimal festlegen, ohne den Wert zu überprüfen. Es hindert Sie auch nicht daran, eine Variable neu zu initialisieren. Es gibt viele Aktionen, die aus einer Perspektive als "Fehler" betrachtet werden könnten, aber nur "ineffizient", "unnötig", "sinnlos" usw. Ich habe versucht, diese Perspektive in meiner Antwort anzugeben - dies wird im Allgemeinen nicht berücksichtigt Ein "Bug", weil er nichts Unerwartetes zur Folge hat. Sie sollten Ihr Problem wahrscheinlich ähnlich angehen.

Jim
quelle
Es sollte nicht aufgerufen werden, da ein Aufruf einen Fehler im Aufrufer anzeigt.
user253751
@immibis: Nicht unbedingt. Dieser Anrufer kann beispielsweise ein Listener einer UI-Schaltfläche sein, der auf einen Klick reagiert. (Als Benutzer wird Ihnen wahrscheinlich keine Fehlermeldung angezeigt, wenn Sie versehentlich auf die Stopp-Schaltfläche klicken, wenn das Medium bereits gestoppt ist.)
meriton - Streik
@meriton Wenn es sich um den Listener einer UI-Schaltfläche handelt, liegt die Antwort auf der Hand: "Tun Sie, was immer Sie wollen, wenn die Schaltfläche gedrückt wird". Offensichtlich ist es nicht, es ist etwas Internes in der Anwendung.
user253751
@immibis - Ich habe meine Antwort bearbeitet. Ich hoffe, es hilft.
Jim
1

Was genau tun MediaPlayer.play()und MediaPlayer.stop()tun? - Sind sie Ereignis- Listener für Benutzereingaben oder sind sie tatsächlich Methoden, die eine Art Medienstrom auf dem System starten? Wenn alles, was sie sind, Listener für Benutzereingaben sind, ist es für sie völlig vernünftig, nichts zu tun (obwohl es eine gute Idee wäre, sie zumindest irgendwo zu protokollieren). Wenn sie jedoch die Auswirkungen auf Modell UI steuert, dann könnten sie einen werfen , IllegalStateExceptionweil der Spieler eine Art haben sollte isPlaying=trueoder isPlaying=falseZustand (es muss nicht eine tatsächliche Boolesche Flag wie hier geschrieben sein), und so , wenn Sie anrufen , MediaPlayer.stop()wenn isPlaying=falsedie Diese Methode kann MediaPlayerdas Objekt nicht "stoppen", da es sich nicht in dem zu stoppenden Zustand befindet - siehe Beschreibung desjava.lang.IllegalStateException Klasse:

Signalisiert, dass eine Methode zu einem unzulässigen oder unangemessenen Zeitpunkt aufgerufen wurde. Mit anderen Worten, die Java-Umgebung oder Java-Anwendung befindet sich nicht in einem für die angeforderte Operation geeigneten Zustand.

Warum es gut sein kann, Ausnahmen zu werfen

Viele Leute scheinen zu glauben, dass Ausnahmen generell schlecht sind, da sie niemals geworfen werden sollten (vgl. Joel on Software ). Nehmen wir jedoch an, Sie haben einen MediaPlayerGUI.notifyPlayButton()which-Aufruf MediaPlayer.play()und MediaPlayer.play()rufen eine Reihe anderen Codes auf, der irgendwo in der Folge mit z. B. PulseAudio zusammenarbeitet , aber ich (der Entwickler) weiß nicht, wo, weil ich nicht den gesamten Code geschrieben habe.

Dann, eines Tages, während ich am Code arbeite, klicke ich auf "Abspielen" und nichts passiert. Wenn MediaPlayerGUIController.notifyPlayButton()etwas protokolliert wird, kann ich zumindest in den Protokollen nachsehen, ob der Tastenklick tatsächlich registriert wurde ... aber warum wird er nicht abgespielt? Angenommen, mit den PulseAudio-Wrappern stimmt tatsächlich etwas nicht MediaPlayer.play(). Ich klicke erneut auf den "Play" -Button und diesmal erhalte ich eine 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)

Wenn ich mir diesen Stack-Trace ansehe, kann ich MediaPlayer.play()beim Debuggen alles ignorieren und herausfinden, warum z. B. keine Nachricht an PulseAudio gesendet wird, um einen Audiostream zu starten.

Auf der anderen Seite, wenn Sie keine Ausnahme machen, habe ich nichts anderes zu tun als: "Das blöde Programm spielt keine Musik ab"; Obwohl es nicht so schlimm , wie explizite Fehler versteckt , wie eigentlich eine Ausnahme beim Schlucken , das war in der Tat geworfen , ich ist immer noch potentiell andere Zeit zu verschwenden , wenn etwas schief gehen tut ... so schön, und eine Ausnahme auf sie werfen.

irrantlinguist
quelle
0

Ich bevorzuge es, die API so zu gestalten, dass es für den Verbraucher schwieriger oder unmöglich wird, Fehler zu machen. Beispielsweise könnten Sie anstelle von MediaPlayer.play()und angeben MediaPlayer.stop(), MediaPlayer.playToggle()welche Option zwischen "Angehalten" und "Spielen" wechselt. Auf diese Weise ist die Methode immer aufrufsicher - es besteht kein Risiko, in einen illegalen Zustand zu gelangen.

Dies ist natürlich nicht immer möglich oder einfach zu bewerkstelligen. Das Beispiel, das Sie angegeben haben, ist vergleichbar mit dem Versuch, ein Element zu entfernen, das bereits aus der Liste entfernt wurde. Sie können entweder sein

  • pragmatisch darüber und keinen Fehler zu werfen. Dies vereinfacht sicherlich eine Menge Code, z. B. anstatt dass if (list.contains(x)) { list.remove(x) }Sie nur schreiben müssen list.remove(x)). Es kann aber auch Fehler verbergen.
  • oder Sie können pedantisch sein und einen Fehler signalisieren, dass die Liste dieses Element nicht enthält. Der Code kann zuweilen komplizierter werden, da Sie ständig überprüfen müssen, ob die Voraussetzungen erfüllt sind, bevor Sie die Methode aufrufen. Der Vorteil ist jedoch, dass eventuelle Fehler leichter aufgespürt werden können.

Wenn das Aufrufen, MediaPlayer.stop()wenn es bereits gestoppt ist, in Ihrer Anwendung keinen Schaden anrichtet, dann würde ich es unbemerkt laufen lassen, weil es den Code vereinfacht und ich ein Faible für idempotente Methoden habe. Aber wenn Sie absolut sicher sind, dass MediaPlayer.stop()dies unter diesen Bedingungen niemals aufgerufen werden würde , würde ich einen Fehler auslösen, da der Code möglicherweise irgendwo anders fehlerhaft ist und die Ausnahme dabei helfen würde, ihn aufzuspüren.

Pedro Rodrigues
quelle
3
Ich bin nicht einverstanden, playToggle()dass die Bedienung einfacher ist: Um den gewünschten Effekt zu erzielen (Spieler spielen oder nicht spielen), müssten Sie den aktuellen Status kennen. Wenn Sie also eine Benutzereingabe erhalten, in der Sie aufgefordert werden, den Player zu stoppen, müssen Sie zunächst nachfragen, ob der Player gerade spielt, und dann den Status abhängig von der Antwort umschalten. Das ist viel umständlicher, als nur eine stop()Methode aufzurufen und damit fertig zu sein.
cmaster
Nun, ich habe nie gesagt, dass es einfacher zu bedienen ist - meine Behauptung war, dass es sicherer ist. In Ihrem Beispiel wird außerdem davon ausgegangen, dass dem Benutzer separate Stopp- und Wiedergabetasten zugewiesen werden. Wenn das in der Tat der Fall wäre dann ja, playTogglewäre eine Verwendung sehr umständlich. Aber wenn der Benutzer stattdessen auch eine Umschalttaste erhalten würde, playTogglewäre dies einfacher zu bedienen als das Abspielen / Stoppen.
Pedro Rodrigues