Praca z kolekcjami i Optionalami.

Odpowiedz Nowy wątek
2018-10-10 22:35
1

Cześć załóżmy, że mam następującą sytuację:

Optional<User> user = userService.getUserById(1);

i następnie chcę wziąć wszystkie konta tego usera. Załóżmy, że user ma metodę getTransactions, która zwraca Set<Optional<Transaction>> oraz Transaction posiada metodę Optional<Iban> getIban.

Robię to w następując sposób:

        List<Iban> ibans= user // Optional<User>
                .map(u::getTransactions()) // Optional<Set<Optional<Transaction>>> nie mogę użyć flatMap bo zadziała na Set
                .map(Collection::stream) // Optional<Stream<Optional<Transaction>>>
                .map(str -> str
                        .filter(Optional::isPresent) // Stream<Optional<Transaction>>
                        .map(Optional::get) // Stream<Transaction>
                        .map(Transaction::getIban) // Stream<Optional<Iban>>
                        .filter(Optional::isPresent) // Stream<Optional<Iban>>
                        .map(Optional::get) // Stream<Iban>
                        .collect(Collectors.toSet())
                ).orElse(Collections.emptySet());

Mój kod nie podoba mi się z kilku względów:

  • mapowanie na stram
  • zagnieżdżone przetwarzanie map
  • naprzemienne get i isPresent

Jak można radzić sobie w takich przypadkach?

EDIT:

Tą część mogę oczywiście wyciągnąć do lambdy, ale to nie rozwiązuje większości bolączek tego kodu.

                        .filter(Optional::isPresent) // Stream<Optional<Transaction>>
                        .map(Optional::get) // Stream<Transaction>
                        .map(Transaction::getIban) // Stream<Optional<Iban>>
                        .filter(Optional::isPresent) // Stream<Optional<Iban>>
                        .map(Optional::get) // Stream<Iban>
                        .collect(Collectors.toSet())
edytowany 1x, ostatnio: Crude Monte Carlo, 2018-10-10 23:27

Pozostało 580 znaków

2018-10-10 22:40
0

poczytaj trochę o zasadach czystego kodu i solid bo słabo to wygląda. Klasa jest zbyt sprzężona, zbyt wiele zależności.

Pozostało 580 znaków

2018-10-10 23:31
0
witold12 napisał(a):

poczytaj trochę o zasadach czystego kodu i solid bo słabo to wygląda. Klasa jest zbyt sprzężona, zbyt wiele zależności.

Którą zasadę SOLID łamie ten przykład? Klasa ma jedną zależność user.

Pozostało 580 znaków

2018-10-11 03:28
2

Niestety ale nie ma żadnego fajnego wrappera na to filter(isPresent)+map(get)+collect() w Javie, a szkoda :) Ale zauważ że musisz to robić tylko raz! Bo na optionalach można wykonywać map które zwraca kolejnego optionala. Więc zamiast

.filter(Optional::isPresent) // Stream<Optional<Transaction>>
.map(Optional::get) // Stream<Transaction>
.map(Transaction::getIban) // Stream<Optional<Iban>>
.filter(Optional::isPresent) // Stream<Optional<Iban>>
.map(Optional::get) // Stream<Iban>
.collect(Collectors.toSet())

Można równie dobrze zrobić tylko

.map(o -> o.map(t->t.getIban()))
.filter(Optional::isPresent)
.map(Optional::get)
.collect(Collectors.toSet())

Na PW przyjmuje tylko (ciekawe!) zlecenia. Masz problem? Pisz na forum, nie do mnie.
edytowany 1x, ostatnio: Shalom, 2018-10-11 03:29

Pozostało 580 znaków

2018-10-11 09:34
  1. Nie wiadomo czemu User::getTransactions zwraca Set<Optional<Transaction>>. Set<Transaction> byłoby łatwiej i raczej bez utraty ogólności. W razie gdyby, możesz sobie w user dowalić metodę getValidTransactions. To upraszcza ten kod.
  2. Dalszym uproszczeniem jest rezygnacja z java,util. i Streamów na korzyść biblioteki VAVR. Wtedy (łacznie z punktem 1) ten kod wyglada tak:

import io.vavr.collection.*;
import io.vavr.control.Option;

Set<Iban> ibans = user
                .map(User::getTransactions)
                .map(str ->
                            str.map(tr -> tr.getIban())
                                    .filter(Option::isDefined)
                                    .map(Option::get)
                ).getOrElse(HashSet.empty());

I mam wrażenie, że jeszcze się to da ładniej zapisać. ale dla mnie jest nadal za rano.

EDIT:
A tu kod nieco późniejszy:

  Seq<Iban> ibans = user
                .map(User::getTransactions)
                .flatMap(str ->
                        Option.sequence(str.map(tr -> tr.getIban()))
                ).getOrElse(List.empty());

Bardzo lubie Singletony, dlatego robię po kilka instancji każdego.
edytowany 7x, ostatnio: jarekr000000, 2018-10-11 10:58
Pokaż pozostałe 2 komentarze
Hmm a jak ostatecznie będzie pusty Option i wykona się to OrElse i całość zwróci listę, to przypisze do Set<iban>? - Pinek 2018-10-11 10:59
W tym drugim przypadku nie ma Set, dlatego właśnie jest Seq. Do Set by się nie przypisało. - jarekr000000 2018-10-11 11:05
O kurde ale wiksa, naprawdę ślepy jestem :D - Pinek 2018-10-11 11:07
@jarekr000000: a to nie jest tak, że lepiej używać streamów z vavra żeby nie tworzyć nowej kolekcji po każdym użyciu map/filter itp? - tdudzik 2018-10-11 11:13
@tdudzik: zależy jak duże są te kolekcje. Ja zakładam (pragmatycznie), że autor nie przerabia milionów transakcji i ibanów per user. Tylko pewnie 20-100. - jarekr000000 2018-10-11 11:29

Pozostało 580 znaków

2018-10-11 11:16
0

Tak jak pisali wyżej, ten Set<Optional<transaction>> powoduje brzydotę kodu. Po co ci empty optionale w Secie?

Jeśli chodzi o paskudne .filter(Optional::isPresent).map(Optional::get) to w javie 9 weszła metoda Optional#stream i w czystej javie można sobie poradzić z tematem tak:

userService.getUserById(1) //Optional<User>
    .stream() //Stream<User>
    .map(user::getTransactions) //Stream<Set<Optional<Transaction>>>>
    .flatMap(Collection::stream) //Stream<Optional<Transaction>>>
    .flatMap(Optional::stream) //Stream<Transaction>
    .map(Transaction::getIban) //Stream<Optional<Iban>>
    .flatMap(Optional::stream) //Stream<Iban>
    .collect(Collectors.toSet())

Nie dysponując dziewiątką, generalnie nie ma innego wyjścia. Jak wolisz bez zagnieżdżania, to możesz od razu zestrimować optionala usera:

Stream.of(userService.getUserById(1)) //Stream<Optional<User>>
    .filter(Optional::isPresent) //Stream<Optional<User>>
    .map(Optional::get) //Stream<User>
    .map(user::getTransactions) //Stream<Set<Optional<Transaction>>>>
    .flatMap(Collection::stream) //Stream<Optional<Transaction>>>
    .filter(Optional::isPresent) //Stream<Optional<Transaction>>>
    .map(Optional::get) //Stream<Transaction>
    .map(Transaction::getIban) //Stream<Optional<Iban>>
    .filter(Optional::isPresent) //Stream<Optional<Iban>>
    .map(Optional::get) //Stream<Iban>
.collect(Collectors.toSet())
Czemu nie zamienić Optional<Transaction> na Optional<Iban> przez map na optionalu, zamiast robić kolejny raz filter+get? ;) - Shalom 2018-10-11 11:28
Mea culpa, mea culpa, mea maxima culpa - Tyvrel 2018-10-11 11:37

Pozostało 580 znaków

2018-10-11 12:44
0

W sumie śmiechłem z Set<Optional<whatever>>,jaki to ma sens? Coś jest w Secie albo nie ma.

Tak to jest gdy uczy się z durnych filmików na YT, kursów udemy czy innych bootcampow zamiast korzystać z dokumentacji - witold12 2018-10-11 12:51
Nie mówiąc o tym, jak np. prawdziwych obiektów jest np. 10, a Optionali wsadzonych 10 milionów. - CountZero 2018-10-11 13:17
@witold12: co Ty masz do kursów na Udemy? - scibi92 2018-10-11 20:37

Pozostało 580 znaków

2018-10-11 13:33
1
CountZero napisał(a):

W sumie śmiechłem z Set<Optional<whatever>>,jaki to ma sens? Coś jest w Secie albo nie ma.

Zwróciłem uwagę, że to pewnie jest bez sensu..., ale
Jest jakaś bardzo mała szansa, że np. ktoś chce wiedzieć, że mamy było rozpoczęte 20 transakcji, z czego tylko 15 zakończyło się sukcesem, a reszta jest w jakimś stanie invalid (i np. bedzie wyczyszczona pod koniec dnia). I może ktoś (dla statystyk) chcieć tak to przechowywać ( Set<Optional<Trx>> - see Edit). (Powinno być wtedy: List<Optional<Trx>>)
Aczkolwiek strzelam, że tu raczej jest to wynik nieuwagi i niezamierzona komplikacja.

Edit (samokrytyka po minucie): od razu widać, że to na górze to jednak nonsens.
List<Optional<trx>> miałoby sens. Ale nie Set.


Bardzo lubie Singletony, dlatego robię po kilka instancji każdego.
edytowany 5x, ostatnio: jarekr000000, 2018-10-11 13:36
W sensie, że Optional broniłby przed tym stanem "invalid" który po odwołaniu mógłby rozwalić aplikacje? - CountZero 2018-10-11 15:54
Nie, po prostu w Secie elementy się nie powtarzają, więc będziemy mieli co najwyżej jeden dodatkowy Optional.empty(). Mała użyteczność. - jarekr000000 2018-10-11 20:47
Aha, w tym sensie... No niewielka raczej. - CountZero 2018-10-11 20:57

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