Code review projektu do CV - fullstack Java dev

Question

Hey!

Mógłbym prosić o review mojego projektu do CV? (fullstack Java dev). Może jakieś porady, albo co zrobić lepiej... :)

https://github.com/gjmOfficial/Centrala-schroniska

Z góry dzięki :)

Answer 1

Spoko sprawa z code review. Nie patrzę w ogóle na frontend bo się po prostu na tym nie znam, jeśli zaś chodzi o backend:

1) Pakietyzacja, nie jest najlepszym pomysłem robić pakietu np. Animal, i trzymać tam wszystko co jest z tym związane, w tym pakiecie masz taki miszmasz, masz encje, rest controller dao i exception. 

Dużo lepszym pomysłem jest mieć oddzielne pakiety pakiety entity, controllers, repository (czym tam dao jak wolisz), exception itd. gdzie w pakiecie entity będziesz trzymać wszystkie encje, w pakiecie controllers będziesz trzymać kontrolerty itd itp. pakiety prezentują w ten sposób logiczny podział.

 

 2) AnimalController, po pierwsze, nie musisz zwracać ResposneEntity, jest to zbędne np @PostMapping domyślnie zwraca HttpStatus.Ok czyli 200, dodatkowo post powinien zwrócić to co postował jeśli robi save to zwraca to coś z ID w twoim przypadku było by coś takiego

@PostMapping("/animal")
    public Animal addAnimal(@RequestHeader(value = "x-auth", required = false) String jwt, @RequestBody Animal animal) {
        workerService.checkIfWorkerIsValidByJwt(jwt);
        return shelter.addAnimal(animal);
    }

Zauważ że metoda addAnimal powinna zwrócić animal, twoje dao podczas metody .save zwraca animal i powinnieneś je odesłać na front (animal będzie miec ustawione ID). 

Dodatkowy problem jest taki że zwracasz tutaj encje na front, co nie powinno mieć miejsca, jest wzorzec DTO (data trasfer object) który powinieneś zwrócić na front, i tak samo przyjąć w rest controllerze i dokonać konwersji z DTO na Encje.

Robi się to dlatego żeby przez przypadek nie ustawić czegoś na encji zarządzanej przez hibernate, np. robisz set na jakimś polu na encji która jest zarządzana przez hibernate i to automatycznie zostanie zapisane do bazy danych, nawet nie trzeba robić żadnego save przez dao. No i jest lipa w takim przypadku

3) ControllerAdvice to nie aspekt :) ale to się tyczy punktu 1 jako pakietyzacji

Przepraszam ale nie dam rady w tym momencie zrobić pełnego review. Ogólnie baaaaaardzo duży plus dla Ciebie że w ogóle prosisz o review! Nie jest źle! Jest trochę do poprawy ale masz w tym momencie bardzo dobry punkt startowy żeby zrobić coś fajnego, zajrzę jeszcze koło 20 dziś to podeślę jeszcze kilka tematów

Comments

gdzie w pakiecie entity będziesz trzymać wszystkie encje, w pakiecie controllers będziesz trzymać kontrolerty itd itp

W pakiecie entity warto jeszcze rozbić na pakiety osobno np. animals i workers itd. 

---

@Arkadiusz Fajdek, moim zdaniem rozbijanie aplikacji na pakiety zwiazane z typem klasy, moze szybko doprowadzic do balaganu. Bo gdy aplikacja sie rozrasta dodaje sie duzo klas i pakietu typu service robi sie smietnik. Najlepszym rozwiazaniem jest tworzenie modulow/pakietow per funkcjonalnosc. Agregujace wszystko to co jest potrzebne w danym problemie. 

Zwlaszcza ze mozna robic podpakiety

---

Moim zdaniem właśnie Ty masz tutaj rację. Jestem  właśnie w trakcie nauki DDD i miedzy innymi właśnie o to w tym chodzi. O dzielenie programu według funkcjonalności.

Answer 2

Brak testów jednostkowych po stronie backendu. Warto się tym zająć.

Poniższa klasa:

https://github.com/gjmOfficial/Centrala-schroniska/blob/master/backend/src/main/java/com/gjm/centralaschroniskabackend/shelter/Shelter.java

ma najprawdopodobniej za dużą odpowiedzialność. Za pomocą obiektu tej klasy możesz jednocześnie dodawać nowe zwierzaki, a także generować jakieś raporty. Najpewniej wypadałoby to rozdzielić.

 

Tutaj masz kwiatka:

CentralaSchroniskaBackendApplication

Chyba się domyślasz co mam na myśli.

I prawdopodobnie, o ile dobrze widzę to nie masz walidacji requestów. Jest to bardzo ważny element takiej aplikacji. Na przykład pomyśl co się stanie jak ktoś poda jako imię zwierzaka taką wartość:

@#$%^&*()_12341

Answer 3

Co do frontu.

1. Containers powinny byc wydzielone od dummy components.

2. Niektore rzeczy mozna bardziej wydzielic. Np : Cell i Row to sa osobne komponenty.

3. Zapytania do api fajnie wydzielic jako osobne services

4. 'dosyc' niedawno wprowoadzono skladnie async await. Polecam sie zapoznac

5. Niektore komponenty sa nieczytelne - https://github.com/gjmOfficial/Centrala-schroniska/blob/master/frontend/src/components/auth/WorkerRegistrationForm.js . Powinienes to jakos podzielic.

6. Zpoznaj sie z destrukturyzacja http://shebang.pl/artykuly/destrukturyzacja/

7. Jezeli to mozliwe uzywamy const nie let.