Quoi de mieux IllegalStateException ou une exécution de méthode silencieuse? [fermé]

16

Disons que j'ai une classe MediaPlayer qui a des méthodes play () et stop (). Quelle est la meilleure stratégie à utiliser lors de la mise en œuvre de la méthode d'arrêt au cas où la méthode de lecture n'a pas été appelée auparavant. Je vois deux options: lever une exception parce que le joueur n'est pas dans un état approprié, ou ignorer silencieusement les appels à la méthode d'arrêt.

Quelle devrait être la règle générale lorsqu'une méthode n'est pas censée être appelée dans certaines situations, mais que son exécution ne nuit pas au programme en général?

x2bool
la source
21
N'utilisez pas d'exceptions pour modéliser un comportement qui n'est pas exceptionnel.
Alexander - Rétablir Monica
1
Hey! Je ne pense pas que ce soit un doublon. Ma question concerne davantage IllegalStateException et la question ci-dessus concerne l'utilisation des exceptions en général.
x2bool
1
Au lieu d'échouer silencieusement, pourquoi ne pas également enregistrer l'anomalie en tant qu'avertissement? Certes, le Java que vous utilisez prend en charge différents niveaux de journalisation (ERREUR, AVERTISSEMENT, INFO, DÉBOGAGE).
Eric Seastrand
3
Notez que Set.add ne lève pas d'exception si l'élément est déjà dans l'ensemble. Son but est de "s'assurer que l'élément est dans l'ensemble", plutôt que "d'ajouter l'élément à l'ensemble", afin qu'il atteigne son objectif en ne faisant rien si l'élément est déjà dans l'ensemble.
user253751
2
Le mot du jour: idempotence
Jules

Réponses:

37

Il n'y a pas de règle. Cela dépend entièrement de la façon dont vous voulez faire "sentir" votre API.

Personnellement, dans un lecteur de musique, je pense qu'une transition de l'état Stoppedà Stoppedla méthode Stop()est une transition d'état parfaitement valide. Ce n'est pas très significatif, mais c'est valable. Dans cette optique, lever une exception semblerait pédant et injuste. Cela donnerait à l'API l'impression d'être l'équivalent social de parler à l'enfant ennuyeux dans le bus scolaire. Vous êtes coincé avec la situation ennuyeuse, mais vous pouvez y faire face.

Une approche plus «sociable» consiste à reconnaître que la transition est au pire inoffensive et à être gentil avec vos développeurs consommateurs en l'autorisant.

Donc, si cela ne tenait qu'à moi, je sauterais l'exception.

MetaFight
la source
15
Cette réponse nous rappelle que parfois c'est une bonne idée d'esquisser les transitions d'état pour des interactions même simples.
6
Dans cette optique, lever une exception semblerait pédant et injuste. Cela donnerait à l'API l'impression d'être l'équivalent social de parler à l'enfant ennuyeux dans le bus scolaire. > Les exceptions ne sont pas conçues pour vous punir: elles sont conçues pour vous dire que quelque chose d'inattendu s'est produit. Personnellement, je serais très reconnaissant pour une MediaPlayer.stop()méthode qui a jeté un IllegalStateExceptionau lieu de passer des heures à déboguer du code et à se demander pourquoi l'enfer "rien ne se passe" (c'est-à-dire "cela ne fonctionne tout simplement pas").
errantlinguist
3
Bien sûr, si votre conception indique que la transition de bouclage n'est pas valide, alors OUI, vous devez lever une exception. Cependant, ma réponse concernait un design où cette transition est parfaitement OK.
MetaFight
1
StopOrThrow()semble horrible. Si vous descendez cette allée, pourquoi ne pas utiliser le modèle standard de TryStop()? De plus, généralement, lorsque le comportement d'une API n'est pas clair (comme la plupart d'entre eux), les développeurs ne sont pas censés simplement deviner ou expérimenter, ils doivent consulter la documentation. C'est pourquoi j'aime tellement MSDN.
MetaFight
1
Personnellement, je préfère l'option fail-fast, donc je choisirais IllegalStateException afin d'indiquer à l'utilisateur de l'API qu'il y a quelque chose de mal dans sa logique, même si c'est inoffensif. Je reçois souvent ces exceptions de mon propre code, vraiment utile pour détecter que mon travail inachevé est déjà faux
Walfrat
17

Il existe deux types distincts d'actions que l'on peut souhaiter effectuer:

  1. Testez simultanément que quelque chose est dans un état et changez-le en un autre.

  2. Définissez quelque chose dans un état particulier, sans tenir compte de l'état précédent.

Certains contextes nécessitent une action, et certains nécessitent l'autre. Si un lecteur multimédia qui atteint la fin du contenu restera dans un état de "lecture", mais avec la position figée à la fin, alors une méthode qui affirme simultanément que le lecteur était dans un état de lecture tout en le mettant à "stop" l'état peut être utile si le code souhaite s'assurer que rien n'a causé l'échec de la lecture avant la demande d'arrêt (ce qui peut être important si, par exemple, quelque chose enregistrait le contenu en cours de lecture comme moyen de convertir le média d'un format à un autre) . Dans la plupart des cas, cependant, ce qui importe, c'est qu'après l'opération, le lecteur multimédia se trouve dans l'état attendu (c'est-à-dire arrêté).

Si un lecteur multimédia s'arrête automatiquement lorsqu'il atteint la fin du média, une fonction qui affirme que le lecteur était en cours d'exécution serait probablement plus ennuyeuse qu'utile, mais dans certains cas, savoir si le lecteur était en cours d'exécution au moment où il a été arrêté pourrait sois utile. La meilleure approche pour répondre aux deux besoins serait peut-être que la fonction renvoie une valeur indiquant l'état précédent du joueur. Un code qui se soucie de cet état pourrait examiner la valeur de retour; un code qui ne se soucie pas pourrait simplement l'ignorer.

supercat
la source
2
+1 pour la note sur le retour de l'ancien état: cela permet de l'implémenter d'une manière qui rend toute l'opération (interrogation de l'état et transition vers un état défini) atomique, ce qui est à tout le moins une fonctionnalité intéressante. Cela faciliterait l'écriture d'un code utilisateur robuste qui n'échoue pas sporadiquement en raison d'une condition de concurrence étrange.
cmaster - réintègre monica
Un bool TryStop()et un void Stop()exemple de code en feraient une excellente réponse.
RubberDuck
2
@RubberDuck: Ce ne serait pas un bon modèle. Le nom TryStopimpliquerait qu'une exception ne devrait pas être levée si le joueur ne peut pas être forcé à un état arrêté. Essayer d'arrêter le joueur alors qu'il est déjà arrêté n'est pas une condition exceptionnelle, mais c'est une condition qui pourrait intéresser un appelant.
supercat
1
Alors j'ai mal compris votre réponse @supercat
RubberDuck
2
Je voudrais souligner que tester et configurer de cette manière n'est PAS une violation de la loi de déméter à condition que l'API fournisse également un moyen de tester l'état de lecture sans le changer. Cela pourrait être une méthode complètement différente, par exemple isPlaying().
candied_orange
11

Il n'y a pas de règle générale. Dans ce cas spécifique, l'intention de l'utilisateur de votre API est d'empêcher le lecteur de lire le média. Si le lecteur ne lit pas le média, le MediaPlayer.stop()peut ne rien faire et l'objectif de l'appelant de méthode sera toujours atteint - le média ne joue pas.

Le lancement d'une exception nécessiterait que l'utilisateur de l'API vérifie si le joueur joue actuellement ou intercepte et gère l'exception. Cela rendrait l'API plus une corvée à utiliser.

kmaczuga
la source
11

Le but des exceptions n'est pas de signaler que quelque chose de mauvais s'est produit; c'est pour signaler que

  1. Quelque chose de mauvais s'est produit
  2. que je ne sais pas comment réparer ici
  3. que l'appelant, ou quelque chose dans la pile d'appels, sache comment gérer
  4. donc je renonce rapidement, interrompant l'exécution du chemin de code actuel afin d'éviter les dommages ou la corruption des données, et laissant le soin à l'appelant de nettoyer

Si l'appelant essaie Stopun MediaPlayerqui est déjà dans un état arrêté, ce n'est pas un problème que le MediaPlayerne peut pas résoudre (il ne peut tout simplement rien faire.) Ce n'est pas un problème qui, s'il continue, entraînera des dommages ou des données la corruption, car elle peut réussir simplement en ne faisant rien. Et ce n'est pas vraiment un problème qu'il est plus raisonnable de s'attendre à ce que l'appelant soit en mesure de résoudre que leMediaPlayer .

Par conséquent, vous ne devez pas lever d'exception dans cette situation.

Mason Wheeler
la source
+1. C'est la première réponse qui montre pourquoi une exception n'est pas appropriée ici. Les exceptions indiquent que l'appelant est susceptible d'avoir besoin de connaître le cas exceptionnel. Dans ce cas, presque certainement, le client de la classe en question ne se souciera pas si l'appel à «stop ()» a réellement provoqué une transition d'état, il ne voudra donc probablement pas connaître une exception.
Jules
5

Vois-le de cette façon:

Si le client appelle Stop () lorsque le lecteur ne joue pas, Stop () réussit automatiquement car le lecteur est actuellement à l'état arrêté.

17 sur 26
la source
3

La règle est que vous faites ce que requiert le contrat de votre méthode.

Je peux voir plusieurs façons de définir des contrats significatifs pour une telle stopméthode. Il pourrait être parfaitement valable que la stopméthode ne fasse absolument rien si le joueur ne joue pas. Dans ce cas, vous définissez votre API par ses objectifs. Vous voulez passer à l' stoppedétat, et la stopméthode le fait - simplement en passant de l' stoppedétat à lui-même sans erreur.

Y at - il des raisons pour lesquelles vous voulez jeter une exception? Si le code utilisateur ressemblera à la fin:

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

alors il n'y a aucun avantage à avoir cette exception. La question est donc: l'utilisateur se souciera-t-il du fait que le lecteur a déjà été arrêté? A-t-il un autre wa pour l'interroger?

Le joueur peut être observable et peut déjà avertir les observateurs de certaines choses - par exemple, avertir les observateurs lorsqu'il passe playingde stoppingà stopped. Dans ce cas, il n'est pas nécessaire que la méthode lève une exception. Appeler stopne fera rien si le joueur est déjà arrêté, et l'appeler lorsqu'il n'est pas arrêté informera les observateurs de la transition.

dans l'ensemble, cela dépend de l'endroit où finira votre logique. Mais je pense qu'il y a de meilleurs choix de conception que de lever une exception.

Vous devez lever une exception si l' appelant doit intervenir. Dans ce cas, il n'a pas à le faire, vous pouvez simplement laisser le joueur tel qu'il est et cela continue de fonctionner.

Polygnome
la source
3

Il existe une stratégie générale simple qui vous aide à prendre cette décision.

Considérez comment l'exception serait gérée si elle devait être levée.

Imaginez donc votre lecteur de musique, les clics de l'utilisateur s'arrêtent puis s'arrêtent à nouveau. Souhaitez-vous afficher un message d'erreur dans ce scénario? Je n'ai pas encore vu de joueur qui le fasse. Souhaitez-vous que le comportement de l'application soit différent d'un simple clic sur stop (comme enregistrer l'événement ou envoyer un rapport d'erreur en arrière-plan)? Probablement pas.

Cela signifie que l'exception devrait être capturée et avalée quelque part. Et cela signifie que vous feriez mieux de ne pas lever l'exception en premier lieu parce que vous ne voulez pas entreprendre une action différente .

Cela ne s'applique pas seulement aux exceptions, mais à tout type de branchement: fondamentalement, s'il n'y a pas de différence de comportement observable, il n'y a pas besoin de branchement.

Cela dit, il n'y a pas beaucoup de mal à définir votre stop()méthode pour renvoyer une booleanvaleur enum ou enum indiquant si l'arrêt a été "réussi". Vous ne l'utiliserez probablement jamais, mais une valeur de retour peut être plus facilement et naturellement ignorée qu'une exception.

biziclop
la source
2

Voici comment Android le fait: MediaPlayer

En un mot, stopquand startn'a pas été appelé n'est pas un problème, le système reste à l'arrêt, mais s'il est appelé sur un joueur qui ne sait même pas ce qu'il joue, une exception est levée, car il n'y a pas de bonne raison d'appeler arrêtez là.

njzk2
la source
1

Quelle devrait être la règle générale lorsqu'une méthode n'est pas censée être appelée dans certaines situations, mais que son exécution ne nuit pas au programme en général?

Je vois ce qui pourrait être une contradiction mineure dans votre déclaration qui pourrait clarifier la réponse pour vous. Pourquoi la méthode n'est-elle pas censée être appelée et pourtant son exécution ne fait pas de mal ?

Pourquoi la méthode "n'est pas censée être appelée"? Cela semble être une restriction auto-imposée. S'il n'y a pas de "dommage" ou d'impact sur votre API, il n'y a pas d'exception. Si la méthode ne doit vraiment pas être appelée car elle peut créer des états invalides ou imprévisibles, une exception doit être levée.

Par exemple, si je me dirigeais vers un lecteur de DVD et que j'appuyais sur «stop» avant d'appuyer sur «start» et que ça plantait, cela n'aurait aucun sens pour moi. Il devrait simplement rester là ou, pire cas, reconnaître "stop" sur l'écran. Une erreur serait ennuyeuse et ne serait d'aucune utilité.

Cependant, puis-je mettre un code d'alarme pour le désactiver "quand il est déjà éteint (appel de la méthode), ce qui peut le faire entrer dans un état qui l'empêcherait de l'armer correctement plus tard? Si c'est le cas, lancez une erreur même si, techniquement, "il n'y a pas eu de mal" car il peut y avoir un problème plus tard. Je voudrais en savoir même si ce n'est pas vraiment entré dans cet état. Juste la possibilité est suffisante. Ce serait un IllegalStateException.

Dans votre cas, si votre machine d'état peut gérer l'appel invalide / inutile, ignorez-le, sinon lancez une erreur.

ÉDITER:

Notez qu'un compilateur n'appelle pas d'erreur si vous définissez deux fois une variable sans inspecter la valeur. Cela ne vous empêche pas non plus de réinitialiser une variable. Il existe de nombreuses actions qui pourraient être considérées comme un "bug" d'un point de vue, mais qui sont simplement "inefficaces", "inutiles", "inutiles", etc. J'ai essayé de fournir cette perspective dans ma réponse - faire ces choses n'est généralement pas considéré un "bug" car il n'entraîne rien d'inattendu. Vous devriez probablement aborder votre problème de la même manière.

Jim
la source
Il n'est pas censé être appelé car l'appelant indique un bogue dans l'appelant.
user253751
@immibis: Pas nécessairement. Par exemple, cet appelant pourrait être un écouteur au clic d'un bouton de l'interface utilisateur. (En tant qu'utilisateur, vous
n'aimeriez
@meriton S'il s'agissait de l'écouteur au clic d'un bouton d'interface utilisateur, la réponse serait évidente: "faites ce que vous voulez si vous appuyez sur le bouton". De toute évidence, ce n'est pas le cas, c'est quelque chose de interne à l'application.
user253751
@immibis - J'ai modifié ma réponse. J'espère que ça aide.
Jim
1

Que font-ils MediaPlayer.play()et MediaPlayer.stop()font-ils exactement ? - sont-ils des écouteurs d' événements pour la saisie des utilisateurs ou sont-ils en fait des méthodes qui démarrent une sorte de flux multimédia sur le système? Si tout ce qu'ils sont sont des écouteurs pour les entrées des utilisateurs, alors il est parfaitement raisonnable qu'ils ne fassent rien (bien que ce soit une bonne idée de les enregistrer au moins quelque part). Cependant, s'ils affectent le modèle contrôlé par votre interface utilisateur, ils pourraient lancer un IllegalStateExceptioncar le joueur devrait avoir une sorte d' état isPlaying=trueou isPlaying=false(ce ne doit pas être un drapeau booléen réel comme écrit ici), et donc lorsque vous appelez MediaPlayer.stop()quand isPlaying=false, le La méthode ne peut pas réellement "arrêter" le MediaPlayercar l'objet n'est pas dans l'état approprié pour être arrêté - voir la description dujava.lang.IllegalStateException classe:

Signale qu'une méthode a été invoquée à un moment illégal ou inapproprié. En d'autres termes, l'environnement Java ou l'application Java n'est pas dans un état approprié pour l'opération demandée.

Pourquoi lever des exceptions peut être bon

Beaucoup de gens semblent penser que les exceptions sont mauvaises en général, car elles ne devraient jamais être levées (cf. Joel sur le logiciel ). Cependant, supposons que vous ayez un MediaPlayerGUI.notifyPlayButton()qui appelle MediaPlayer.play(), et MediaPlayer.play()invoque un tas d'autres codes et quelque part sur la ligne avec laquelle il s'interface, par exemple PulseAudio , mais moi (le développeur) ne sais pas où parce que je n'ai pas écrit tout ce code.

Puis, un jour en travaillant sur le code, je clique sur "play" et rien ne se passe. Si MediaPlayerGUIController.notifyPlayButton()je note quelque chose, au moins je peux regarder dans les journaux et vérifier que le clic sur le bouton a bien été enregistré ... mais pourquoi ne joue-t-il pas? Eh bien, disons qu'il y a en fait quelque chose de mal avec les wrappers PulseAudio qui MediaPlayer.play()invoque. Je clique à nouveau sur le bouton "jouer", et cette fois je reçois 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)

En regardant cette trace de pile, je peux tout ignorer avant de MediaPlayer.play()déboguer et passer mon temps à comprendre pourquoi, par exemple, aucun message n'est transmis à PulseAudio pour démarrer un flux audio.

D'un autre côté, si vous ne lancez pas d'exception, je n'aurai rien pour commencer à travailler avec autre que: "Le programme stupide ne joue aucune musique"; Bien que ce ne soit pas aussi mauvais qu'une erreur explicite se cachant , comme avaler une exception qui a en fait été levée , vous perdez quand même potentiellement le temps des autres quand quelque chose ne va pas ... alors soyez gentil et lancez-leur une exception.

errantlinguist
la source
0

Je préfère concevoir l'API d'une manière qui rend plus difficile ou impossible pour le consommateur de faire des erreurs. Par exemple, au lieu d'avoir MediaPlayer.play()et MediaPlayer.stop(), vous pouvez indiquer MediaPlayer.playToggle()qui bascule entre "arrêté" et "en cours de lecture". De cette façon, la méthode est toujours sûre d'appeler - il n'y a aucun risque d'entrer dans un état illégal.

Bien sûr, ce n'est pas toujours possible ou facile à faire. L'exemple que vous avez donné est comparable à essayer de supprimer un élément qui a déjà été supprimé de la liste. Vous pouvez soit être

  • pragmatique à ce sujet et ne jette aucune erreur. Cela simplifie certainement beaucoup de code, par exemple, au lieu d'avoir à écrire, if (list.contains(x)) { list.remove(x) }vous n'avez besoin que list.remove(x)). Mais, cela peut aussi cacher des bugs.
  • ou vous pouvez être pédant et signaler une erreur que la liste ne contient pas cet élément. Le code peut parfois devenir plus compliqué, car vous devez constamment vérifier si les conditions préalables sont remplies avant d'appeler la méthode, mais l'avantage est que les éventuels bogues sont plus faciles à localiser.

Si appeler MediaPlayer.stop()quand il est déjà arrêté n'a aucun mal dans votre application, alors je le laisserais s'exécuter silencieusement car cela simplifie le code et j'ai quelque chose pour les méthodes idempotentes. Mais si vous êtes absolument certain que MediaPlayer.stop()cela ne serait jamais appelé dans ces conditions, je lancerais une erreur car il pourrait y avoir un bogue ailleurs dans le code et l'exception aiderait à le retrouver.

Pedro Rodrigues
la source
3
Je ne suis pas d'accord pour playToggle()être plus facile à utiliser: pour obtenir l'effet souhaité (joueur jouant ou ne jouant pas), il vous faudrait connaître son état actuel. Donc, si vous obtenez une entrée utilisateur vous demandant d'arrêter le lecteur, vous devez d'abord vous demander si le lecteur est en cours de lecture, puis changer son état ou non en fonction de la réponse. C'est beaucoup plus lourd que d'appeler simplement une stop()méthode et d'en finir.
cmaster - réintègre monica
Eh bien, je n'ai jamais dit qu'il était plus facile à utiliser - j'ai prétendu que c'était plus sûr. De plus, votre exemple suppose que l'utilisateur recevra des boutons d'arrêt et de lecture distincts. Si tel était effectivement le cas, alors oui, un playToggleserait très lourd à utiliser. Mais si l'utilisateur avait également un bouton à bascule à la place, il playToggleserait plus facile à utiliser que jouer / arrêter.
Pedro Rodrigues