Использование условия finally для выполнения работы после возврата плохой стиль / опасно?

30

Как часть написания Iterator, я обнаружил, что пишу следующий фрагмент кода (обработка ошибок)

public T next() {
  try {
    return next;
  } finally {
    next = fetcher.fetchNext(next);
  }
}

немного легче читать, чем

public T next() {
  T tmp = next;
  next = fetcher.fetchNext(next);
  return tmp;
}

Я знаю, что это простой пример, где разница в удобочитаемости может быть не такой значительной, но меня интересует общее мнение о том, не стоит ли использовать try-finally в таких случаях, когда нет исключений, или если это на самом деле предпочтительнее, когда это упрощает код.

Если это плохо: почему? Стиль, производительность, подводные камни, ...?

Заключение Спасибо вам за все ваши ответы! Я думаю, что вывод (по крайней мере для меня) заключается в том, что первый пример мог бы быть более читабельным, если бы это был общий шаблон, но это не так. Поэтому путаница, вызванная использованием конструкции вне ее предназначения, наряду с, возможно, запутанным потоком исключений, перевесит любые упрощения.

Галле Кнаст
источник
10
Я лично смог бы понять второй больше, чем первый, но я редко использую finallyблоки.
Кристиан Манн
2
return current = fetcher.fetchNext (current); // Как насчет этого, иначе вам действительно нужна предварительная загрузка?
шарфридж
1
@scarfridge Пример о реализации Iterator, где для работы вам действительно нужна какая-то предварительная выборка hasNext(). Попробуй сам.
Maaartinus
1
@maaartinus Я знаю об этом. Иногда hasNext () просто возвращает true, например, последовательность града, а иногда выбор следующего элемента непомерно дорог. В последнем случае следует прибегнуть к извлечению элемента только по требованию, аналогичному отложенной инициализации.
шарфридж
1
@scarfridge Проблема с обычным использованием Iteratorзаключается в том, что вам нужно получить значение в hasNext()(потому что выборка довольно часто - единственный способ выяснить, существует ли оно) и вернуть его так next()же, как это сделал OP.
Maaartinus

Ответы:

18

Лично я бы задумался над идеей разделения командных запросов . Когда вы приступите прямо к этому, у next () есть две цели: объявленная цель получения элемента next и скрытый побочный эффект изменения внутреннего состояния next. Итак, вы выполняете объявленную цель в теле метода, а затем добавляете скрытый побочный эффект в предложение finally, которое кажется ... неловким, хотя и не "неправильным", в точности.

На самом деле все сводится к тому, насколько понятен код, я бы сказал, и в этом случае ответ - «Мех». Вы «пытаетесь» с помощью простого оператора return, а затем выполняете скрытый побочный эффект в блоке кода, который предназначен для безопасного восстановления после ошибок. То, что вы делаете, это «умный», а «умный» код часто побуждает сопровождающих бормотать: «Что ... о, я думаю, я понял». Лучше, чтобы люди, читающие ваш код, бормотали: «Угу, да, имеет смысл, да ...»

Итак, что если вы отделили мутацию состояния от вызовов доступа? Я бы себе вопрос читаемости вы обеспокоены становится спорным, но я не знаю, как это влияет на вашу широкую абстракцию. Во всяком случае, что-то рассмотреть.

Эрик Дитрих
источник
1
+1 за «умный» комментарий. Это очень точно отражает этот пример.
2
Действие next () «return and advance» вызывается для OP громоздким интерфейсом итератора Java.
Кевин Клайн
37

Чисто с точки зрения стиля, я думаю, что эти три строки:

T current = next;
next = fetcher.getNext();
return current;

... и более очевидны, и короче, чем блок try / finally. Поскольку вы не ожидаете каких-либо исключений, использование tryблока просто запутает людей.

StriplingWarrior
источник
2
Блок try / catch / finally может также добавить дополнительные издержки, я не уверен, что javac или JIT-компилятор удалят их, даже если вы не генерируете исключение.
Мартейн Вербург
2
@MartijnVerburg да, javac по-прежнему добавляет запись в таблицу исключений и дублирует код finally, даже если блок try пуст.
Даниэль Любаров
@Daniel - спасибо, я был слишком ленив, чтобы выполнить javap :-)
Martijn Verburg
@Martijn Verburg: AFAIK, блок try-catch-finallt, по сути, является бесплатным до тех пор, пока не выдается фактическое исключение. Здесь javac не пытается и не нуждается в оптимизации, поскольку JIT позаботится об этом.
Maaartinus
@maaartinus - спасибо, что имеет смысл - нужно снова прочитать исходный код OpenJDK :-)
Martijn Verburg
6

Это будет зависеть от назначения кода внутри блока finally. Канонический пример - закрытие потока после чтения / записи из него, своего рода «очистка», которая должна выполняться всегда. Восстановление объекта (в данном случае итератора) в правильное состояние IMHO также считается очисткой, поэтому здесь я не вижу проблем. Если бы OTOH вы использовали, returnкак только было найдено ваше возвращаемое значение, и добавили много несвязанного кода в блок finally, это затенило бы цель и сделало бы ее все менее понятной.

Я не вижу никаких проблем в его использовании, когда «нет никаких исключений». Очень часто используется try...finallyбез, catchкогда код может только выдать, RuntimeExceptionи вы не планируете обрабатывать их. Иногда, наконец, это просто гарантия, и вы знаете по логике вашей программы, что никогда не будет выброшено исключение (классическое условие «этого никогда не должно случиться»).

Подводные камни: любое исключение, возникшее внутри tryблока, заставит finallyБока бежать. Это может привести вас в противоречивое состояние. Итак, если ваше возвращение было что-то вроде:

return preProcess(next);

и этот код вызвал исключение, fetchNextвсе равно будет работать. OTOH, если вы закодировали это как:

T ret = preProcess(next);
next = fetcher.fetchNext(next);
return ret;

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

Производительность: было бы интересно декомпилировать такой код, чтобы увидеть, как он работает под капотом, но я не знаю достаточно о JVM, чтобы даже сделать правильное предположение ... Исключения, как правило, "исключительные", поэтому код, который обрабатывает их не нужно оптимизировать по скорости (отсюда и совет никогда не использовать исключения в нормальном потоке управления вашими программами), но я не знаю об этом finally.

mgibsonbr
источник
Разве вы не предоставляете довольно вескую причину, чтобы не использовать finallyтаким образом? Я имею в виду, как вы говорите, код в конечном итоге имеет нежелательные последствия; еще хуже, вы, вероятно, даже не думаете об исключениях.
Андрес Ф.
2
Нормальная скорость потока управления существенно не зависит от конструкций обработки исключений. Штраф за производительность выплачивается только при фактическом создании и перехвате исключений, а не при входе в блок try или при вводе блока finally в обычной работе.
yfeldblum
1
@AndresF. Правда, но это также справедливо для любого кода очистки. Например, предположим, что ссылка на открытый поток установлена nullошибочным кодом; попытка чтения из него вызовет исключение, попытка закрыть его в finallyблоке вызовет другое исключение. Кроме того, это ситуация, когда состояние вычислений не имеет значения, вы хотите закрыть поток независимо от того, произошло ли исключение. Могут быть и другие подобные ситуации. По этой причине я рассматриваю это как ловушку для правильного использования, а не как рекомендацию никогда не использовать это.
mgibsonbr
Существует также проблема, заключающаяся в том, что когда оба блока try и finally генерируют исключение, исключение в try никогда никем не замечено, но, вероятно, оно является причиной проблемы.
Ханс-Петер Стёрр
3

Заголовок: «... наконец, условие для выполнения работы после возврата ...» является ложным. Блок finally происходит перед возвратом функции. В этом весь смысл, наконец, на самом деле.

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

Предполагаемое использование блоков finally предназначено для обработки исключений, когда должны возникать некоторые последствия независимо от создаваемых исключений, например, очистка какого-либо ресурса, закрытие соединения с БД, закрытие файла / сокета и т. Д.

Ницан Вакарт
источник
+1, я бы добавил, что даже если спецификация языка гарантирует, что значение сохраняется до того, как оно будет возвращено, оно не соответствует ожиданиям читателя и, следовательно, его следует избегать, когда это возможно. Отладка в два раза
сложнее,
0

Я был бы очень осторожен, потому что (в общем, не в вашем примере) часть в try может генерировать исключение, и если оно будет выброшено, ничего не будет возвращено, и если вы действительно намеревались выполнить, наконец, после return, это будет выполнить, когда вы этого не хотели.

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

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

user470365
источник