getResultList() z JPA i Optional

getResultList() z JPA i Optional
GC
  • Rejestracja:ponad 11 lat
  • Ostatnio:ponad 6 lat
  • Postów:249
0

Witam,
Mam metodę, która odnajduje użytkownika w bazie danych po adresie email. Każdy użytkownik ma unikalny adres email.
getSingleResult() - zwraca jeden obiekt, jeśli nie ma rzuca wyjątkiem
getResultList() - zwraca listę (w moim przypadku z jedną osobą), jeśli nie ma użytkownika zwraca pustą listę, nie rzuca wyjątkiem.

Pytanie czy użycie Optional tutaj jest poprawne lub jak go się powinno użyć?

Kopiuj
    public Optional<Person> findByEmail(String email){
        TypedQuery query = entityManager.createQuery("select p from Person p where p.email=:email", Person.class);
        query.setParameter("email", email);
        List result = query.getResultList();
        if(!result.isEmpty()){
            return Optional.ofNullable((Person) result.get(0));
        }
        return null;
    } 
jarekr000000
  • Rejestracja:ponad 8 lat
  • Ostatnio:dzień
  • Lokalizacja:U krasnoludów - pod górą
  • Postów:4708
3

Dobrze, że pytasz bo masz w tym kodzie prawie maximum WTF.
Zaraz wyjaśniam.

Po pierwsze nigdy nie zwracaj z metody null. return null; - zabronione!
1aoc6w.jpg

Za return null grozi kara w postaci obowiązkowego programowania w JavaSkrypcie do lat 2.

Po to jest między innymi jest **optional **żeby **nulla **nie używać.

Ale jak metoda deklaruje, że zwraca optional, a mimo to daje **null **to już za to jest najwyższy wymiar kary: automatyczny zakaz kodowania i zostajesz project managerem - koniec dobrze zapowiadającej się kariery.

Jak powinieneś to napisać? hmmm...
Najkrócej możesz tak (robię to z pamięci - nieczęsto pisze w standardowej javie z jpa- więc mogę się pomylić).

Kopiuj
 
public Optional<Person> findByEmail(String email){
        TypedQuery<Person> query = entityManager.createQuery("select p from Person p where p.email=:email", Person.class);
        query.setParameter("email", email);
        query.setMaxResults(1);     
        return query.getResultList().stream().findFirst(); // armata na wróbla ale wygląda git
    } 

Kilka drobnych uwag:

Kopiuj
Optional.ofNullable((Person) result.get(0));

NIe ma sensu - Optional.ofNullable - używasz do opakowania czegoś co może być nullem (np. stary kod, i powinno to się zdarzać rzadko).
Normalnie tylko Optional.of.

a result.get(0) - nie będzie nigdy nullem (albo lista pusta i nawet do tego miejsca w kodzie nie dojdziesz, albo dostaniesz obiekt i wtedy Optional.of(sth)).

TypedQuery query - powinno być TypedQuery<Person> query. Wtedy masz później listę z typem i nie musisz rzutować tak jak tu:(Person) result.get(0).

A poprawne użycie Optional to dopiero poznasz w metodzie, która to wywołuje.
**if ( findByEmail("blabla").isPresent()) ** jest często niepoprawne.

I tu dedykuje przeróbkę wierszyka (jest przecie weekend):

Kopiuj
 
Ifów mniej oczom lżej, 
cieszy się hackerzyk,
return wrócił, empty rzucił,
ku monadom bieży.

jeden i pół terabajta powinno wystarczyć każdemu
edytowany 13x, ostatnio: jarekr000000
GC
Jeszcze jedno pytanie, skoro to if ( findByEmail("blabla").isPresent()) jest często niepoprawne to jak mam inaczej sprawdzać?
jarekr000000
@gcmarcin to zależy co potrzebujesz, ale generalnie warto jest korzystać z trzech metod optionala, map(...), flatMap(..) i ifPresent(...) (ostatnie ma IF a nie IS). Mam link do PDF po angielsku do tego: http://qconsp.com/sp2014/system/files/presentation-slides/MonadicJava-MarioFusco.pdf ale możesz też wkleić co chcesz zrobić w javie i może się uda coś wymyśleć. Na jednym ifie niewiele widać, ale podejście monadyczne bardzo się sprawdza jak masz serię operacji (w PDF jest przykład) - możesz wtedy oszczędzić dużo kodu i uniknąć potencjalnych błedów.
GC
  • Rejestracja:ponad 11 lat
  • Ostatnio:ponad 6 lat
  • Postów:249
0

Ten fragment chyba pokazuje co chce. Chodź za dobrze to nie jest, bo równie dobrze w ten sam sposób mógłbym napisać różne od null...
@jarekr000000

Kopiuj
    @GetMapping("/user/{email}")
    public ResponseEntity<?> findUser(@PathVariable String email){
        Optional<Person> byEmail = personService.findByEmail(email);
        if(byEmail.isPresent())
            return ResponseEntity.ok(byEmail.get());
        return new ResponseEntity<Object>(HttpStatus.NO_CONTENT);//jeśli nie  ma
    }
 
edytowany 1x, ostatnio: gcmarcin
jarekr000000
  • Rejestracja:ponad 8 lat
  • Ostatnio:dzień
  • Lokalizacja:U krasnoludów - pod górą
  • Postów:4708
2

Czyli możesz to zapisać tak:

Kopiuj
    @GetMapping("/user/{email}")
    public ResponseEntity<?> findUser(@PathVariable String email){
        return  personService.findByEmail(email)
             .map( ResponseEntity::ok)
             .orElse(HttpStatus.NO_CONTENT);
    }
 

jeden i pół terabajta powinno wystarczyć każdemu
edytowany 1x, ostatnio: jarekr000000
GC
Dzięki wielkie, muszę poczytać o tym jak to działa.

Zarejestruj się i dołącz do największej społeczności programistów w Polsce.

Otrzymaj wsparcie, dziel się wiedzą i rozwijaj swoje umiejętności z najlepszymi.