Użycie vavr przy pisaniu CRUD w springu

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:

@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:

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:

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

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

1

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

1

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

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

lepiej jest stworzyć jakiegoś ValidationResolver np:

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

Generalnie linie:

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ł:

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.

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