Czy użycie klauzuli wreszcie do wykonywania pracy po powrocie jest w złym stylu / niebezpieczne?

30

W ramach pisania iteratora znalazłem, że piszę następujący fragment kodu (obsługa usuwania błędów)

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

znajdowanie go jest nieco łatwiejsze do odczytania niż

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

Wiem, że jest to prosty przykład, w którym różnica w czytelności może nie być tak przytłaczająca, ale interesuje mnie ogólna opinia na temat tego, czy źle jest używać try-wreszcie w przypadkach takich jak ten, w których nie występują wyjątki, lub jeśli jest to rzeczywiście preferowane, gdy upraszcza kod.

Jeśli to źle: dlaczego? Styl, wydajność, pułapki, ...?

Wniosek Dziękujemy wszystkim za odpowiedzi! Wydaje mi się, że wniosek (przynajmniej dla mnie) jest taki, że pierwszy przykład byłby bardziej czytelny, gdyby był powszechnym wzorcem, ale tak nie jest. Dlatego zamieszanie wprowadzone przez użycie konstruktu poza jego przeznaczeniem, wraz z prawdopodobnie zaciemnionym przepływem wyjątków, przeważy nad wszelkimi uproszczeniami.

Halle Knast
źródło
10
Osobiście byłbym w stanie zrozumieć drugi bardziej niż pierwszy, ale rzadko używam finallybloków.
Christian Mann,
2
return current = fetcher.fetchNext (current); // A co powiesz na to, czy naprawdę potrzebujesz pobierania z góry?
szalik
1
@ scarfridge Przykład dotyczy implementacji Iterator, gdzie rzeczywiście potrzebujesz pewnego rodzaju pobierania wstępnego, hasNext()aby działać. Spróbuj sam.
maaartinus
1
@maaartinus Jestem tego świadomy. Czasami hasNext () po prostu zwraca true, np. Sekwencja gradowa, a czasem pobranie następnego elementu jest zbyt drogie. W tym drugim przypadku powinieneś pobierać element tylko na żądanie, podobnie jak leniwa inicjalizacja.
szalik
1
@ scarfridge Problem z powszechnym użyciem Iteratorpolega na tym, że musisz pobrać wartość do hasNext()(bo pobieranie jest dość często jedynym sposobem, aby dowiedzieć się, czy istnieje) i zwrócić ją next()tak, jak zrobił to OP.
maaartinus

Odpowiedzi:

18

Osobiście zastanowiłbym się nad pomysłem rozdzielenia zapytań poleceń . Kiedy przejdziemy do sedna, next () ma dwa cele: reklamowany cel odzyskania elementu obok i ukryty efekt uboczny mutacji wewnętrznego stanu następnego. Więc to, co robisz, to realizacja reklamowanego celu w treści metody, a następnie zajęcie się ukrytym efektem ubocznym w klauzuli wreszcie, która wydaje się ... niezręczna, ale nie „błędna”.

To, co tak naprawdę sprowadza się do tego, to zrozumiałość kodu, powiedziałbym, w tym przypadku odpowiedź brzmi „meh”. „Próbujesz” prostej instrukcji return, a następnie wywołujesz ukryty efekt uboczny w bloku kodu, który powinien służyć do bezpiecznego usuwania błędów po awarii. To, co robisz, jest „sprytne”, a „sprytny” kod często skłania opiekunów do bełkotania „co ... och, myślę, że to rozumiem”. Lepiej dla osób czytających Twój kod, aby mamrotać „tak, uh-huh, ma sens, tak ...

A co, jeśli oddzielisz mutację stanu od wywołań akcesora? Wyobrażam sobie, że kwestia czytelności, o którą się martwisz, staje się dyskusyjna, ale nie wiem, jak wpływa to na twoją szerszą abstrakcję. W każdym razie coś do rozważenia.

Erik Dietrich
źródło
1
+1 za „sprytny” komentarz. To bardzo dokładnie oddaje ten przykład.
2
Działanie „return and forward” funkcji next () jest wymuszane na OP przez nieporęczny interfejs iteratora Java.
kevin cline
37

Sądzę, że wyłącznie z punktu widzenia stylu te trzy linie:

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

... są zarówno bardziej oczywiste, jak i krótsze niż blok try / wreszcie. Ponieważ nie spodziewasz się żadnych wyjątków, użycie tryklocka po prostu wprowadzi ludzi w błąd.

StriplingWarrior
źródło
2
Blok try / catch / wreszcie może również dodać dodatkowy narzut, nie jestem pewien, czy javac lub kompilator JIT usuną je, nawet jeśli nie zgłaszasz wyjątku.
Martijn Verburg
2
@MartijnVerburg tak, javac nadal dodaje wpis do tabeli wyjątków i duplikuje ostatecznie kod, nawet jeśli blok try jest pusty.
Daniel Lubarov
@Daniel - dzięki, że byłem zbyt leniwy, aby wykonać javap :-)
Martijn Verburg
@Martijn Verburg: AFAIK, blok try-catch-finallt jest zasadniczo bezpłatny, o ile nie zostanie zgłoszony żaden wyjątek. Tutaj javac nie próbuje i nie musi niczego optymalizować, ponieważ zajmie się tym JIT.
maaartinus
@maaartinus - dzięki, że to ma sens - trzeba ponownie przeczytać kod źródłowy OpenJDK :-)
Martijn Verburg
6

Zależy to od celu kodu w bloku wreszcie. Kanonicznym przykładem jest zamknięcie strumienia po jego odczytaniu / zapisaniu, pewnego rodzaju „czyszczenie”, które zawsze musi być wykonane. Przywracanie obiektu (w tym przypadku iteratora) do prawidłowego stanu IMHO również liczy się jako czyszczenie, więc nie widzę tutaj żadnego problemu. Jeśli użyjesz OTOH, returngdy tylko zostanie zwrócona wartość zwrotna, i dodasz dużo niepowiązanego kodu w bloku ostatecznie, to zaciemni to cel i sprawi, że wszystko będzie mniej zrozumiałe.

Nie widzę żadnego problemu w korzystaniu z niego, gdy „nie ma żadnych wyjątków”. Bardzo często używa się go try...finallybez catchchwili, gdy kod może tylko wrzucić, RuntimeExceptiona Ty nie planujesz obsługiwać go. Czasami w końcu jest tylko zabezpieczeniem i wiesz dla logiki swojego programu, że żaden wyjątek nigdy nie zostanie zgłoszony (klasyczny warunek „nigdy nie powinno się zdarzyć”).

Pułapki: każdy wyjątek zgłoszony w trybloku spowoduje finallyuruchomienie bocka. To może wprowadzić cię w niespójny stan. Tak więc, jeśli zwrot byłby podobny do:

return preProcess(next);

i ten kod zgłosił wyjątek, fetchNextnadal działał. OTOH, jeśli kodujesz go w następujący sposób:

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

to by się nie uruchomiło. Wiem, że zakładasz, że kod try nigdy nie podniesie żadnego wyjątku, ale w przypadku spraw bardziej złożonych niż to, skąd możesz być pewien? Jeśli twój iterator byłby obiektem długowiecznym, istniałby on nadal, nawet jeśli w bieżącym wątku wystąpiłby nieodwracalny błąd, ważne jest, aby utrzymywać go przez cały czas w poprawnym stanie. W przeciwnym razie nie ma to większego znaczenia ...

Wydajność: interesujące byłoby zdekompilowanie takiego kodu, aby zobaczyć, jak działa on pod maską, ale nie znam wystarczająco dużo JVM, aby nawet zgadywać ... Wyjątki są zwykle „wyjątkowe”, więc kod, który obsługuje nie trzeba ich optymalizować pod kątem szybkości (stąd rada, aby nigdy nie używać wyjątków w normalnym przepływie kontroli programów), ale nie wiem finally.

mgibsonbr
źródło
Czy to nie jest naprawdę dobry powód, aby unikać używania finallyw ten sposób? Mam na myśli, jak mówisz, kod kończy się niechcianymi konsekwencjami; co gorsza, prawdopodobnie nawet nie myślisz o wyjątkach.
Andres F.
2
Konstrukcje obsługujące wyjątki nie mają istotnego wpływu na normalną kontrolną prędkość przepływu. Kara za wydajność jest wypłacana tylko wtedy, gdy faktycznie rzuca się i łapie wyjątki, a nie przy wejściu do bloku try lub wejściu do bloku ostatecznie podczas normalnej pracy.
yfeldblum
1
@AndresF. To prawda, ale dotyczy to również każdego kodu czyszczenia. Załóżmy na przykład, że odwołanie do otwartego strumienia jest ustawione nullprzez błędny kod; próba odczytania z niego spowoduje zgłoszenie wyjątku, próba zamknięcia go w finallybloku spowoduje zgłoszenie kolejnego wyjątku. Poza tym jest to sytuacja, w której stan obliczeń nie ma znaczenia, chcesz zamknąć strumień niezależnie od tego, czy wystąpił wyjątek. Mogą być też inne sytuacje tego typu. Z tego powodu traktuję to jako pułapkę dla ważnych zastosowań, a nie zalecenie, aby nigdy jej nie używać.
mgibsonbr
Istnieje również problem polegający na tym, że gdy zarówno try, jak i blok ostatecznie zgłoszą wyjątek, wyjątek podczas próby nigdy nie jest widoczny dla nikogo, ale prawdopodobnie jest to przyczyną problemu.
Hans-Peter Störr
3

Tytuł: „... wreszcie klauzula dotycząca wykonywania pracy po powrocie ...” jest fałszywy. Ostatecznie blok następuje przed powrotem funkcji. To właśnie o to w końcu chodzi.

To, co nadużywasz tutaj, to kolejność oceny, w której wartość next jest przechowywana do zwrócenia przed mutacją. Nie jest to powszechna praktyka, a moim zdaniem błędna, ponieważ powoduje, że kodujesz niesekwencyjny, a zatem znacznie trudniej go przestrzegać.

Zamierzone zastosowanie bloków w końcu służy do obsługi wyjątków, w których muszą wystąpić pewne konsekwencje, niezależnie od zgłaszanych wyjątków, np. Czyszczenia niektórych zasobów, zamykania połączenia DB, zamykania pliku / gniazda itp.

Nitsan Wakart
źródło
+1, dodałbym, że nawet jeśli specyfikacja języka gwarantuje, że wartość jest przechowywana przed jej zwróceniem, to żałośnie stoi w sprzeczności z oczekiwaniami czytelnika, a zatem należy tego unikać, gdy jest to racjonalnie możliwe. Debugowanie jest dwa razy trudniejsze niż kodowanie ...
jmoreno
0

Byłbym bardzo ostrożny, ponieważ (ogólnie rzecz biorąc, nie w twoim przykładzie) część w try może rzucić wyjątek, a jeśli zostanie wyrzucony, nic nie zostanie zwrócone, a jeśli rzeczywiście zamierzasz wykonać to, co jest w końcu po powrocie, to zrobi to wykonaj, gdy nie chcesz.

Inną puszką robaka jest, ogólnie rzecz biorąc, pożyteczne działanie, które w końcu może generować wyjątki, dzięki czemu można zakończyć naprawdę niechlujną metodą.

Z tego powodu ktoś, kto to czyta, musi pomyśleć o tych problemach, więc trudniej to zrozumieć.

użytkownik470365
źródło