Mógłby mi ktoś zrobić code review apki do zarządzania kinem?
https://github.com/Sampeteq/cinema-app
0
2
Tak na telefonie w kilka miejsc spojrzałem i
po pierwsze, to, że można używać w Javie 'var' nie oznacza, że należy używać tego w każdym miejscu w kodzie,
druga sprawa która wpadła mi w oko to taka, że trzymasz tam jakiś interfejs w tej samej klasie co klasa gdzie go implementujesz,
po trzecie nie poprawnie wstrzykujesz serwisy - samo zdefiniowanie serwisu jako pola w klasie to za mało,
po czwarte - taka klasa jak
SpringDataJpaTicketRepository jest bez sensu bo robi dokładnie to samo co repozytorium samo w sobie
po piąte - jak robisz repozytorium to nie dodawaj tam 'SpringDataJpa' - jest to kompletnie nie potrzebne
po szóste - ValidationException jest nie potrzebne, robi to samo co runtime exception
po siódme - rzucasz wyjątkami bez opakowania ich w odpowiednie zwrotki do resta - poczytaj o controller advice
po ósme - w kontrolerach raczej sobie darujmy logike, znalazłem gdzieś tam jakiś builder na post chyba, lepiej mieć to w serwisie
Jak nie zapomnę to później na pc spojrzę szerzej i powiem o które miejsca mi chodzi i dopiszę kolejne punkty bo to dopiero początek pewnie
0
Escanor16 napisał(a):
Tak na telefonie w kilka miejsc spojrzałem i
po pierwsze, to, że można używać w Javie 'var' nie oznacza, że należy używać tego w każdym miejscu w kodzie,
W którym miejscu byś nie używał vara?
Escanor16 napisał(a):
druga sprawa która wpadła mi w oko to taka, że trzymasz tam jakiś interfejs w tej samej klasie co klasa gdzie go implementujesz,
Nie, ten interfejs pochodzi ze Spring Data JPA i jest wstrzykiwany w klasę, która implementuje domenowy interfejs repozytorium
Escanor16 napisał(a):
po trzecie nie poprawnie wstrzykujesz serwisy - samo zdefiniowanie serwisu jako pola w klasie to za mało,
One są wstrzykiwane przez konstruktor, tylko go nie widać, bo jest generowany przez Lomboka
Escanor16 napisał(a):
po czwarte - taka klasa jak
SpringDataJpaTicketRepository jest bez sensu bo robi dokładnie to samo co repozytorium samo w sobie
Ta klasa implementuje domenowy interfejs TicketRepository, SpringDataJpaTicketRepository to konkretna implementacja, która siedzi w infrastrukturze
Escanor16 napisał(a):
po piąte - jak robisz repozytorium to nie dodawaj tam 'SpringDataJpa' - jest to kompletnie nie potrzebne
Dodałem to, żeby zaznaczyć, skąd pochodzi ta implementacja
Escanor16 napisał(a):
po szóste - ValidationException jest nie potrzebne, robi to samo co runtime exception
Jest potrzebne, bo to wspólny interfejs, dla wszystkich wyjątków, które lecą przy walidacji biznesowej. Dzięki temu mam 1 exception handler dla tego typu wyjątków i mogę zwrócić userowi message tego błędu z odpowiednim statusem (422) zgodnie ze standardem resta
Escanor16 napisał(a):
po siódme - rzucasz wyjątkami bez opakowania ich w odpowiednie zwrotki do resta - poczytaj o controller advice
Przecież mam exception handlery z rest controller advice, nie widziałeś?
Escanor16 napisał(a):
po ósme - w kontrolerach raczej sobie darujmy logike, znalazłem gdzieś tam jakiś builder na post chyba, lepiej mieć to w serwisie
Nie mam jak inaczej tego przekazać, żeby też ładnie to wyglądało w Swagerze. Tam nie jakieś wielkiej logiki, jest tylko budowanie query z parametrów
0
Jedynie rzucilem okiem. Nie rozumiem jednej rzeczy. Po co testy domenowe (np Ticket czy Repertoire) robisz w jakichs springowych abominacjach?
0
Seken napisał(a):
Jedynie rzucilem okiem. Nie rozumiem jednej rzeczy. Po co testy domenowe (np Ticket czy Repertoire) robisz w jakichs springowych abominacjach?
Mam testy e2e, one muszą używać Springa