Dlaczego Optional.get () bez wywołania isPresent () jest zły, ale nie iterator.next ()?

23

Korzystając z nowego interfejsu API strumieni Java8 w celu znalezienia jednego konkretnego elementu w kolekcji, piszę taki kod:

    String theFirstString = myCollection.stream()
            .findFirst()
            .get();

Tutaj IntelliJ ostrzega, że ​​wywoływana jest metoda get () bez uprzedniego sprawdzenia isPresent ().

Jednak ten kod:

    String theFirstString = myCollection.iterator().next();

... nie daje ostrzeżenia.

Czy istnieje jakaś głęboka różnica między tymi dwiema technikami, która sprawia, że ​​podejście do przesyłania strumieniowego jest w pewnym sensie „niebezpieczne”, dlatego tak ważne jest, aby nigdy nie wywoływać get () bez wywołania najpierw isPresent ()? Przeglądając go, znajduję artykuły mówiące o tym, jak programiści są nieostrożni z Opcjonalnym <> i zakładam, że mogą wywoływać get () za każdym razem.

Chodzi mi o to, że lubię rzucać na mnie wyjątkami jak najbliżej miejsca, w którym mój kod jest błędny, co w tym przypadku byłoby wywołaniem get (), gdy nie ma elementu do znalezienia. Nie chcę pisać bezużytecznych esejów, takich jak:

    Optional<String> firstStream = myCollection.stream()
            .findFirst();
    if (!firstStream.isPresent()) {
        throw new IllegalStateException("There is no first string even though I totally expected there to be! <sarcasm>This exception is much more useful than NoSuchElementException</sarcasm>");
    }
    String theFirstString = firstStream.get();

... chyba że istnieje jakieś niebezpieczeństwo w prowokowaniu wyjątków od get (), których nie jestem świadomy?


Po przeczytaniu odpowiedzi Karla Bielefeldta i komentarza Hulka, zdałem sobie sprawę, że mój powyższy kod wyjątku był nieco niezdarny, oto coś lepszego:

    String errorString = "There is no first string even though I totally expected there to be! <sarcasm>This exception is much more useful than NoSuchElementException</sarcasm>";
    String firstString = myCollection.stream()
            .findFirst()
            .orElseThrow(() -> new IllegalStateException(errorString));

Wygląda to bardziej przydatne i prawdopodobnie stanie się naturalne w wielu miejscach. Nadal mam wrażenie, że będę mógł wybrać jeden element z listy bez konieczności zajmowania się tym wszystkim, ale może to być po prostu konieczność przyzwyczajenia się do tego rodzaju konstrukcji.

Telewizor Frank
źródło
7
Część poświęconą wyjątkom z twojego ostatniego przykładu można by napisać o wiele bardziej zwięźle, jeśli użyjesz orElseThrow - i byłoby jasne dla wszystkich czytelników, że zamierzasz zgłosić wyjątek.
Hulk

Odpowiedzi:

12

Po pierwsze, należy wziąć pod uwagę, że kontrola jest złożona i prawdopodobnie zajmuje stosunkowo dużo czasu. Wymaga to analizy statycznej i między tymi dwoma wywołaniami może być sporo kodu.

Po drugie, idiomatyczne użycie tych dwóch konstrukcji jest bardzo różne. Programuję na pełny etat w Scali, która używa Optionsznacznie częściej niż Java. Nie mogę sobie przypomnieć ani jednego przypadku, gdzie musiałem użyć .get()na zasadzie Option. Prawie zawsze powinieneś powracać Optionalz funkcji lub używać orElse()zamiast niej. Są to znacznie lepsze sposoby obsługi brakujących wartości niż zgłaszanie wyjątków.

Ponieważ .get()rzadko powinno się go wywoływać w normalnym użyciu, sensowne jest, aby IDE rozpoczęło złożoną kontrolę, gdy zobaczy, .get()czy jest prawidłowo używane. Natomiast iterator.next()właściwe i wszechobecne jest użycie iteratora.

Innymi słowy, nie chodzi o to, że jedno jest złe, a drugie nie, chodzi o to, że jeden jest częstym przypadkiem, który ma fundamentalne znaczenie dla jego działania i jest wysoce prawdopodobne, że rzuci wyjątek podczas testowania programisty, a drugi jest rzadko używany przypadek, który może nie być wystarczający do rzucenia wyjątku podczas testowania programisty. To są przypadki, o których ostrzegają IDE.

Karl Bielefeldt
źródło
Być może muszę dowiedzieć się więcej o tej Opcjonalnej firmie - głównie widziałem rzeczy związane z java8 jako dobry sposób na mapowanie list obiektów, które często robimy w naszej aplikacji internetowej. Więc powinieneś wysyłać wszędzie opcjonalne <> s? To wymaga trochę przyzwyczajenia się, ale to kolejny post na SE! :)
Frank w telewizji
@ TV's Frank To mniej, że powinny być wszędzie, a bardziej, że pozwalają ci pisać funkcje, które przekształcają wartość zawartego typu, i łączysz je za pomocą maplub flatMap. Optionalto przede wszystkim świetny sposób na uniknięcie konieczności wiązania wszystkiego nullczekami.
Morgen
15

Klasa Opcjonalna jest przeznaczona do użycia, gdy nie wiadomo, czy element, który zawiera, jest obecny. Istnieje ostrzeżenie, aby poinformować programistów, że istnieje dodatkowa możliwa ścieżka kodu, którą mogą być w stanie z łatwością obsługiwać. Chodzi o to, aby w ogóle uniknąć wyjątków. Przedstawiony przypadek użycia nie jest przeznaczony do tego, dlatego jest on dla ciebie nieistotny - jeśli naprawdę chcesz zgłosić wyjątek, śmiało i wyłącz go (chociaż tylko dla tej linii, a nie globalnie - powinieneś podejmowanie decyzji, czy obsłużyć nie obecną sprawę na zasadzie instancja po instancji, a ostrzeżenie przypomni ci o tym.

Prawdopodobnie należy wygenerować podobne ostrzeżenie dla iteratorów. Jednak po prostu nigdy tego nie zrobiono, a dodanie jednego teraz prawdopodobnie spowodowałoby, że zbyt wiele ostrzeżeń w istniejących projektach byłoby dobrym pomysłem.

Zauważ też, że zgłoszenie własnego wyjątku może być bardziej przydatne niż tylko wyjątek domyślny, ponieważ zawiera opis tego, jaki element powinien istnieć, ale nie może być bardzo pomocny w debugowaniu w późniejszym czasie.

Jules
źródło
6

Zgadzam się, że nie ma w tym żadnej różnicy, jednak chciałbym zwrócić uwagę na większą wadę.

W obu przypadkach masz typ, który zapewnia metodę, która nie gwarantuje, że zadziała i wcześniej powinieneś wywołać inną metodę. To, czy Twój IDE / kompilator / etc ostrzega cię o jednej sprawie, czy innej, zależy tylko od tego, czy obsługuje tę sprawę i / lub czy masz skonfigurowaną do tego celu.

Mimo to oba fragmenty kodu są zapachami kodu. Wyrażenia te wskazują na podstawowe wady projektowe i dają kruchy kod.

Oczywiście masz rację, że chcesz szybko ponieść porażkę, ale rozwiązaniem dla mnie przynajmniej nie może być po prostu zlekceważenie go i wyrzucenie rąk w powietrze (lub wyjątek na stos).

Wiadomo, że twoja kolekcja nie jest na razie pusta, ale możesz naprawdę polegać na jej typie: Collection<T>- i to nie gwarantuje ci braku pustki. Nawet jeśli wiesz, że teraz jest niepusty, zmienione wymagania mogą prowadzić do zmiany kodu z góry i nagle twój kod zostaje dotknięty pustą kolekcją.

Teraz możesz odpychać się i mówić innym, że miałeś otrzymać niepustą listę. Możesz być szczęśliwy, że szybko znajdziesz ten „błąd”. Niemniej jednak masz zepsute oprogramowanie i musisz zmienić kod.

Porównaj to z bardziej pragmatycznym podejściem: ponieważ otrzymujesz kolekcję, która może być pusta, zmuszasz się do zajęcia się tą sprawą, używając Opcjonalnej # mapy, #lubElse lub dowolnej z tych metod, z wyjątkiem #get.

Kompilator działa dla ciebie tak dużo, jak to możliwe, dzięki czemu możesz polegać w jak największym stopniu na umowie typu statycznego o uzyskanie Collection<T>. Kiedy Twój kod nigdy nie narusza tej umowy (na przykład przez jeden z tych dwóch śmierdzących łańcuchów wywołań), Twój kod jest znacznie mniej kruchy w przypadku zmieniających się warunków otoczenia.

W powyższym scenariuszu zmiana powodująca pustą kolekcję danych wejściowych nie miałaby żadnego znaczenia dla Twojego kodu. To tylko kolejne prawidłowe dane wejściowe, a zatem nadal obsługiwane poprawnie.

Kosztem tego jest to, że musisz zainwestować czas w lepsze projekty, w przeciwieństwie do a) ryzykowania łamliwego kodu lub b) niepotrzebnego komplikowania rzeczy przez rzucanie wyjątków.

Na koniec: ponieważ nie wszystkie IDE / kompilatory ostrzegają o wywołaniach iterator (). Next () lub opcjonalnych.get () bez uprzedniej kontroli, taki kod jest na ogół oznaczany w recenzjach. Za to, co jest warte, nie pozwoliłbym, aby taki kod pozwalał na przejrzenie recenzji i zamiast tego wymagał czystego rozwiązania.

Szczery
źródło
Chyba mogę się trochę zgodzić co do zapachu kodu - podałem powyższy przykład, aby napisać krótki post. Innym bardziej uzasadnionym przypadkiem może być wykreślenie obowiązkowego elementu z listy, co nie jest dziwną rzeczą w aplikacji IMO.
Telewizja Frank
Spot on. „Wybierz element z właściwością p z listy” -> list.stream (). Filter (p) .findFirst () .. i jeszcze raz jesteśmy zmuszeni zadać pytanie „a jeśli go nie ma?” .. codzienny chleb i masło. Jeśli jest to obowiązkowe i „właśnie tam” - dlaczego ukryłeś je w wielu innych rzeczach?
Frank
4
Ponieważ może to lista ładowana z bazy danych. Może lista jest zbiorem statystycznym, który został zebrany w innej funkcji: wiemy, że mamy obiekty A, B i C, chcę znaleźć dowolną liczbę dla B. Wiele ważnych (i w mojej aplikacji, często) przypadków.
Telewizja Frank
Nie wiem o twojej bazie danych, ale moja nie gwarantuje co najmniej jednego wyniku na zapytanie. To samo dotyczy zbiorów statystycznych - kto ma powiedzieć, że zawsze będą puste? Zgadzam się, że łatwo jest uzasadnić, że powinien istnieć ten lub inny element, ale wolę odpowiednie pisanie statyczne niż dokumentację lub, co gorsza, pewne spostrzeżenia z dyskusji.
Frank