Code Review - aplikacja mikropożyczek (zadanie rekrutacyjne)

Question

Cześć!

Ostatnio poszukiwałem pracy i od jednej z firm otrzymałem zadanie rekrutacyjne do wykonania (niestety za późno się odezwali i musiałem im odmówić, bo przyjąłem inną ofertę ;) ).

Piszę tego posta, ponieważ prosiłbym o code-review, konstruktywną krytykę, abym wiedział co można jeszcze poprawić. Przy okazji może ktoś chciałby się sprawdzić i sam rozwiązać takie zadanie, które można napotkać w procesie rekrutacji. Zadanie jest następujące:

„Cel: Prosta aplikacja mikro pożyczkowa.

Wymagania biznesowe:

- Klient może wnioskować o pożyczkę wysyłając kwotę oraz termin

- Dokonana jest analiza wniosku pod względem ryzyka. Aplikacja jest uznana za ryzykowną jeśli:

- wniosek jest pomiędzy 0:00 a 6:00 rano i wniosek jest na kwotę maksymalną

- trzykrotne wnioskowanie z tego samego adresu IP

- pożyczka jest przyznana jeśli przechodzi przez proces ryzyka bez błędów

- w trakcie trwania umowy o pożyczkę, klient może jednorazowo wnioskować o odroczenie terminu spłaty (przedłużenie pożyczki) o 14 dni.

Wymagania techniczne:

- SOLID

- tylko backend, zero gui

- REST API

- testy jednostokowe

- test akceptacyjny dla pozytywnego przypadku”

Link do mojego kodu: https://gitlab.com/Sanecki/micro-loan

Z góry dziękuję bardzo!

Comments

tak z ciekawości, to jest zadanie na juniora?

---

wiesz co, z tego co pamiętam to na Java Developera (bodajże wymagali min 2 lata na stanowisku Java Developera lub Junior Java Developera)

---

Pawi125 moim zdaniem na juniora mogłoby to być, nie jest to nic skomplikowanego i wymagaja samych podstaw ale wiadoko że dochodzi stres itp wiec nie przesadzalbym z czyms wiecej...

---

Tak jasne mogło być na juniora. Tak pytam z ciekawości co teraz wymagają na mida a co na juniora :)

Answer 1

Co ja bym zrobił inaczej:

1. https://gitlab.com/Sanecki/micro-loan/-/blob/master/src/main/java/pl/csanecki/microloan/loan/dto/LoanQuery.java - zrezygnowałbym z typów prostych w DTO. Jeżeli zmienna jest polem danej klasy, to nawet niezainicjowana przyjmuje defaultową wartość (dla inta to 0). 

2. https://gitlab.com/Sanecki/micro-loan/-/blob/master/src/main/java/pl/csanecki/microloan/loan/dto/UserRequest.java - raczej bym nie tworzył UserRequest na podstawie HttpServletu. Zostawiłbym zwykłego Stringa, bo w razie konieczności nie będziesz musiał tworzyć nowego konstruktora.

3. DTO nie powinny mieć metod oraz według mnie. Zastanowiłbym się nad typami pól, w takim sensie że nie wypychałbym obiektu LocalDateTime nazewnątrz, nawet jeśli Spring radzi sobie z tym i mapuje to na Stringa.

4. https://gitlab.com/Sanecki/micro-loan/-/blob/master/src/main/java/pl/csanecki/microloan/loan/LoanController.java - zastosowałbym konwencję nazewnictwa REST'ową. Spring udostępnia bardzo fajny sposób autentykacji adnotacją @PreAuthorize, gdzie mając jedną metodę z jednym URL (korzystając z PathVariables) można łatwo wydzielić logikę do sprawdzania czy User ma prawa żeby "wykonać metodę" (prawa, np jest Adminem lub jest osobą zalogowaną). Nie sprawdzałbym czy coś jest negatywną odpowiedzią wykorzystując operator insatnceof. Raczej bym dodał jakieś Enum.  Zrezygnowałbym też z tekstowych komunikatów błędu (w takim sensie jakim to masz). Raczej bym jakieś customowe kody błędu zastosował - REST'owe kody błędu są dla programisty, frontend powinien je w miarę fajny sposób przedstawić. A dodanie internacjonalizacji do takich błędów, nie będzie zbyt łatwe :D

5. https://gitlab.com/Sanecki/micro-loan/-/blob/master/src/main/java/pl/csanecki/microloan/loan/service/LoanServiceImpl.java - Optional ma metodę orElse (czy jakoś tak) gdzie możesz stworzyć obiekt w przypadku gdy Optional.isEmpty(). Nie musisz sprawdzać ifem czy Optional.isPresent() / Optional.isEmpty(). Polecam plugin Lombok oraz adnotacje takie jak @Data, @Builder itd. Odpada Ci takie coś jak masz w metodzie registerLoan(UserRequest userRequest, LoanQuery loanQuery). Ładniej to wygląda jak Buildera zastosujesz.

Plus na pewno za tworzenie metod/zmiennych które opisują warunki w Ifach :D

Comments

Dzięki wielkie! Z przyjemnością zapoznam i zastosuję się do uwag :)