[Nofenak]

Zrobiłby mi ktoś code review?
https://github.com/Sampeteq/cinema-app
#java #naukaprogramowania #programowanie

[cppguy]

@Nofenak:

[Nanari]

@Nofenak: Jak tam po angielsku wszystko, musisz po polsku.

[Patres]

@Nofenak: Tak

[humblehound]

@Nofenak: Wszystko wygląda dość legitnie, natomiast ten event-driven handling to trochę overkill. Masz po jednym listenerze dla każdego eventu, prościej by było po prostu wywołać te handlery tam gdzie trzeba. Imo event-driven flow ma sens kiedy faktycznie chcemy się bawić w mikroserwisy, tutaj wprost piszesz że to jest monolit. Tak po za tym to ciężko mi się przyczepić, gj

[Saly]

@Nofenak: w docker file powinnieneń budować jara. Na temat kodu to średnio mogę się wypowiedzieć, bo nie jestem into java, ale:
* domain/Room.java nie powinien mieć w sobie bazodanowego śmiecia. Powinieneś mieć osobny "czysty" typ domenowy zwracany przez repozytorium a to co masz teraz powinno iść tam gdzie jest implementacja
* brak serwisów domenowych
* dwustrona relacja pomiędzy appllication a domain: application używa domain a domain oczekuje całą tranzakcyjność od springa.

[Krolik]

@Nofenak: To jakaś kolejna odmiana FizzBuzzEnterprise?

Klasy które mają po jednej metodzie? Pakiety, podpakiety, podpodpodpakiety a w środku po 3 pliki? Klasy, które nic nie robią a tylko przekazują sterowanie gdzieś dalej? Po ch*j to? Nie cierpię takiego stylu kodowania.

Projekt tej wielkości powinien być płaski. Jeden pakiet, kilka klas reprezentujących dane wyciągane z bazy, jeden serwis zawierający całą logikę.

[Patres]

@Nofenak: Nie wiem jaki masz poziom, ale ogólnie bardzo spoko to wygląda. Podoba mi się podział na domeny i że REST API jest zgodne ze standardem. Przeklinałem losowe klasy:

1. UserPasswordResetHandlerService
log.info("Mail sent");
Ten logger niczego nie mówi, dodaj kontekst (np. typ maila)

2. SeatDto
boolean isFree
Unikałbym booleanów jak ognia. Przyjdzie Ci zaraz product owner i powie że chce miec nowy typ - częsciowo free itp. Nawet jak mamy 2

[Saly]

Projekt tej wielkości powinien być płaski. Jeden pakiet, kilka klas reprezentujących dane wyciągane z bazy, jeden serwis zawierający całą logikę.

@Krolik: ma to sens jak chcesz sobie pocwiczyć. W praktyce taki kod nie ma sensu przy tej skali i zerowej logice

[horus666]

@Nofenak: Jeszcze się tylko zesrać na środku i będzie komplet.

[Krolik]

@Saly: Ale co poćwiczyć? Pisanie projektów poszatkowanych bez sensu zelmerem?

[ElderWrath]

@Nofenak: Taki randomowy strzał, po co robisz UserPasswordNewService, UserCreateService, UserPasswordResetService, itd.
Nie lepiej po prostu zrobić jeden serwis dla danej funkcjonalności, np. ogólny UserService w którym możesz zrobić tworzenie, pobieranie, edytowanie usera i jego składowych

[Killeras]

@ElderWrath:

@Nofenak

https://github.com/Sampeteq/cinema-app/blob/main/src/main/java/com/cinema/users/application/services/UserAdminCreateService.java

nie wstrzykuj w ten sposób wartości z propertek,lepiej jest to robić prze konstruktor, wtedy masz robisz adnotację nad tym Stringiem i spring ci to ogarnia, aktualnie ta klasa jest nietestowalna jednostkowo, bo żeby wartości wstrzyknąć musisz mieć kontekst springa podniesiony, żeby wczytać dane z pliku properties

https://github.com/Sampeteq/cinema-app/blob/main/src/main/java/com/cinema/rooms/application/services/RoomAvailableService.java
to jest strasznie słabe, co jeśli będziesz miał milion pokoi w bazie?

https://github.com/Sampeteq/cinema-app/blob/main/src/main/java/com/cinema/rooms/application/services/RoomConfigService.java

tutaj dwie rzeczy w metodzie readRoomsConfigAsJson robisz coś

[Killeras]

@Nofenak: https://github.com/Sampeteq/cinema-app/blob/main/src/main/java/com/cinema/repertoire/domain/Seat.java

jak gdzieś w klasie używasz pola to nie musisz pisać this.pole tylko samo pole

[Myzreal]

@Krolik: Jak kiedyś nabierzesz na tyle doświadczenia żeby dołączyć do dużego i długoterminowego projektu to może docenisz staranne paczkowanie, enkapsulację na poziomie modułów i inne takie "głupoty" :)

[Krolik]

@Myzreal: pracuję w projekcie mającym 500 tys. linii kodu + miliony linii kodu w zależnościach

[Myzreal]

@Krolik: No to patrząc na twoje podejście do tematu to nie zazdroszczę nikomu w tym projekcie xD

[Krolik]

@Myzreal: nie, po prostu początkujący jak OP (i zapewne Ty też skoro go bronisz) mają zupełnie inne pojęcie dobrego kodu, takie trochę niedojrzałe. Dla nich dobry kod to taki gdzie nasrają tysiące wzorców GoF, klas, interfejsów, dziedziczenia, cieknących abstrakcji i funkcji "na przyszłość" bo a nuż kiedyś kod trzeba będzie zmienić. Przerost formy nad treścią albo inaczej overengineering, typowy dla projektów w Javie. Zabawnie jest patrzeć jak potem ten super uniwersalny

[Myzreal]

@Krolik Spoko ziomek, z czasem zrozumiesz

[szmichal]

@Krolik: this. Ale tego uczy sie po latach. Lepszy prosty, sekwencyjny kod, pare testowalnych serwisow niz zaciemnione mediatory i #!$%@? wie, gdzie w sumie idzie request.