• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Code review projektu do CV - fullstack Java dev

0 głosów
176 wizyt
pytanie zadane 3 lutego w Java przez hiimRealgjm Początkujący (360 p.)
zmienione kategorie 3 lutego przez Arkadiusz Waluk

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

3 odpowiedzi

+1 głos
odpowiedź 3 lutego przez Arkadiusz Fajdek Mądrala (7,270 p.)

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

komentarz 3 lutego przez YourDoom Bywalec (2,410 p.)

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. 

2
komentarz 3 lutego przez Mateusz51 Nałogowiec (27,860 p.)

@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

1
komentarz 3 lutego przez mibdbz Bywalec (2,060 p.)
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.
0 głosów
odpowiedź 3 lutego przez mbabane Maniak (69,900 p.)

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
0 głosów
odpowiedź 4 lutego przez k.wichura Pasjonat (19,590 p.)
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.

Podobne pytania

0 głosów
1 odpowiedź 112 wizyt
pytanie zadane 28 lutego w Java przez anonymousProgrammer Nowicjusz (230 p.)
+2 głosów
1 odpowiedź 708 wizyt
0 głosów
1 odpowiedź 170 wizyt
pytanie zadane 4 lutego w Java przez varespol Użytkownik (660 p.)
Porady nie od parady
Komentarze do pytań nie służą do odpowiadania, od tego jest wydzielona sekcja odpowiedzi. Funkcją komentarzy jest natomiast możliwość uzyskania dodatkowych informacji na temat samego posta.Komentarze

64,924 zapytań

111,392 odpowiedzi

234,425 komentarzy

46,754 pasjonatów

Przeglądających: 231
Pasjonatów: 12 Gości: 219

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto dwie polecane książki warte uwagi. Pełną listę znajdziesz tutaj.

...