Praca z kolekcjami i Optionalami.

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())
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.

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.

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())
3
  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());
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())
0

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

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.

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