Co jest lepsze IllegalStateException lub cicha metoda wykonywania? [Zamknięte]

16

Powiedzmy, że mam klasę MediaPlayer, która ma metody play () i stop (). Jakiej najlepszej strategii użyć przy wdrażaniu metody zatrzymania w przypadku, gdy metoda odtwarzania nie była wcześniej wywoływana. Widzę dwie opcje: wyrzuć wyjątek, ponieważ odtwarzacz nie jest w odpowiednim stanie, lub po cichu zignoruj ​​wywołania metody stop.

Jaka powinna być ogólna zasada, gdy nie należy wywoływać metody w pewnej sytuacji, ale jej wykonanie nie szkodzi ogólnie programowi?

x2bool
źródło
21
Nie używaj wyjątków do modelowania zachowania, które nie jest wyjątkowe.
Alexander - Przywróć Monikę
1
Hej! Nie sądzę, że to duplikat. Moje pytanie dotyczy bardziej IllegalStateException, a powyższe pytanie dotyczy ogólnie stosowania wyjątków.
x2bool
1
Zamiast TYLKO po cichu zawieść, dlaczego NIE ZAREJESTROWAĆ również anomalii jako ostrzeżenia? Z pewnością Java, której używasz, obsługuje różne poziomy dziennika (ERROR, WARN, INFO, DEBUG).
Eric Seastrand
3
Zauważ, że Set.add nie zgłasza wyjątku, jeśli element jest już w zestawie. Jego celem jest „upewnienie się, że element jest w zestawie”, a nie „dodanie elementu do zestawu”, więc osiąga swój cel, nie robiąc nic, jeśli element jest już w zestawie.
user253751
2
Słowo dnia: idempotence
Jules

Odpowiedzi:

37

Nie ma reguły. Wszystko zależy od tego, jak chcesz, aby Twój interfejs API „czuł się”.

Osobiście w odtwarzacz muzyki, myślę, że przejście od stanu Stopped, aby Stoppedza pomocą metody Stop()jest całkowicie poprawny stan przejściowy. Nie ma to większego znaczenia, ale jest ważne. Mając to na uwadze, wprowadzenie wyjątku wydawałoby się pedantyczne i niesprawiedliwe. Sprawiłoby to, że API wydawałoby się społecznym odpowiednikiem rozmowy z irytującym dzieckiem w szkolnym autobusie. Utkniesz w irytującej sytuacji, ale możesz sobie z tym poradzić.

Bardziej „towarzyskim” podejściem jest przyznanie, że przejście w najgorszym przypadku jest nieszkodliwe i że należy na nie pozwolić życzliwym deweloperom.

Gdyby to zależało ode mnie, pominąłbym wyjątek.

MetaFight
źródło
15
Ta odpowiedź przypomina nam, że czasem dobrym pomysłem jest naszkicowanie przejść stanu dla nawet prostych interakcji.
6
Mając to na uwadze, wprowadzenie wyjątku wydawałoby się pedantyczne i niesprawiedliwe. Sprawiłoby to, że API wydawałoby się społecznym odpowiednikiem rozmowy z irytującym dzieckiem w szkolnym autobusie. > Wyjątki nie mają na celu ukarania cię: mają na celu poinformowanie cię, że wydarzyło się coś nieoczekiwanego. Osobiście byłbym bardzo wdzięczny za MediaPlayer.stop()metodę, która rzuciła IllegalStateExceptionzamiast godzin debugowania kodu i zastanawiała się, dlaczego do cholery „nic się nie dzieje” (tj. „To po prostu nie działa”).
errantlinguist
3
Oczywiście, jeśli Twój projekt mówi, że przejście w pętlę zwrotną jest nieprawidłowe, to TAK, powinieneś zgłosić wyjątek. Jednak moja odpowiedź dotyczyła projektu, w którym przejście jest całkowicie OK.
MetaFight,
1
StopOrThrow()brzmi okropnie. Jeśli idziesz tą alejką, dlaczego nie zastosować standardowego wzoru TryStop()? Ponadto, ogólnie rzecz biorąc, gdy zachowanie API jest niejasne (jak większość z nich), nie oczekuje się, że programiści po prostu zgadną lub eksperymentują, powinni przejrzeć dokumentację. Dlatego tak bardzo lubię MSDN.
MetaFight
1
Osobiście wolę opcję szybkiego działania, więc wybrałbym IllegalStateException, aby wskazać użytkownikowi interfejsu API, że coś jest nie tak z jego logiką, nawet jeśli jest to nieszkodliwe. Często zdarzają mi się te wyjątki z mojego własnego kodu, bardzo pomocne w wykrywaniu, że moja niezrozumiała praca jest już błędna
Walfrat
17

Istnieją dwa różne rodzaje działań, które możesz chcieć wykonać:

  1. Jednocześnie przetestuj, czy coś jest w jednym stanie i zmień to na inny.

  2. Ustaw coś na konkretny stan, bez względu na poprzedni stan.

Niektóre konteksty wymagają jednej akcji, a inne wymagają drugiej. Jeśli odtwarzacz multimedialny, który osiągnie koniec zawartości, pozostanie w stanie „odtwarzania”, ale z zamrożoną pozycją na końcu, wówczas metoda, która jednocześnie zapewnia, że ​​odtwarzacz był w stanie odtwarzania, ustawiając go na „zatrzymanie” stan może być przydatny, jeśli kod chciałby się upewnić, że nic nie spowodowało niepowodzenia odtwarzania przed żądaniem zatrzymania (co może być ważne, jeśli np. coś nagrywało odtwarzaną zawartość jako sposób konwersji multimediów z jednego formatu na inny) . W większości przypadków ważne jest jednak to, że po operacji odtwarzacz multimedialny jest w oczekiwanym stanie (tj. Zatrzymany).

Jeśli odtwarzacz multimediów automatycznie zatrzyma się, gdy osiągnie koniec multimediów, funkcja, która zapewnia, że ​​odtwarzacz był uruchomiony, byłaby prawdopodobnie bardziej irytująca niż pomocna, ale w niektórych przypadkach wiedza o tym, czy odtwarzacz działał w momencie zatrzymania, mogłaby być użytecznym. Być może najlepszym podejściem do zaspokojenia obu potrzeb byłoby zwrócenie wartości funkcji wskazującej poprzedni stan odtwarzacza. Kod dbający o ten stan może sprawdzać wartość zwracaną; kod, który go nie obchodzi, może go po prostu zignorować.

supercat
źródło
2
+1 za notatkę o przywróceniu starego stanu: Pozwala to na implementację go w sposób, który sprawia, że ​​cała operacja (sprawdzanie statusu i przejście do określonego statusu) jest atomowa, co jest przynajmniej przyjemną funkcją. Ułatwiłoby to napisanie solidnego kodu użytkownika, który nie zawiedzie się sporadycznie z powodu dziwnych warunków wyścigu.
cmaster
A bool TryStop()i void Stop()przykład kodu sprawiłby, że jest to naprawdę doskonała odpowiedź.
RubberDuck,
2
@RubberDuck: To nie byłby dobry wzór. Nazwa TryStopsugeruje, że wyjątek nie powinien zostać zgłoszony, jeśli gracz nie może zostać zmuszony do zatrzymania. Próba zatrzymania odtwarzacza, gdy jest już zatrzymany, nie jest wyjątkowym warunkiem, ale jest stanem, który może być zainteresowany dzwoniącym.
supercat
1
Potem źle zrozumiałem twoją odpowiedź @supercat
RubberDuck
2
Chciałbym zauważyć, że testowanie i ustawianie w ten sposób NIE jest naruszeniem prawa demeter, pod warunkiem, że API zapewnia również sposób testowania stanu odtwarzania bez jego zmiany. Powiedzmy, że może to być zupełnie inna metoda isPlaying().
candied_orange
11

Nie ma ogólnej zasady. W tym konkretnym przypadku intencją użytkownika interfejsu API jest powstrzymanie odtwarzacza przed odtwarzaniem multimediów. Jeśli odtwarzacz nie odtwarza multimediów, MediaPlayer.stop()może nic nie robić, a cel metody wywołującej nadal zostanie osiągnięty - media nie odtwarzają.

Zgłoszenie wyjątku wymagałoby od użytkownika interfejsu API sprawdzenia, czy gracz aktualnie gra lub złapania i obsługi wyjątku. Sprawiłoby to, że interfejs API byłby większym obowiązkiem.

kmaczuga
źródło
11

Wyjątki nie mają na celu zasygnalizowania, że ​​stało się coś złego; ma to sygnalizować

  1. Stało się coś złego
  2. że nie wiem jak to naprawić tutaj
  3. że osoba dzwoniąca lub coś z tego zestawu powinna wiedzieć, jak sobie z tym poradzić
  4. więc ratuję się szybko, zatrzymuję wykonanie bieżącej ścieżki kodu, aby zapobiec uszkodzeniu lub uszkodzeniu danych, i pozostawiam abonentowi do wyczyszczenia

Jeśli dzwoniący próbuje Stopuzyskać stan, MediaPlayerktóry jest już w stanie zatrzymania, nie jest to problem, którego MediaPlayernie można rozwiązać (może po prostu nic nie zrobić). Nie jest to problem, który, jeśli będzie kontynuowany, doprowadzi do uszkodzenia lub danych korupcja, ponieważ może być skuteczna po prostu przez nic nie robienie. I nie jest tak naprawdę problemem, że rozsądniej jest oczekiwać, że rozmówca będzie w stanie rozwiązać problem niż MediaPlayer.

Dlatego nie powinieneś zgłaszać wyjątku w tej sytuacji.

Mason Wheeler
źródło
+1. To pierwsza odpowiedź, która pokazuje, dlaczego wyjątek nie jest tutaj odpowiedni. Wyjątki wskazują, że osoba dzwoniąca prawdopodobnie będzie musiała wiedzieć o wyjątkowym przypadku. W takim przypadku prawie na pewno klient danej klasy nie będzie dbał o to, czy wywołanie „stop ()” faktycznie spowodowało zmianę stanu, więc prawdopodobnie nie chciałby wiedzieć o wyjątku.
Jules
5

Spójrz na to w ten sposób:

Jeśli klient wywoła Stop (), gdy odtwarzacz nie gra, wówczas Stop () automatycznie zakończy się powodzeniem, ponieważ odtwarzacz jest obecnie w stanie zatrzymania.

17 z 26
źródło
3

Zasadą jest, że robisz to, czego wymaga umowa twojej metody.

Widzę wiele sposobów definiowania znaczących umów dla takiej stopmetody. Może to być całkowicie poprawne, jeśli stopmetoda nie robi absolutnie nic, jeśli gracz nie gra. W takim przypadku definiujesz swój interfejs API według jego celów. Chcesz przejść do stoppedstanu, a stopmetoda robi to - po prostu przechodząc ze stoppedstanu do siebie bez błędu.

Czy jest jakiś powód, dla którego chcesz zgłosić wyjątek? Jeśli kod użytkownika będzie wyglądał tak na końcu:

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

to nie ma żadnej korzyści z posiadania tego wyjątku. Pytanie brzmi: czy użytkownikowi zależy na tym, że odtwarzacz został już zatrzymany? Czy on ma jeszcze jedno pytanie, żeby to zapytać?

Gracz może być obserwowalny i może już powiadamiać obserwatorów o pewnych rzeczach - np. Powiadamiając obserwatorów, kiedy przechodzą od playingdo stoppingdo stopped. W takim przypadku zgłoszenie wyjątku przez metodę nie jest konieczne. Wywołanie stopnic nie zrobi, jeśli gracz jest już zatrzymany, a wywołanie go, gdy nie zostanie zatrzymane, powiadomi obserwatorów o przejściu.

w sumie zależy to od tego, gdzie skończy się twoja logika. Ale myślę, że są lepsze opcje projektowania niż rzucenie wyjątku.

Powinieneś rzucić wyjątek, jeśli dzwoniący musi interweniować. W tym przypadku nie musi, możesz po prostu pozwolić, aby gracz był taki, jaki jest i nadal działa.

Polygnome
źródło
3

Istnieje prosta ogólna strategia, która pomaga w podjęciu tej decyzji.

Zastanów się, jak zostałby obsłużony wyjątek, gdyby miał zostać zgłoszony.

Więc wyobraź sobie swój odtwarzacz muzyki, kliknięcia użytkownika zatrzymaj, a następnie zatrzymaj ponownie. Czy chcesz wyświetlić komunikat o błędzie w tym scenariuszu? Nie widziałem jeszcze takiego gracza. Czy chcesz, aby zachowanie aplikacji było inne niż kliknięcie raz zatrzymaj (np. Zaloguj zdarzenie lub wyślij raport o błędzie w tle)? Prawdopodobnie nie.

Oznacza to, że wyjątek musiałby zostać gdzieś złapany i połknięty. A to oznacza, że ​​lepiej nie rzucać wyjątkiem, ponieważ nie chcesz podejmować innych działań .

Nie dotyczy to tylko wyjątków, ale wszelkiego rodzaju rozgałęzień: w zasadzie jeśli nie ma zauważalnej różnicy w zachowaniu, nie ma potrzeby rozgałęziania.

To powiedziawszy, nie ma żadnej szkody w definiowaniu stop()metody zwracania booleanwartości enum lub wskazującej, czy zatrzymanie zakończyło się powodzeniem. Prawdopodobnie nigdy go nie użyjesz, ale wartość zwracaną można łatwiej i naturalnie zignorować niż wyjątek.

biziclop
źródło
2

Oto jak to robi Android: MediaPlayer

Krótko mówiąc, stopkiedy startnie został wywołany, nie stanowi problemu, system pozostaje w stanie zatrzymania, ale jeśli zostanie wezwany do gracza, który nawet nie wie, w co gra, zostanie zgłoszony wyjątek, ponieważ nie ma dobrego powodu, aby zadzwonić Zatrzymaj się tam.

njzk2
źródło
1

Jaka powinna być ogólna zasada, gdy nie należy wywoływać metody w pewnej sytuacji, ale jej wykonanie nie szkodzi ogólnie programowi?

Widzę, co może być niewielką sprzecznością w twoim oświadczeniu, które może wyjaśnić ci odpowiedź. Dlaczego nie należy wywoływać metody, a mimo to jej wykonanie nie szkodzi ?

Dlaczego metoda „nie powinna być wywoływana”? To wydaje się być narzuconym przez siebie ograniczeniem. Jeśli nie ma „szkody” ani wpływu na interfejs API, nie ma wyjątku. Jeśli metoda naprawdę nie powinna zostać wywołana, ponieważ może tworzyć niepoprawne lub nieprzewidywalne stany, należy zgłosić wyjątek.

Na przykład, jeśli podszedłem do odtwarzacza DVD i wcisnąłem „stop” przed uderzeniem w „start” i to się zawiesiło, nie miałoby to dla mnie sensu. Powinien po prostu tam usiąść lub, co gorsza, potwierdzić „zatrzymanie” na ekranie. Błąd byłby irytujący i nie byłby w żaden sposób pomocny.

Czy mogę jednak wprowadzić kod alarmu, aby wyłączyć go, gdy jest już wyłączony (wywołanie metody), co może spowodować, że przejdzie w stan uniemożliwiający prawidłowe uzbrojenie go później? Jeśli tak, wyślij błąd, chociaż technicznie „nie doszło do szkody”, ponieważ może wystąpić problem później. Chciałbym o tym wiedzieć, nawet jeśli tak naprawdę nie przejdzie do tego stanu. Wystarczy taka możliwość. To byłoby IllegalStateException.

W twoim przypadku, jeśli maszyna stanu może obsłużyć niepoprawne / niepotrzebne wywołanie, zignoruj ​​je, w przeciwnym razie wyrzuć błąd.

EDYTOWAĆ:

Zauważ, że kompilator nie wywołuje błędu, jeśli ustawisz zmienną dwa razy bez sprawdzania wartości. Nie zapobiega to również ponownej inicjalizacji zmiennej. Istnieje wiele działań, które można uznać za „błąd” z jednej perspektywy, ale są one po prostu „nieefektywne”, „niepotrzebne”, „bezcelowe” itp. Próbowałem podać tę perspektywę w mojej odpowiedzi - robienie tych rzeczy nie jest ogólnie brane pod uwagę „błąd”, ponieważ nie powoduje niczego nieoczekiwanego. Prawdopodobnie powinieneś podejść do problemu w podobny sposób.

Jim
źródło
Nie należy go wywoływać, ponieważ wywołanie go oznacza błąd w dzwoniącym.
user253751
@immibis: Niekoniecznie. Na przykład osoba dzwoniąca może być odbiornikiem kliknięcia przycisku interfejsu użytkownika po kliknięciu. (Jako użytkownik prawdopodobnie nie spodoba ci się komunikat o błędzie, jeśli przypadkowo klikniesz przycisk zatrzymania, jeśli media są już zatrzymane.)
meriton
@meriton Jeśli byłby to nasłuchiwanie przycisku interfejsu użytkownika po kliknięciu, odpowiedź byłaby oczywista - „rób, co chcesz, jeśli przycisk zostanie naciśnięty”. Oczywiście nie jest, jest to coś wewnętrznego w aplikacji.
user253751
@immibis - Zredagowałem swoją odpowiedź. Mam nadzieję, że to pomoże.
Jim
1

Co dokładnie zrobić MediaPlayer.play()i MediaPlayer.stop()zrobić - są one zdarzeń słuchaczy do danych wprowadzonych przez użytkownika lub są oni faktycznie metody, które rozpoczynają jakiegoś strumienia mediów w systemie? Jeśli są tylko słuchaczami wprowadzanymi przez użytkownika, to jest całkowicie rozsądne, aby nic nie robili (chociaż dobrym pomysłem byłoby ich gdzieś zalogować). Jednakże, jeżeli mają one wpływ na modelu Twój UI kontroluje, a następnie mogli rzucać IllegalStateExceptionponieważ gracz powinien mieć jakiś isPlaying=truelub isPlaying=falsestan (to nie musi być rzeczywisty Boolean flagę jak napisane tutaj), a więc, gdy dzwonisz MediaPlayer.stop(), kiedy isPlaying=falseThe metoda nie może faktycznie „zatrzymać”, MediaPlayerponieważ obiekt nie jest w odpowiednim stanie do zatrzymania - zobacz opisjava.lang.IllegalStateException klasa:

Sygnalizuje, że metoda została wywołana w nielegalnym lub nieodpowiednim czasie. Innymi słowy, środowisko Java lub aplikacja Java nie jest w odpowiednim stanie dla żądanej operacji.

Dlaczego wprowadzanie wyjątków może być dobre

Wydaje się, że wiele osób uważa, że ​​wyjątki są złe, ponieważ nigdy nie należy ich rzucać (por. Joel w sprawie oprogramowania ). Powiedzmy jednak, że masz MediaPlayerGUI.notifyPlayButton()wywoływanie MediaPlayer.play(), które MediaPlayer.play()wywołuje wiele innych kodów i gdzieś poniżej łączy się z np. PulseAudio , ale ja (programista) nie wiem, gdzie, ponieważ nie napisałem całego tego kodu.

Pewnego dnia podczas pracy nad kodem klikam „zagraj” i nic się nie dzieje. Jeśli MediaPlayerGUIController.notifyPlayButton()coś loguje, przynajmniej mogę zajrzeć do logów i sprawdzić, czy kliknięcie przycisku zostało zarejestrowane ... ale dlaczego nie gra? Powiedzmy, że coś jest nie tak z opakowaniami PulseAudio, które MediaPlayer.play()wywołują. Ponownie klikam przycisk „Odtwórz” i tym razem otrzymuję 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)

Patrząc na ślad stosu, mogę zignorować wszystko przed MediaPlayer.play()debugowaniem i poświęcić czas na zastanawianie się, dlaczego np. Nie jest przekazywana wiadomość do PulseAudio w celu uruchomienia strumienia audio.

Z drugiej strony, jeśli nie wyrzucisz wyjątku, nie będę miał nic do pracy poza: „Głupi program nie odtwarza żadnej muzyki”; Chociaż nie jest tak zły, jak jawne ukrywanie błędów, takie jak faktyczne połykanie wyjątku, który został faktycznie zgłoszony , wciąż potencjalnie marnujesz czas innych, gdy coś pójdzie nie tak ... więc bądź miły i rzuć im wyjątek.

errantlinguist
źródło
0

Wolę zaprojektować interfejs API w sposób, który utrudnia lub uniemożliwia konsumentowi popełnienie błędów. Na przykład zamiast MediaPlayer.play()i MediaPlayer.stop()możesz podać, MediaPlayer.playToggle()które przełącza między „zatrzymaniem” a „graniem”. W ten sposób metoda jest zawsze bezpieczna do wywołania - nie ma ryzyka przejścia w stan nielegalny.

Oczywiście nie zawsze jest to możliwe lub łatwe do zrobienia. Podany przykład jest porównywalny z próbą usunięcia elementu, który został już usunięty z listy. Możesz być albo

  • pragmatycznie i nie rzucajcie żadnych błędów. To z pewnością upraszcza wiele kodu, np. Zamiast pisać if (list.contains(x)) { list.remove(x) }, potrzebujesz tylko list.remove(x)). Ale może również ukrywać błędy.
  • lub możesz być pedantyczny i zasygnalizować błąd, że lista nie zawiera tego elementu. Czasami kod może się komplikować, ponieważ przed wywołaniem metody musisz stale sprawdzać, czy spełnione są warunki wstępne, ale zaletą jest to, że ewentualne błędy są łatwiejsze do wyśledzenia.

Jeśli wywoływanie, MediaPlayer.stop()gdy jest już zatrzymane, nie ma żadnej szkody w twojej aplikacji, pozwoliłbym mu działać cicho, ponieważ upraszcza kod i mam coś do idempotentnych metod. Ale jeśli jesteś absolutnie pewien, MediaPlayer.stop()że nigdy nie zostanie wywołany w tych warunkach, zwrócę błąd, ponieważ może być błąd w innym miejscu kodu, a wyjątek pomoże go wyśledzić.

Pedro Rodrigues
źródło
3
Nie zgadzam się, że jestem playToggle()łatwiejszy w użyciu: Aby osiągnąć pożądany efekt (grający lub nie grający), musisz znać jego obecny stan. Tak więc, jeśli dostaniesz od użytkowników prośbę o zatrzymanie odtwarzacza, najpierw musisz zapytać, czy gracz aktualnie gra, a następnie przełączyć jego stan, czy nie, w zależności od odpowiedzi. Jest to o wiele bardziej kłopotliwe niż zwykłe wywoływanie stop()metody i wykonywanie jej.
cmaster
Cóż, nigdy nie powiedziałem, że jest łatwiejszy w użyciu - twierdziłem, że jest bezpieczniejszy. Ponadto twój przykład zakłada, że ​​użytkownik otrzyma osobne przyciski zatrzymania i odtwarzania. Gdyby tak rzeczywiście było, to tak, a playTogglebyłoby bardzo kłopotliwe w użyciu. Ale jeśli użytkownik otrzymałby również przycisk przełączania, playTogglebyłby łatwiejszy w użyciu niż odtwarzanie / zatrzymywanie.
Pedro Rodrigues,