Cześć, planuje przebranżowić się z automatyka na programistę i napisałem pierwszą stronkę w Javie. Posiada ona jedną bardzo dużą wadę, o której wiem, ale może póki co nie będę zdradzał co to jest. Proszę o ocenę jakości kodu oraz co należałoby poprawić. Kod: https://github.com/mateuszgrzelak/pogaduszki. Adres: http://pogaduszki.com.pl/
0
2
Na pierwszy rzut oka - minusy: zero testów. Plusy: wymyśliłeś klasę o nazwie ClassWithOnlyOneFieldString
.
1
Na szybko,dodałbym cursor: pointer;
do buttonów, no i może jakaś walidacja hasła (wymuszenie większej ilości znaków itp.)
1
- Zamiast sprawdzać czy message jest nullem mogłeś sobie zrówić Optionala (owrapuj sobie w repozytorum, JPA to ogarnia), zrobić mapa i odpowiedzieć obiektem lub 404.
- Nie rozumiem dlaczego używasz Gsona, bo spring chyba ma jakąś zależność do Jacksona i jeśli zwrócisz obiekt to powinien to przemielić na jsona
- Tworzenie mappera dla każdego requestu też nie ma chyba sensu. Nie używałem nigdy Gsona ale wydaje mi się, że mogłeś z niego zrobić beana i byłby zainicjalizowany raz a nie za każdym requestem
- Za dużo logiki w kontrolerze (+ zależności do repozytoriów). On ma przyjąć request, przekazać go do miejsca które zajmie się obsługą wiadomości (jakiś service) i ewentualnie zwrócić (w twoim przypadku nie zwraca nic)
- Nie wiem co robi ten kontroler po jego nazwie/adresie. Message i czat to nie są dobre nazwy. Opisz metodę nazwą która mówi co to robi
- jak robiłeś refaktor nazw pakietów to nie wszędzie je zmieniłeś
- Jeśli to nie copy paste to nie umieszczaj inormacji w stylu "jak chcesz uruchomić to lokalnie to se to zmień", w springu możesz sobie tworzyć profile (@profile), które załadują Ci odpowiednią konfigurację na podstawie tego z jakim profilem odpalasz aplikację. Wtedy wystarczy wspomnieć o tym, że są 2 profile w dokumentacji (albo wystarczy że ktoś looknie do resources i zobaczy jakieś masz properties)
- Dlaczego definiujesz to w dwóch różnych miejscach? https://github.com/mateuszgrzelak/pogaduszki/blob/master/src/main/java/pl/com/pogaduszki/chat/configuration/WebSecurityConfig.java#L47 stwórz sobie z tego beana tak jak w tym przykładzie. Jak zmienisz w jednym miejscu to nie zapomnisz zmienić tego w drugim (https://www.baeldung.com/spring-security-registration-password-encoding-bcrypt#define-the-password-encoder)
Ogólnie:
- User ma swoje DTO, dlaczego inne modele ich nie mają? Dobrze jest to ujednolicić jeśli nie stosujesz podejścia "encja na twarz i pchasz" jak w przypadku MessagesDB (no i nazewnictwo - User (mongo), MessageDB (mongo), Message (dto), UserDTO (dto). Nazwy się nie trzymają kupy)