Что лучше IllegalStateException или тихое выполнение метода? [закрыто]

16

Допустим, у меня есть класс MediaPlayer, в котором есть методы play () и stop (). Какую стратегию лучше всего использовать при реализации метода stop в случае, если метод play ранее не вызывался. Я вижу два варианта: генерировать исключение, потому что игрок не находится в соответствующем состоянии, или молча игнорировать вызовы метода stop.

Каково должно быть общее правило, когда метод не должен вызываться в некоторой ситуации, но его выполнение не вредит программе в целом?

x2bool
источник
21
Не используйте исключения для моделирования поведения, которое не является исключительным.
Александр - Восстановить Монику
1
Привет! Я не думаю, что это дубликат. Мой вопрос больше касается IllegalStateException, и вышеупомянутый вопрос касается использования исключений в целом.
x2bool
1
Вместо того, чтобы просто молча терпеть неудачу, почему бы ТАКЖЕ не регистрировать аномалию как предупреждение? Конечно, используемая вами Java поддерживает разные уровни журналов (ERROR, WARN, INFO, DEBUG).
Эрик Сеастранд
3
Обратите внимание, что Set.add не выдает исключение, если элемент уже находится в наборе. Его цель - «убедиться, что элемент находится в наборе», а не «добавить элемент в набор», поэтому он достигает своей цели, ничего не делая, если элемент уже находится в наборе.
user253751
2
Слово дня: идемпотентность
Жюль

Ответы:

37

Там нет правила. Все зависит от того, как вы хотите, чтобы ваш API "чувствовал".

Лично в музыкальном проигрывателе, я думаю, что переход от состояния Stoppedк Stoppedс помощью метода Stop()является вполне допустим переходным состоянием. Это не очень значимо, но это действительно. Имея это в виду, бросить исключение может показаться педантичным и несправедливым. Это заставит API почувствовать себя социальным эквивалентом разговора с надоедливым ребенком в школьном автобусе. Вы застряли с неприятной ситуацией, но вы можете справиться с этим.

Более «общительный» подход заключается в признании того, что переход в худшем случае безвреден, и будьте добры к потребляющим разработчикам, разрешив его.

Так что, если бы это зависело от меня, я бы пропустил исключение.

MetaFight
источник
15
Этот ответ напоминает нам, что иногда хорошей идеей является набросок переходов между состояниями даже для простых взаимодействий.
6
Имея это в виду, бросить исключение может показаться педантичным и несправедливым. Это заставит API почувствовать себя социальным эквивалентом разговора с надоедливым ребенком в школьном автобусе. > Исключения не предназначены для того, чтобы наказать вас: они предназначены для того, чтобы сообщить вам, что произошло что-то неожиданное. Лично я был бы очень благодарен за MediaPlayer.stop()метод, который бросил IllegalStateExceptionвместо того, чтобы тратить часы на отладку кода и удивляться, почему, черт возьми, «ничего не происходит» (то есть «это просто не работает»).
errantlinguist
3
Конечно, если ваш дизайн говорит, что петлевой переход недействителен, тогда ДА, вы должны выбросить исключение. Тем не менее, мой ответ был о дизайне, где этот переход совершенно нормально.
MetaFight
1
StopOrThrow()звучит ужасно. Если вы идете по этому переулку, почему бы не использовать стандартный шаблон TryStop()? Кроме того, как правило, когда поведение API неясно (как и большинство из них), разработчики не должны просто догадываться или экспериментировать, они должны смотреть на документацию. Вот почему мне так нравится MSDN.
MetaFight
1
Лично я предпочитаю вариант fast-fast, поэтому я бы выбрал IllegalStateException, чтобы указать пользователю API, что в его логике что-то не так, даже если это безвредно. Я часто получаю эти исключения из своего собственного кода, что очень помогает обнаружить, что моя необдуманная работа уже ошибочна
Уолфрат
17

Существует два вида действий, которые можно выполнить:

  1. Одновременно проверьте, что что-то находится в одном состоянии, и измените его на другое.

  2. Установите что-то в определенное состояние, не обращая внимания на предыдущее состояние.

Некоторые контексты требуют одного действия, а некоторые требуют другого. Если медиаплеер, который достигает конца контента, останется в состоянии «воспроизведения», но с позицией, замороженной в конце, тогда метод, который одновременно утверждает, что проигрыватель находился в состоянии воспроизведения, при установке его в «стоп» состояние может быть полезно, если код захочет убедиться, что ничто не вызывало сбой воспроизведения до запроса на остановку (что может быть важно, если, например, что-то записывает воспроизводимый контент в качестве средства преобразования медиа из одного формата в другой) , Однако в большинстве случаев имеет значение то, что после операции медиаплеер находится в ожидаемом состоянии (то есть остановлен).

Если медиаплеер автоматически останавливается, когда достигает конца носителя, функция, которая утверждает, что проигрыватель работал, скорее всего, будет более раздражающей, чем полезной, но в некоторых случаях знание, работает ли проигрыватель во время его остановки, может быть полезным. Возможно, лучший подход для удовлетворения обеих потребностей состоит в том, чтобы функция возвращала значение, указывающее предыдущее состояние игрока. Код, который заботится об этом состоянии, может проверить возвращаемое значение; код, который не заботится, мог бы просто проигнорировать это.

Supercat
источник
2
+1 за примечание о возврате старого состояния: это позволяет реализовать его таким образом, чтобы вся операция (запрос статуса и переход к определенному состоянию) была атомарной, что, по крайней мере, является приятной функцией. Это облегчит написание надежного пользовательского кода, который периодически не выходит из строя из-за какого-то странного состояния гонки.
cmaster - восстановить монику
А bool TryStop()и void Stop()пример кода сделали бы это действительно превосходным ответом.
RubberDuck
2
@RubberDuck: Это не будет хорошим примером. Название TryStopподразумевает, что исключение не должно быть выброшено, если игрок не может быть переведен в остановленное состояние. Попытка остановить игрока, когда он уже остановлен, не является исключительным условием, но это условие, которое может заинтересовать звонящего.
Суперкат
1
Тогда я неправильно понял ваш ответ @supercat
RubberDuck
2
Я хотел бы отметить, что тестирование и установка этого способа НЕ является нарушением закона деметры, при условии, что API также предоставляет способ проверить состояние воспроизведения, не изменяя его. Это может быть совершенно другой метод, скажем isPlaying().
candied_orange
11

Там нет общего правила. В этом конкретном случае, намерение пользователя вашего API состоит в том, чтобы остановить проигрыватель от воспроизведения мультимедиа. Если проигрыватель не воспроизводит носитель, он MediaPlayer.stop()может ничего не делать, и цель вызывающего метода все равно будет достигнута - носитель не воспроизводится.

Создание исключения потребовало бы от пользователя API либо проверки того, играет ли игрок в данный момент, либо перехвата и обработки исключения. Это сделало бы API более рутинным в использовании.

kmaczuga
источник
11

Цель исключений - не сигнализировать о том, что случилось что-то плохое; это сигнализировать, что

  1. Случилось что-то плохое
  2. что я не знаю, как исправить здесь
  3. что вызывающий, или что-то из стека вызовов, должен знать, что делать с
  4. поэтому я быстро выручаю, прекращаю выполнение текущего пути к коду, чтобы предотвратить повреждение или повреждение данных, и оставляю его вызывающей стороне для очистки

Если вызывающий абонент пытается выполнить Stopвызов, MediaPlayerкоторый уже находится в остановленном состоянии, это не является проблемой, MediaPlayerкоторую невозможно решить (он может просто ничего не делать). Это не проблема, которая, если ее продолжить, приведет к повреждению или повреждению данных. коррупция, поскольку она может быть успешной, просто ничего не делая. И на самом деле проблема не в том, что разумнее ожидать, что вызывающий абонент сможет решить проблему, чемMediaPlayer .

Поэтому не следует бросать исключение в этой ситуации.

Мейсон Уилер
источник
+1. Это первый ответ, который показывает, почему исключение здесь не подходит. Исключения указывают на то, что вызывающему абоненту, вероятно, необходимо знать об исключительном случае. В этом случае почти наверняка клиент рассматриваемого класса не будет заботиться о том, действительно ли вызов stop () вызвал переход состояния, поэтому, вероятно, не захочет знать об исключении.
Жюль
5

Рассмотрим этот вариант:

Если клиент вызывает Stop (), когда игрок не играет, то Stop () автоматически успешен, потому что игрок в данный момент находится в состоянии остановки.

17 из 26
источник
3

Правило заключается в том, что вы делаете то, что требует контракт вашего метода.

Я вижу несколько способов определения значимых контрактов для такого stopметода. Это может быть совершенно справедливо, если stopметод абсолютно ничего не делает, если игрок не играет. В этом случае вы определяете свой API по его целям. Вы хотите перейти в stoppedсостояние, и stopметод делает это - просто переходя изstopped состояния в себя без ошибок.

Есть ли какая-то причина, по которой вы хотели бы создать исключение? Если код пользователя в конце будет выглядеть так:

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

тогда нет смысла иметь это исключение. Таким образом, вопрос в том, будет ли пользователь беспокоиться о том, что плеер уже был остановлен? У него есть еще одна ва, чтобы сделать запрос?

Игрок может быть наблюдаемым и может уже уведомлять наблюдателей об определенных вещах - например, уведомлять наблюдателей, когда он переходит от playingк stoppingк stopped. В этом случае наличие метода throw исключение не обязательно. Вызов stopпросто ничего не сделает, если игрок уже остановлен, а вызов, когда он не остановлен, уведомит наблюдателей о переходе.

В общем, это зависит от того, где ваша логика будет в конечном итоге. Но я думаю, что есть лучший выбор дизайна, чем исключение.

Вы должны бросить исключение, если звонящий должен вмешаться. В этом случае ему не нужно, вы можете просто оставить игрока таким, какой он есть, и он продолжает работать.

Polygnome
источник
3

Существует простая общая стратегия, которая поможет вам принять это решение.

Подумайте, как будет обрабатываться исключение, если оно будет выдано.

Итак, представьте ваш музыкальный проигрыватель, пользователь нажимает кнопку «Стоп», а затем снова останавливается. Вы хотите отобразить сообщение об ошибке в этом сценарии? Я еще не видел игрока, который делает. Вы хотите, чтобы поведение приложения отличалось от простого нажатия кнопки «Остановить» один раз (например, регистрация события или отправка отчета об ошибке в фоновом режиме)? Возможно нет.

Это означает, что исключение нужно будет где-то перехватить и проглотить. А это означает, что вам лучше не создавать исключение, потому что вы не хотите предпринимать другие действия .

Это относится не только к исключениям, но и к любому виду ветвления: в основном, если не требуется заметной разницы в поведении, нет необходимости в ветвлении.

Тем не менее, нет особого вреда в определении вашего stop()метода для возврата значения a booleanили enum, указывающего, была ли остановка успешной. Вы, вероятно, никогда не будете его использовать, но возвращаемое значение может быть более легко и естественно проигнорировано, чем исключение.

biziclop
источник
2

Вот как это делает Android: MediaPlayer

Короче говоря, stopесли startне было вызвано, это не проблема, система остается в остановленном состоянии, но при вызове игрока, который даже не знает, что играет, возникает исключение, потому что нет веских причин для вызова. останавливаться на достигнутом.

njzk2
источник
1

Каково должно быть общее правило, когда метод не должен вызываться в некоторой ситуации, но его выполнение не вредит программе в целом?

Я вижу, что может быть незначительным противоречием в вашем утверждении, которое может дать вам ясный ответ. Почему метод не должен вызываться, а его выполнение не вредит ?

Почему метод «не должен вызываться»? Это похоже на добровольное ограничение. Если нет никакого вреда или влияния на ваш API, то нет и исключения. Если метод действительно не должен вызываться, потому что он может создавать недопустимые или непредсказуемые состояния, то должно быть сгенерировано исключение.

Например, если я подошел к DVD-плееру и нажал «стоп», прежде чем нажать «старт», и он рухнул, для меня это не имело бы смысла. Он должен просто сидеть или, в худшем случае, подтверждать «стоп» на экране. Ошибка была бы раздражающей и бесполезной в любом случае.

Однако можно ли ввести код тревоги, чтобы отключить его, когда он уже выключен (вызывая метод), что может привести к тому, что он войдет в состояние, которое помешает правильной постановке на охрану позже? Если это так, то выдайте ошибку, хотя, технически, «не было никакого вреда», потому что позже может возникнуть проблема. Я хотел бы знать об этом, даже если бы это фактически не входило в это состояние. Просто возможности достаточно. Это было бы IllegalStateException.

В вашем случае, если ваш конечный автомат может обработать недействительный / ненужный вызов, игнорируйте его, в противном случае выведите ошибку.

РЕДАКТИРОВАТЬ:

Обратите внимание, что компилятор не вызывает ошибку, если вы устанавливаете переменную дважды без проверки значения. Это также не мешает вам повторно инициализировать переменную. Есть много действий, которые можно считать «ошибкой» с одной точки зрения, но они являются просто «неэффективными», «ненужными», «бессмысленными» и т. Д. Я попытался представить эту точку зрения в своем ответе - выполнение этих вещей обычно не считается «ошибка», потому что это не приводит к чему-либо неожиданному. Вы, вероятно, должны подходить к вашей проблеме аналогично.

Джим
источник
Он не должен вызываться, потому что его вызов указывает на ошибку в вызывающей стороне.
user253751
@immibis: не обязательно. Например, этот вызывающий абонент может быть прослушивателем по нажатию некоторой кнопки пользовательского интерфейса. (Как пользователь, вам, вероятно, не понравится сообщение об ошибке, если вы случайно нажмете кнопку остановки, если носитель уже остановлен.)
meriton
@meriton Если бы это был прослушиватель кнопки пользовательского интерфейса при нажатии, то ответ был бы очевиден - «делай все, что хочешь, если нажата кнопка». Понятно, что это не так, это что-то внутреннее для приложения.
user253751
@immibis - я отредактировал свой ответ. Я надеюсь, что это помогает.
Джим
1

Что именно делают MediaPlayer.play()и MediaPlayer.stop()делают? - они слушатели событий для пользовательского ввода или они на самом деле методы, которые запускают какой-то поток медиа в системе? Если все они являются слушателями пользовательского ввода, то для них совершенно разумно ничего не делать (хотя было бы неплохо хотя бы зарегистрировать их где-нибудь). Однако, если они влияют на модель, которой управляет ваш пользовательский интерфейс, они могут выдать, IllegalStateExceptionпотому что у игрока должно быть какое-то состояние isPlaying=trueили isPlaying=falseсостояние (это не должен быть фактический логический флаг, как написано здесь), и поэтому, когда вы вызываете MediaPlayer.stop()когда isPlaying=false, метод не может на самом деле «остановить», MediaPlayerпотому что объект не находится в соответствующем состоянии для остановки - см. описаниеjava.lang.IllegalStateException учебный класс:

Сигналы о том, что метод был вызван в недопустимое или неподходящее время. Другими словами, среда Java или приложение Java не находятся в надлежащем состоянии для запрошенной операции.

Почему бросать исключения может быть хорошо

Многие люди, кажется, думают, что исключения в целом плохие, так как они никогда не должны быть выброшены (см. Джоэл по программному обеспечению ). Однако, скажем, у вас есть метод, MediaPlayerGUI.notifyPlayButton()который вызывает MediaPlayer.play(), и MediaPlayer.play()вызывает кучу другого кода и где-то вниз по линии, с которой он взаимодействует, например, PulseAudio , но я (разработчик) не знаю, где, потому что я не писал весь этот код.

Затем, однажды, работая над кодом, я нажимаю «играть», и ничего не происходит. Если MediaPlayerGUIController.notifyPlayButton()что-то регистрируется, по крайней мере, я могу просмотреть журналы и убедиться, что нажатие кнопки действительно было зарегистрировано ... но почему оно не воспроизводится? Ну, скажем, на самом деле что-то не так с обертками PulseAudio, которые MediaPlayer.play()вызывают. Я снова нажимаю кнопку «играть», и на этот раз я получаю 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)

Просматривая эту трассировку стека, я могу игнорировать все ранее MediaPlayer.play()при отладке и тратить свое время на выяснение того, почему, например, в PulseAudio не передается сообщение для запуска аудиопотока.

С другой стороны, если вы не выбросите исключение, мне не за что будет работать, кроме: «Глупая программа не воспроизводит музыку»; Хотя это и не так плохо , как явное сокрытие ошибок , таких , как на самом деле глотанием исключение , которое было на самом деле бросили , вы все еще потенциально тратить время чужое , когда что - то пойдет не так ... так быть красиво и бросить исключение на них.

errantlinguist
источник
0

Я предпочитаю разрабатывать API таким образом, чтобы потребителю было трудно или невозможно ошибиться. Например, вместо « MediaPlayer.play()и» MediaPlayer.stop()вы можете указать, MediaPlayer.playToggle()какие переключатели между «остановлен» и «играет». Таким образом, метод всегда безопасен для вызова - нет риска попасть в нелегальное состояние.

Конечно, это не всегда возможно или легко сделать. Приведенный вами пример сопоставим с попыткой удалить элемент, который уже был удален из списка. Вы можете быть

  • прагматично об этом и не выкидывай никаких ошибок. Это, безусловно, упрощает много кода, например, вместо того, чтобы писать if (list.contains(x)) { list.remove(x) }только вам нужно list.remove(x)). Но это также может скрывать ошибки.
  • или вы можете быть педантичным и сообщать об ошибке, что список не содержит этот элемент. Временами код может усложняться, так как вам постоянно приходится проверять, выполнены ли предварительные условия перед вызовом метода, но положительным моментом является то, что возможные ошибки легче отследить.

Если вызов, MediaPlayer.stop()когда он уже остановлен, не причиняет вреда вашему приложению, то я бы позволил ему выполняться тихо, потому что это упрощает код и у меня есть кое-что для идемпотентных методов. Но если вы абсолютно уверены, MediaPlayer.stop()что в таких условиях это никогда не будет вызвано, я бы выдал ошибку, потому что в другом месте кода может быть ошибка, а исключение поможет отследить ее.

Педро Родригес
источник
3
Я не согласен с тем, playToggle()что проще в использовании: для достижения желаемого эффекта (игрок играет или не играет), вам необходимо знать его текущее состояние. Таким образом, если вы получаете пользовательский ввод с просьбой остановить игрока, вам сначала нужно узнать, играет ли игрок в данный момент, а затем переключить его состояние или нет в зависимости от ответа. Это гораздо более обременительно, чем просто вызывать stop()метод и покончить с ним.
cmaster - восстановить монику
Ну, я никогда не говорил, что им было проще пользоваться - я утверждал, что это безопаснее. Кроме того, в вашем примере предполагается, что пользователю будут предоставлены отдельные кнопки остановки и воспроизведения. Если бы это было действительно так, то да, playToggleбыло бы очень громоздким в использовании. Но если бы вместо этого пользователю также была предоставлена ​​кнопка переключения, его playToggleбыло бы проще использовать, чем играть / останавливать.
Педро Родригес