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

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?

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.

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.

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.

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.

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é.

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.

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.

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à.

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.

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.

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.