Czy zagnieżdżone ify to zawsze zło?

Odpowiedz Nowy wątek
2019-05-14 03:34
0

Muszę zaimplementować kilka scenariuszy i nie wiem jaki sposób będzie poprawniejszym - zagnieżdżone ify czy interfejsy funkcyjne?

boolean doSomething(User user, Long jakiesId) { 
        Optional<User> foundedUser = userRepository.findByLogin(user.getLogin());
        if(foundedUser.isPresent()) {
            if(checkEmailsCompatibility(user)) {
                if(!isAlreadyAssigned(user, jakiesId)) {
                      ......
                }else {
                    throw new AlreadyAssignedException(jakiesId);
                }
            }else {
                throw new NotCompatibleEmailAddressesException(user.getLogin());
            }
        }else {
            ............
        }
        return true;
    }

Kod wydawał mi się czytelny do czasu kiedy nie musiałem dla każdego niespełnionego warunku dodać else throw new Exception... No i to if(foundedUser.isPresent()) bardzo śmierdzi.
Nie do końca wiem jaka jest poprawna praktyka rzucania wyjątków. Powinienem to robić już w metodach sprawdzających? W sensie jeśli mam ifWhatever() to dla niespełnionego warunku rzucać Exception czy dopiero poziom wyżej, jak w kodzie który wkleiłem?

Z tymi ifami to jak z tym dowcipem Pana Grabowskiego. dla zainteresowanych

Pozostało 580 znaków

2019-05-14 08:01
8

Ify to ogólnie zło.
Krok1. Użyj funkcji map i getOrElse, aby pozbyć się tego isPresent. Profit.
Krok2. Zamień Optional na vavr/Option. Profit.
Krok3. Pozamieniaj metody check,is... tak aby zwracały Either<Błąd,User>. Płacz, że nic sie nie kompiluje.
Krok4. Na Option zrób toEither, i teraz lecisz flatMapem wszystko. Nie do powstrzymania - jak Kubica.
Krok5. Nie masz żadnego ifa.


Bardzo lubie Singletony, dlatego robię po kilka instancji każdego.
Pokaż pozostałe 3 komentarze
Mądrego to i dobrze posłuchać. - eL 2019-05-14 08:33
@m4ck: @jarekr000000 ten wykład prowadził :) - DamianSn 2019-05-14 10:18
wiem, wiem. tamten komentarz miał mieć formę żartu :D - m4ck 2019-05-14 10:24
No to mnie wkręciłeś, hahaha - DamianSn 2019-05-14 10:41
Aż mi się przypomniała scena z filmu Doktor Strange, gdzie tytułowy bohater spotyka Ancient One i mówi: https://www.youtube.com/watch?v=PnTcG5t3TWw - Madness 2019-05-15 15:46

Pozostało 580 znaków

2019-05-14 08:22
3

Nie lepiej jedna metode walidacyjna? Ktora bedzie skladac sie z tych mniejszych metod? Jakby user mial 10 pol do sprawdzenia to bys 10x sie zagniezdzal? Oczywiscie podejrzewam ze roziwazanie jarker00000 jest najlepsze :D. Ale nie znajac monad ja bym zrobil to tak ze zrobil sobie funkcje validujaca i skladajaca sie z tych podfunckji validujacych ktore by w przypadku validacji sypaly exceptionami. Tak wiec unikneloby sie zagniezdzenia ifow bo bys mial po jednym ifie w kazdej z metod :)

Pokaż pozostałe 2 komentarze
Ale to nie jest trop, a jedynie pytanie OP, czy te if'y to taka tragedia. W sumie to nikt tego oficjalnie w tym wątku nie poparł, a jedynie są podawane sposoby uniknięcia tej konstrukcji. Być może okaże się, że jednak nie jest ona "najgorsza" i w sumie to nawet całkiem OK ;) - cerrato 2019-05-14 09:02
@cerrato javowe (i nie tylko) ify to konstrukcja błedogenna. Jeśli się da tanim kosztem wykpić z ifów to warto. - jarekr000000 2019-05-14 09:08
Zgadzam się z Tobą w pełni. Chciałem jedynie zaznaczyć, że tak właściwie to OP zapytał "czy IF to zawsze zło", ale de facto dyskusja trochę odbiegła od tematu pytania. Nie piszecie teraz o tym, CZY ify są wporzo, a raczej dajecie sposoby, JAK można się ich pozbyć :P - cerrato 2019-05-14 09:13
właściwie to masz rację cerrato, aczkolwiek to ja zle skonstruowałem pytanie. odpowiedzi które dostałem są jak najbardziej celnymi i właśnie takich oczekiwałem :) - m4ck 2019-05-14 09:15
jak tak, to OK i już będę siedzieć cicho ;) - cerrato 2019-05-14 09:17

Pozostało 580 znaków

2019-05-14 08:31
0

o takim rozwiązaniu nie pomyślałem a faktycznie jest bardzo czytelne. jak monady mnie przerosną to na pewno wykorzystam. dzięki! :)

Proponuję nie nazywać monad monadami tylko pojemnikami na wartości i od razu strach znika. :) - Michał Sikora 2019-05-14 08:57
Święte słowa. Mi monada się kojarzy z gonada i przez to zawszę się lekko czerwienię i zawstydzam, kiedy ktoś o nich wspomina. - cerrato 2019-05-14 09:01
@cerrato: hahahah, zwariowana:D - m4ck 2019-05-14 09:07
Najłatwiej to nie przejmować się czy coś jest monadą czy nie, ta wiedza w Javie jest tak de facto zbędna. O wiele bardziej przydatna jest praktyczna umiejętność korzystania czy to z Option czy z Either - DisQ 2019-05-14 09:24
Dodaję do tego, że to takie pojemniki, z których raczej nic nie wyciągamy. Jeśli już to na samym końcu. Maximum: flatMap, map. Minimim: get. Pisząc w ten sposób trudniej popełnić błąd. - jarekr000000 2019-05-14 09:42

Pozostało 580 znaków

2019-05-14 13:14
2

Ify to nie zawsze zło. To normalna konstrukcja. W kodzie powinieneś unikać przede wszystkim "drabiny" ifów (chociaż w pojedynczych przypadkach może to być ok) oraz wielokrotnych zagnieżdżeń (nie ważne, czy to będą ify, czy coś innego - np. callbacki), bo takie rzeczy powodują spadek czytelności kodu i trudność w jego utrzymaniu. U Ciebie to już zaczyna się pojawiać. Można to rozwiązać tak, jak Jarek napisał, przepisując to na kod funkcyjny. Nie wiem, czy zawsze będzie to idealne rozwiązanie. Alternatywą może być przerzucenie całego kodu odpowiedzialnego za walidację w osobne miejsce (np. do oddzielnej metody). Wtedy będziesz miał jednego ifa lub jedno wywołanie metody i w lambdzie możesz sobie przekazać kod, który musisz wykonać.

edytowany 1x, ostatnio: wiciu, 2019-05-14 13:15

Pozostało 580 znaków

2019-05-14 21:18
0

NO PRZECIEŻ TO SIĘ NIE KOMPILUJE!! Dostaję Type mismatch: cannot convert from Either<Object,User> to Either<Exception,User> Próbowałem też z Try<> ale tam to w ogóle tak namieszałem, że potrzebowałem git reset żeby kod działał chociaż trochę.

Either<Exception, User> checkEmailsCompatibility(User user){
        return userRepository.findByLogin(user.getLogin())
                .filter(u -> u.getEmail().equalsIgnoreCase(user.getEmail()))
                .map(u -> Either.right(u)).orElse(Either.left(new NotCompatibleEmailAddressesException(user.getLogin())));
    }
edytowany 1x, ostatnio: m4ck, 2019-05-14 21:20

Pozostało 580 znaków

2019-05-14 21:32
1

Java. type inference. Fail.
Zamień
.map(u -> Either.right(u)).
na
.map(u -> Either.<Exception,User>right(u)).

Przy okazji:

Either<Exception,T> to w zsadzie Try<T>.
Ale przeważnie lepszy jest
Either<E,T> gdzie E to twoja klasa, która NIE dziedziczy z Exceptiona. Bo wtedy nie musi potencjalnie pamiętać StackTrace - profit.
Na grzyba Ci stack trace - jak to wcale nie jest błąd programu, tylko zwykły problem błędnych danych wejściowych.


Bardzo lubie Singletony, dlatego robię po kilka instancji każdego.
edytowany 2x, ostatnio: jarekr000000, 2019-05-14 21:34

Pozostało 580 znaków

2019-05-14 21:37
0

... nie wierzę. dzięki!

neo na poczatku tez nie wierzyl - filemonczyk 2019-05-14 22:50

Pozostało 580 znaków

2019-05-15 00:00
V-2
8

Nawet przy skromniejszej refaktorce, zakładającej zachowanie starych, dobrych ifów - wystarczyłoby je poodwracać, żeby zredukować zagnieżdżenie.

Rzucenie wyjątku przerywa bieg działania metody, Nie ma zatem potrzeby dodawać po nim jeszcze else.

W miejsce:

if (checkEmailsCompatibility(user)) {
    if (!isAlreadyAssigned(user, jakiesId)) {
        ......
    } else {
        throw new AlreadyAssignedException(jakiesId);
    }
} else {
    throw new NotCompatibleEmailAddressesException(user.getLogin());
}

Wystarczyłoby więc zrobić:

if (!checkEmailsCompatibility(user)) {
    throw new NotCompatibleEmailAddressesException(user.getLogin());
}
if (isAlreadyAssigned(user, jakiesId)) {
    throw new AlreadyAssignedException(jakiesId);
}
......

Fail fast. I już nie ma zbędnych zagnieżdżeń. Jest za to czytelniej. Czy czytelniej niż z konstrukcją typu:

.filter(u -> u.getEmail().equalsIgnoreCase(user.getEmail()))
.map(u -> Either.right(u)).orElse(Either.left(new NotCompatibleEmailAddressesException(user.getLogin())))

itp., to już temat na osobne rozważania ;)


Nie ma najmniejszego powodu, aby w CV pisać "email" przed swoim adresem mailowym, "imię i nazwisko" przed imieniem i nazwiskiem" ani "zdjęcie mojej głowy od przedniej strony" obok ewentualnego zdjęcia.
edytowany 2x, ostatnio: V-2, 2019-05-15 00:03
Pomijając zbędne używanie exceptionów tutaj, to dla mnie te ify są czytelniejsze. Ale może to jeszcze kwestia przyzwyczajenia. - kkojot 2019-05-15 07:38
Zgadzam się co do wyjątków - ale równie dobrze mogłoby być zamiast nich zwracanie jakiejś wartości reprezentującej (negatywny) rezultat walidacji, bez zmiany dla struktury. Boreturn tak samo "wysiada" z metody. @kkojot - V-2 2019-05-15 09:54
Co do czytelności, styl functional vs. imperatywny to oczywiście kwestia przyzwyczajenia, ale dochodzi tu problem taki, że Javy nie zaprojektowano z myślą o functional programming. Stosowanie tego stylu w Javie często jest, moim zdaniem, siodłaniem krowy i prowadzi do potworków budzących mieszane uczucia estetyczne. Dobrze wiedzieć, że można - ja bym tu tego nie robił. CC @jarekr000000 - V-2 2019-05-15 10:05
Zupełnie się zgadzam, że styl funkcyjny w javie często słabo wygląda. I może być mniej czytelny. Za to jest dużo bezpieczniejszy (expressions vs statements). Musieć tak wybierać jest mega kijowo. Często sie ten kod funkcyjny daje uładnić, tylko trzeba więcej wysiłku niż w stmts włożyć. - jarekr000000 2019-05-15 10:25

Pozostało 580 znaków

2019-05-15 08:05
0
wiciu napisał(a):

W kodzie powinieneś unikać przede wszystkim "drabiny" ifów (...) oraz wielokrotnych zagnieżdżeń...

Dodam jeszcze, żeby unikać ifów na więcej niż kilka linijek.
U mnie czasem piszą ify na 2 ekrany. ;(

Pozostało 580 znaków

2019-05-15 08:09
1

Zasadniczne pytanie w całej tej historii: po co te ify? Należy programować w taki sposób aby nie było możliwe wywołanie doSomething w kontekście gdy isAlreadyAssigned zwróci true.

Pozostało 580 znaków

Odpowiedz
Liczba odpowiedzi na stronę

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0