Użycie vavr przy pisaniu CRUD w springu

Użycie vavr przy pisaniu CRUD w springu
M1
  • Rejestracja:ponad 10 lat
  • Ostatnio:ponad 4 lata
  • Postów:20
0

Hej, piszę aplikację w której użytkownik będzie mógł się zapisać na spotkania. Najpierw napisałem to z wykorzystaniem wyjątków i wyglądało to mniej więcej tak:

Kopiuj
@PostMapping("book")
@PreAuthorize("hasRole('STUDENT')")

public ResponseEntity<?> bookMeeting(Principal principal, @Valid @NotBlank @RequestParam(value="id") Long id)
{
User user = userService.findByUsername(principal.getName()); 
if(user== null) throw UserNotFoundException("no such user") // to w ogole jest mozliwe?

Meeting meeting = meetingService.findById(id);
if(meeting == null) throw MeetingNotFound

return  meetingService.bookMeeting(meeting,user);
}

W meetingService.bookMeeting() sprawdzałem czy meeting.getUser() == null (czyli czy spotkanie nie jest już zarezerwowane) i tez rzucałem jakiś wyjątek na koniec jak wszystko było OK to zwracałem jakieś Response, w wyjątkach miałem jakiś @ResponseStatus i jak coś poszło nie tak to zwracało jakąś wiadomość.

Po obejrzeniu paru filmików p. Jarka uznałem, że nie ma sensu rzucać wyjątków gdy nie występują jakieś skrajne sytuację.
teraz ciało bookMeeting wyglądałoby tak:

Kopiuj
Option<User> user = userService.findByUsername(principal.getName());
Option<Meeting> meeting = meetingService.findById(id);

Either<Error, Meeting>  book = meetingService.bookMeeting(meeting,user);
if(book.isRight()) return ResponseEntity.ok(book.getRight());
else return ResponseEntity.badRequest(book.getLeft().getMessage());

więc bookMeeting powinno wyglądać mniej więcej tak:

Kopiuj
if(user.empty()) return Either.Left(new Error("no such user"));
if(meeting.empty()) return Either.Left(new Error("no such meeting"));
if(meeting.get().getUser().empty()) return Either.Left(new Error("meeting already booked"));

... jakies operacje typu meeting.setUser(user), zapisz do bazy danych

return Either.Right(...);

Kod wydaję się być minimalnie lepszy pozbyłem się tego skakania do wyjątków ale chyba dalej jest to słabe przez to sprawdzanie czy kontener jest pusty. Próbowałem to napisać z map(), zrobić takie map().map().map() jak coś poszło nie tak to odpowiedni Error ale średnio mi to wychodziło, nie wiem jak w środku użyc Meeting::setUser z argumentem, poprzednio sprawdzając wszystko czy istnieje i jeżeli coś nie istnieje to zwrócic odpowienio Either.Left(Error)

Jak to powinno być napisane, żeby wszystko było ok?
z góry dzięki

edytowany 2x, ostatnio: Madek123
Korges
  • Rejestracja:około 5 lat
  • Ostatnio:33 minuty
  • Postów:569
1

Generalnie isRight() to to samo jak isEmpty() czy == null
Zobacz mój projekt (budowanie Either.Left jest u mnie troche skopane)
https://github.com/Korges/java-rest-mail-api/blob/master/src/main/java/com/korges/demo/service/EmailFacadeServiceImpl.java

edytowany 1x, ostatnio: Korges
vpiotr
  • Rejestracja:ponad 13 lat
  • Ostatnio:prawie 3 lata
1

Nie znam sie wiec sie wypowiem:
Zobacz to, moze pomoze https://rafasf.com/posts/using-either-to-handle-errors/

Skoq
  • Rejestracja:około 6 lat
  • Ostatnio:około miesiąc
  • Lokalizacja:Kraków
  • Postów:255
1

Musisz wiedzieć, że Either jest right biased zatem zamiast pisać:

Kopiuj
if(book.isRight()) return ResponseEntity.ok(book.getRight());
else return ResponseEntity.badRequest(book.getLeft().getMessage());

lepiej jest stworzyć jakiegoś ValidationResolver np:

Kopiuj
public static ResponseEntity resolveEither(Either<ResponseError, ?> data) {
        return data
                .map(ResponseEntity::ok)
                .getOrElseGet(ValidationResolver::jakasMetodaDoTworzeniaErrora);
    }

I tak to właśnie jest
99xmarcin
  • Rejestracja:około 5 lat
  • Ostatnio:5 miesięcy
  • Postów:2420
1

Generalnie linie:

Kopiuj
if(book.isRight()) return ResponseEntity.ok(book.getRight());
else return ResponseEntity.badRequest(book.getLeft().getMessage());

To operacje na brzegu systemu. IO monad musi się kiedyś wykonać, Either trzeba kiedyś rozpakować. W języku z pattern matchingiem nikogo by nie zdziwiło jak byś zrobił:

Kopiuj
result match {
  case Left(error) -> ResponseEntity.badRequest(error.userMessage);
  case Right(result) ->  ResponseEntity.ok(result);
}

Ja radzę dodać sobie pomocniczą funkcję która skonwertuje Either na ResponseEntity: toResponseEntity(result) tak żeby unikać tego if'a we wszystkich metodach.

To co mnie dziwi z filozoficznego punktu widzenia to to że meetingService.bookMeeting przyjmuje argumenty o których wie że są mogą być złe (Optional).
Ja bym to pewnie zapisał tak że od razu zwracał bym błąd z kontrolera jak Usera nie ma w bazie. A do bookMeeting przekazywał po prostu referencje o których zakładam że nie są null'ami. Fail fast jak mówią...

Nawet z tymi poprawkami meeting.get().getUser().empty() wygląda nieczytelnie. Lepiej to nazwać isBooked() lub podobnie. Warto rozważyć trzymanie stanu spotkania w enumie, jako jawną wartość a nie przez stan pól null/nie-null. Ale pozostawiam to do twojej oceny.

Na konie wracając do przykładu z wyjątkami. W klasycznych business app zwykle robi się jeden BusinessException i nim ogarnia wszystkie przypadki. Czasami dodaje się EntityNotFoundException żeby genrować prosto 404. W wyjątku EntityNotFound zawsze warto zwrócić ID które było szukane - ułatwi to debugowanie.
Można się też zastanowić czy nie lepiej mieć na repo metody findUserByIdRequired która rzuci wyjątek jak nie znajdzie usunie to if'ologie z tych miejsc gdzie wymagamy istnienia encji.


Holy sh*t, with every month serenityos.org gets better & better...
edytowany 2x, ostatnio: 99xmarcin

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.