Aspekty techniczne projektu

Cześć,

Czy mógłby ktoś z doświadczeniem zerknąć na projekt pisany do portfolio : https://github.com/rocesS/notarycrm
Zależy mi na ocenie poprawności struktury i zastosowanych rozwiązań z punktu widzenia praktycznego. Na chwilę obecną przyswajam wiedzę z zakresu Spring Security aby utworzyć logowanie do w/w serwisu.
Z góry dzięki

Projekt na pierwszy rzut oka wygląda bardzo fajnie, ale zamiast tego techniczego podziału na "service", "controller" itd. wprowawdziłbym podział domenowy typu "person", "user" itd. Ogólnie to poczytaj o package by feature

1. Ale albo projekt jest w całości po angielsku, albo wrzucasz katalog Pliki do readme. Zdecyduj się.

2. Aktualnie nie pisze się projektów tak jak ty to napisałeś. W normalnej robocie wystawiłbyś REST API, a front byłby osobną aplikacją napisaną w React/Vue/Angular. Czyli zamiast @Controller @RestController

3. //wyszukanie po KRS podanym przez użytkownika

   if(statusCode.is2xxSuccessful()) { // dla odpowiedzi z grupy 200

   Niepotrzebne komentarze tylko przeszkadzają zamiast pomagać.

4. Twoje wyjątki rozszerzające Throwable tylko po, żeby później znaleźć się w sygnaturze metody to coś na co ktoś ogarniający Javę od razu zwróci uwagę i zapyta się w jakim celu ty to zrobiłeś.

Więcej tak na szybko to nie widzę czego można się czepić bo kodu mało, chociaż pewnie coś by się jeszcze znalazło. Ale trzeba pochwalić za porządne README. To serio ważniejsze niż myślisz.

Testy są 2/10. Np. ten KrsValidatorServiceTest nic nie testuje, jest do wyrzucenia.

Pobierzenie przeczytałem testy, i szczerze mówiąc nie widzę ani jednego który wnosiłby jakąkolwiek wartość Za to są napisane w taki sposób, że utrudniają refaktor i pracę z kodem. Zapędziłeś się trochę nimi w kozi róg.

Zależności nie są prawidłowo zadbane, przez co coupling jest wysoki; przez co zmiany i napisanie dobrych testów jest trudne; przez co tak na prawdę każda zmiana jest ryzykowna i może spowodować bugi które ciężko znaleźć na lokalu.

1. nie pisz po polsku (pliki do readme)
2. jeśli chodzi o frontend to odchodzi się od podejścia jakei mas z w projekcie na rzecz separacji frontendu i backendu i komunikację za pośrednictwem rest api (front w angular/react/vaadin itp)
3. architektura (struktura katalogów) typowo warstwowa co ma mnóstwo wad -> są inne (package per feature/ hexagonal architecture)
4. wszystko jest publiczne co po części wynika z architektury wartstwowej -> poczytaj czym jest enkapsulacja oraz po co się to stosuje
5. Twoje encje bazodanowe (klasy oznaczone jako @Entity) są "sercem" Twojej aplikacji co niesie za sobą mnóstwo minusów (nawiązanie do architektury hexagonalnej i oddzielenia obiektów domenowych od warstwy stricte persystencji)
6. Twoje encje są anemiczne: porzechowują tylko dane, nie zawierają logiki (np. w encji User w metodzie setFirstName mógłbyś umieścić logikę odpoiwiedzialnąza np. sprawdzenie czy input nie jest nullem, pustym stringiem czy czy ma np. wymaganą minimalną/maksymalną długość - tę walidację przeniosłeś na silnik bazy danych za pomocą adnotacji Column co już utrudnia otestowanie tego unitowo z poziomu aplikacji). Generalnei szukaj pod hasłem "anemiczny / bogaty model".
7. Przechowywanie password w postaci Stringa z uwagi na specyfikę obiektó typu String (string pool) nie jest ani zalecane ani bezpieczne
8. komentarze w kodzie przydają się sporadycznie, u Ciebie są zbędne
9. Mocno operujesz na wyjątkach co nie pozostaje bez impaktu ani na design ani na performance, zamiast tego mógłbyś skorzystać z np. Either z biblioteki vavr.io (po szczegóły odsyłam na blogi typu baeldung)
10. nie potrzebujesz @Autowired nad konstruktorami
11. ogólne nazewnictwo typu UserService jest turbo mało mówiące
12. Wyjątki typu checked (jak np. w UserService) to generalnie była wtopa twórców javy (oczywiście można polemizować)
13. w metodzie delete w UserService najpierw wołasz countById a jak wynik będzie > 1 to usuwasz. Po co? countById jest mniej efektywna, możesz użyć existById któej wynik typu boolean jest o wiele więcej mówiący (pomijam fakt, że w momencie gdy rekord istnieje to countById nigdy nie zwróci więcej niż 1 bo sprawdzasz po id bazodanowym które jest unikalne z definicji)
14. Gdzieś pisałem o nazewnictwie. LegalPersonController i pole service -> teraz jest jeden ale jak się rozrośnie i dojdzie N innych zależności do klasy to fakt czym ten service jest gdzieś imho zaniknie i warto od razu nadawać jasne nazwy (choćby miałby to być stricte typ pola np. legalPersonService
15. Optionali używasz w miejscu instrukcji if (w LegalPersonService) a nie do tego on powstał.
16. LegalPersonController.saveLegalPerson() jako argument metody w kontrolerze przyjmujesz obiekt który jest encją bazodanową -> słabo, od tego służą dto (Data Transfer Object)
17. przy nazewnictwie testów bardzo pomaga konwencja "should X when y"
18. jeśli chodzi o ciało testu to dla czytelności stosuje się podejście "given -> when -> then"
19. testy w spocku są imho dużo czytelniejsze niż w junit
20. W obiektach Entity nie deklarujesz żadnego konstruktora (kompilator generuje bezparametrowy który i tak jest wymagany przez hibernate) przez co np. w testach musisz obiekty setupować setterami co generalnie umożliwia stworzenie obiektu w niepoprawnym stanie (gdy zapomnisz ustawić niektórych pól). Konstruktory po coś są i warto z nich korzystać.
21. Masz aplikację typu crud (chociażchyba nawet niepełny) więc zainteresuj się piramidą testów a potem doczytaj dlaczego to bullshit i warto uzależniać ilość konkretnego rodzaju testów od aplikacji (w crudzie bez logiki biznesowej większą wartość majątesty integracyjne/e2e aniżeli unit testy pisane na siłę)
22. Aplikacja jest prosta ale jako, że jest to do portfolio to warto byłoby pokazać, że wiesz do czego służy interfejs i jak go użyć

Gdybyś zmienił podejście do frontendu i ewentualnie wystawił samo rest api to już miałbyś większe pole do popisu aby sobie rozbudowywać ten projekt.

BTW: sklonowałem repo i aplikacja mi się nie buduje (wywala się na testach). Nie chce mi się weryfikować czy t owina mojego środowiska ale pamiętaj aby nie mergować do mastera kodu który się nie buduje.

A no i (imho) najważniejsze: długa droga przed Tobą dlatego zostaw to Spring Security i szlifuj podstawy ;)

RequiredNickname napisał(a):

1. przy nazewnictwie testów bardzo pomaga konwencja "should X when y"

Z takimi poleceniami to ostrożnie.

Ja bym powiedział że warto zwracać uwagę na czytelność testów i ich nazw, i na pewno należy sobie wypracować taki styl nazewnictwa, żeby patrząc na test było widać:

• dokładnie jakie zachowanie jest spodziewane
• kiedy test przechodzi a kiedy faili

A to czy nazwiesz je konkretnie should/when, it/if czy jakkolwiek inaczej, to ma drugorzędne znaczenie. Ważne jest żeby w nazwie testu była zawarta treść, a konkretny buzz word jak should, tak na prawdę jest nieistotny.

RequiredNickname napisał(a):

1. jeśli chodzi o ciało testu to dla czytelności stosuje się podejście "given -> when -> then"

A sugerujesz tutaj coś konkretnego? Bo to znowu wygląda jak przerost formy nad treścią. Given/when/then bardzo brzmi jak BDD, to to właśnie sugerujesz? Czy chodzi Ci po prostu od odepsarowanie setupu, calla, i asercji?

Pytam, bo ogólnie given/when/then się wzięło z BDD, gdzie mamy jakiś początkowy stan (ale stan, nie argumenty), akcję, i sprawdzamy wyjściowy stan (Ale stan, a nie wynik). Więc wyglądało to np tak:

givenSystemIsRunning();          // stan początkowy
whenAdminSendsTerminateSignal(); // akcja
thenAssertSystemIsNotRunning();  // asercja na spodziewany stan

Niestety większość ludzi nie skumała idei, i przerodziło się w to w jakiś bez sens, gdzie ludzie piszą coś takiego:

// given
string mail = "test@gmail.com";         // to nie jest stan tylko input
// when
bool valid = isValid(mail);             // to nie jest akcja, tylko pure-funkcja
// then
assertTrue(valid);                      // to nie jest asercja stanu, tylko wyniku

i to oczywiście nie ma sensu całkowicie i jest robione po nic.

Powinno się to zapisać tak:

assertTrue(isValid("test@gmail.com"));

Z czytelnościąnazw testów naturalnie masz rację, zastosowałem niejako skrót myślowy bo wydaje mi się, że dla kogoś kto nigdy nie pisał testów porada typu "zastosuj podejście should x when y" jest bardziej zrozumiała niż "używaj czytelnych nazw" a wraz z nabywaniem doświadczenia można poczuć się na tyle pewnie własnych umiejętności by móc ocenić co jest czytelne a co nie ;)

Given/When/Then to podstawa w czasie pisania testów w Spocku i moim zdaniem bardzo poprawia to czytelność (tam na szczęście nie trzeba pisać komentarzy).
Tak chodziło mi o odseparowanie setupu, calla i asercji.
W tych testach naturalnie nie ma to większego znaczenia bo są bardzo małe ale chodzi o wypracowanie podejścia na zaś (chociażby przez stosowanie linijek odstępów) przed spotkaniem z produkcyjnym kodem gdzie setup testu może pochłonąć więcej niż dwie linijki kodu ;)

RequiredNickname napisał(a):

Z czytelnościąnazw testów naturalnie masz rację, zastosowałem niejako skrót myślowy bo wydaje mi się, że dla kogoś kto nigdy nie pisał testów porada typu "zastosuj podejście should x when y" jest bardziej zrozumiała niż "używaj czytelnych nazw" a wraz z nabywaniem doświadczenia można poczuć się na tyle pewnie własnych umiejętności by móc ocenić co jest czytelne a co nie ;)

Oj właśnie moim zdaniem jest zupełnie odwrotnie. Moim zdaniem jak powiesz komuś "używaj should x when y", to on nie będzie wiedzał po co to robi i nie dostanie żadnej wartości z tego.

RequiredNickname napisał(a):

Given/When/Then to podstawa w czasie pisania testów w Spocku i moim zdaniem bardzo poprawia to czytelność (tam na szczęście nie trzeba pisać komentarzy).
Tak chodziło mi o odseparowanie setupu, calla i asercji.
W tych testach naturalnie nie ma to większego znaczenia bo są bardzo małe ale chodzi o wypracowanie podejścia na zaś przed spotkaniem z produkcyjnym kodem gdzie setup testu może pochłonąć więcej niż dwie linijki kodu ;)

No to sorry, ale moim zdaniem ta rada nie ma sensu. Albo ma tylko w bardzo drobnym kontekście.

Jeśli setup testu może pochłonąć dużo kodu, to należy to wynieść do funkcji osobnej, tak żeby sam test był krótki. Jeśli np. test wymaga wydrukowania jakiegoś dokumentu, co zajmuje powiedzmy 20 linijek; to naturalnie nie należy wsadzić tych dwudziestu linijek do testu, tylko wynieść to do funkcji, i użyć jej w teście.

Given/when/then IMO ma tylko sens jeśli masz jakiś stan (początkowy albo końcowy), np. "given a new logged user, when a new post is added, then the user has a notification". Jeśli testujesz pure-funkcje, to oczywiście nie masz stanu, więc given/when/then nie ma sensu. Albo może powinienem powiedzieć że jak masz pure-funkcje, to jedyny krok jaki masz to when. Tylko wtedy po co to pisać.

Parametr wejściowy to nie jest given.

Nofenak napisał(a):

Projekt na pierwszy rzut oka wygląda bardzo fajnie, ale zamiast tego techniczego podziału na "service", "controller" itd. wprowawdziłbym podział domenowy typu "person", "user" itd. Ogólnie to poczytaj o package by feature

Generalnie starałem się utrzymać strukturę/wzorzec MVC, stąd taki podział i pytanie czy w tak niewielkim projekcie ma on rację bytu. "package by feature" - nie znałem tego pojęcia, na pewno się z nim zaposnam

anckor napisał(a):

1. Ale albo projekt jest w całości po angielsku, albo wrzucasz katalog Pliki do readme. Zdecyduj się.

2. Aktualnie nie pisze się projektów tak jak ty to napisałeś. W normalnej robocie wystawiłbyś REST API, a front byłby osobną aplikacją napisaną w React/Vue/Angular. Czyli zamiast @Controller @RestController

3. //wyszukanie po KRS podanym przez użytkownika

   if(statusCode.is2xxSuccessful()) { // dla odpowiedzi z grupy 200

   Niepotrzebne komentarze tylko przeszkadzają zamiast pomagać.

4. Twoje wyjątki rozszerzające Throwable tylko po, żeby później znaleźć się w sygnaturze metody to coś na co ktoś ogarniający Javę od razu zwróci uwagę i zapyta się w jakim celu ty to zrobiłeś.

Więcej tak na szybko to nie widzę czego można się czepić bo kodu mało, chociaż pewnie coś by się jeszcze znalazło. Ale trzeba pochwalić za porządne README. To serio ważniejsze niż myślisz.

1. Poprawię go w całości na angielski
2. Rozumiem, o takie smaczki mi chodziło wstawiając "projekt" do oceny.
3. Te komentarze są dla mnie - gdy będę wracał do tego kodu. Na początku robiłem je ponieważ korzystałem z tutoriala, teraz mając większą wiedze o HTTP wiem co dany fragment znaczy.
4. Mógłbyś powiedzieć coś więcej ? Zamist do superklasy odpowiedzialnej za wyjątki powinienem do 'Exceptions' sie odnieść?

Mało kodu bo staram się wszystko pisać schludnie i ze starannością, a przede wszystkim ze zrozumieniem. Dzięki komentarz i za pochwałę README

Riddle napisał(a):

Testy są 2/10. Np. ten KrsValidatorServiceTest nic nie testuje, jest do wyrzucenia.

Pobierzenie przeczytałem testy, i szczerze mówiąc nie widzę ani jednego który wnosiłby jakąkolwiek wartość Za to są napisane w taki sposób, że utrudniają refaktor i pracę z kodem. Zapędziłeś się trochę nimi w kozi róg.

Zależności nie są prawidłowo zadbane, przez co coupling jest wysoki; przez co zmiany i napisanie dobrych testów jest trudne; przez co tak na prawdę każda zmiana jest ryzykowna i może spowodować bugi które ciężko znaleźć na lokalu.

Testy w tym projekcie są moją zmorą. We wszystkich kursach z którymi miałem do czynienia, tak naprawdę nic sensownego na ich temat nie ma. dlatego próbowałem coś wymodzić. Niestety nie jestem na takim poziomie aby te zależności samodzielnie wyeliminować aby coupling był niski. Masz jakieś rady dot. tego ? Dzięki za rzeczowy komentarz!

RequiredNickname napisał(a):

1. nie pisz po polsku (pliki do readme)
2. jeśli chodzi o frontend to odchodzi się od podejścia jakei mas z w projekcie na rzecz separacji frontendu i backendu i komunikację za pośrednictwem rest api (front w angular/react/vaadin itp)
3. architektura (struktura katalogów) typowo warstwowa co ma mnóstwo wad -> są inne (package per feature/ hexagonal architecture)
4. wszystko jest publiczne co po części wynika z architektury wartstwowej -> poczytaj czym jest enkapsulacja oraz po co się to stosuje
5. Twoje encje bazodanowe (klasy oznaczone jako @Entity) są "sercem" Twojej aplikacji co niesie za sobą mnóstwo minusów (nawiązanie do architektury hexagonalnej i oddzielenia obiektów domenowych od warstwy stricte persystencji)
6. Twoje encje są anemiczne: porzechowują tylko dane, nie zawierają logiki (np. w encji User w metodzie setFirstName mógłbyś umieścić logikę odpoiwiedzialnąza np. sprawdzenie czy input nie jest nullem, pustym stringiem czy czy ma np. wymaganą minimalną/maksymalną długość - tę walidację przeniosłeś na silnik bazy danych za pomocą adnotacji Column co już utrudnia otestowanie tego unitowo z poziomu aplikacji). Generalnei szukaj pod hasłem "anemiczny / bogaty model".
7. Przechowywanie password w postaci Stringa z uwagi na specyfikę obiektó typu String (string pool) nie jest ani zalecane ani bezpieczne
8. komentarze w kodzie przydają się sporadycznie, u Ciebie są zbędne
9. Mocno operujesz na wyjątkach co nie pozostaje bez impaktu ani na design ani na performance, zamiast tego mógłbyś skorzystać z np. Either z biblioteki vavr.io (po szczegóły odsyłam na blogi typu baeldung)
10. nie potrzebujesz @Autowired nad konstruktorami
11. ogólne nazewnictwo typu UserService jest turbo mało mówiące
12. Wyjątki typu checked (jak np. w UserService) to generalnie była wtopa twórców javy (oczywiście można polemizować)
13. w metodzie delete w UserService najpierw wołasz countById a jak wynik będzie > 1 to usuwasz. Po co? countById jest mniej efektywna, możesz użyć existById któej wynik typu boolean jest o wiele więcej mówiący (pomijam fakt, że w momencie gdy rekord istnieje to countById nigdy nie zwróci więcej niż 1 bo sprawdzasz po id bazodanowym które jest unikalne z definicji)
14. Gdzieś pisałem o nazewnictwie. LegalPersonController i pole service -> teraz jest jeden ale jak się rozrośnie i dojdzie N innych zależności do klasy to fakt czym ten service jest gdzieś imho zaniknie i warto od razu nadawać jasne nazwy (choćby miałby to być stricte typ pola np. legalPersonService
15. Optionali używasz w miejscu instrukcji if (w LegalPersonService) a nie do tego on powstał.
16. LegalPersonController.saveLegalPerson() jako argument metody w kontrolerze przyjmujesz obiekt który jest encją bazodanową -> słabo, od tego służą dto (Data Transfer Object)
17. przy nazewnictwie testów bardzo pomaga konwencja "should X when y"
18. jeśli chodzi o ciało testu to dla czytelności stosuje się podejście "given -> when -> then"
19. testy w spocku są imho dużo czytelniejsze niż w junit
20. W obiektach Entity nie deklarujesz żadnego konstruktora (kompilator generuje bezparametrowy który i tak jest wymagany przez hibernate) przez co np. w testach musisz obiekty setupować setterami co generalnie umożliwia stworzenie obiektu w niepoprawnym stanie (gdy zapomnisz ustawić niektórych pól). Konstruktory po coś są i warto z nich korzystać.
21. Masz aplikację typu crud (chociażchyba nawet niepełny) więc zainteresuj się piramidą testów a potem doczytaj dlaczego to bullshit i warto uzależniać ilość konkretnego rodzaju testów od aplikacji (w crudzie bez logiki biznesowej większą wartość majątesty integracyjne/e2e aniżeli unit testy pisane na siłę)
22. Aplikacja jest prosta ale jako, że jest to do portfolio to warto byłoby pokazać, że wiesz do czego służy interfejs i jak go użyć

Gdybyś zmienił podejście do frontendu i ewentualnie wystawił samo rest api to już miałbyś większe pole do popisu aby sobie rozbudowywać ten projekt.

BTW: sklonowałem repo i aplikacja mi się nie buduje (wywala się na testach). Nie chce mi się weryfikować czy t owina mojego środowiska ale pamiętaj aby nie mergować do mastera kodu który się nie buduje.

A no i (imho) najważniejsze: długa droga przed Tobą dlatego zostaw to Spring Security i szlifuj podstawy ;)

Ten komentarz przerósł moje najśmielsze oczekiwania. Naprawdę @RequiredNickname jestem bardzo bardzo wdzięczny za tak skrupulatne podejście do codreview(aż pokazałem swojej kobiecie, jak ludzie w sieci potrafią być pomocni ).
Tych punktów jest tak wiele, że zastanawiam się nad kompletną przebudową projektu z uwzględnieniem DAO, DTO i hexagonal architecture - te ostatnie pojęcie jest mi zupełnie nowe.

Gdybyś zmienił podejście do frontendu i ewentualnie wystawił samo rest api to już miałbyś większe pole do popisu aby sobie rozbudowywać ten projekt.

Generalnie mocno zastanawiam się nad przebudową projektu, ale takie podejście jak wyżej z bardzo prostym frontem było podyktowane tym, że już miałem dosyć aplikacji konsolowych. Chciałem żeby choć trochę było widoczne to co napiszę i można było przysłowiowo "poklikać". Mogłbyś powiedzieć na czym miało by wyglądać "wystawienie samego rest api" w mojej aplikacji?

BTW: sklonowałem repo i aplikacja mi się nie buduje (wywala się na testach). Nie chce mi się weryfikować czy t owina mojego środowiska ale pamiętaj aby nie mergować do mastera kodu który się nie buduje.

Mi startuje aplikacja, masz na myśli że testy w src/test/java/com/example/notarycrm nie działają? Kod który się nie buduje mam rozumieć przez to że aplikacja się nie uruchamia ?

rocess napisał(a):

Nofenak napisał(a):

Projekt na pierwszy rzut oka wygląda bardzo fajnie, ale zamiast tego techniczego podziału na "service", "controller" itd. wprowawdziłbym podział domenowy typu "person", "user" itd. Ogólnie to poczytaj o package by feature

Generalnie starałem się utrzymać strukturę/wzorzec MVC, stąd taki podział i pytanie czy w tak niewielkim projekcie ma on rację bytu. "package by feature" - nie znałem tego pojęcia, na pewno się z nim zaposnam

MVC w takiej postaci w jakiej jest prezentowane w większości frameworków (Laravel, Rails, Django, Spring) nie ma nic wspólnego na prawdę z MVC.

Wzorzec MVC został wymyślny przez gościa imieniem Trygve Reenskaug około 1980 roku; i nie było tam żadnej mowy o warstwach. Wzorzec mówił tylko o tym żeby odseparować input, przetwarzanie i output.

Model w MVC to miała być domena biznesowa (niestety autorzy frameworków się pomylili, i pomyśleli że chodzi o modele ORM'owe). Inputem miała być akcja użytkownika dowolna (niestety autorzy frameworków tutaj też się pomylili, i pomyśleli że chodzi tylko o submit formularza). Wyjściem miał być oczywiście widok, ale nie koniecznie widok html'owy tylko każdy możliwy rodzaj outputu (pliki, html, aplikacje mobilne, endpointy ajaxowe, maile, wszystko co jest jakąkolwiek formą prezentacji to jest V w MVC orginalnym). Nic o żadnych warstwach tam nie było mowy. Dodatkowo MVC dotyczyło małych rzeczy; np. osobny MVC dla inputa, osobny dla hasła, osobny dla przycisku, osobny dla nagłówka. W frameworkach "mvc" to to MVC jest dla całego page'a albo nawet całego CRUD'a, co całkowicie nie ma nic wspólnego z orginalnym wzorcem. Jak każdy pomysł w IT, MVC zostało karykaturą swojej początkowej idei.

Więc jeśli chcesz tworzyć warstwy w aplikacji w imię MVC, to warto się zastanowić.

anckor napisał(a):

Testy w tym projekcie są moją zmorą. We wszystkich kursach z którymi miałem do czynienia, tak naprawdę nic sensownego na ich temat nie ma. dlatego próbowałem coś wymodzić. Niestety nie jestem na takim poziomie aby te zależności samodzielnie wyeliminować aby coupling był niski. Masz jakieś rady dot. tego ? Dzięki za rzeczowy komentarz!

Nie obiwniam Cię, jest bardzo mało wartościowych materiałów na ten temat.

RequiredNickname napisał(a):

Ten komentarz przerósł moje najśmielsze oczekiwania. Naprawdę @RequiredNickname jestem bardzo bardzo wdzięczny za tak skrupulatne podejście do codreview(aż pokazałem swojej kobiecie, jak ludzie w sieci potrafią być pomocni ).
Tych punktów jest tak wiele, że zastanawiam się nad kompletną przebudową projektu z uwzględnieniem DAO, DTO i hexagonal architecture - te ostatnie pojęcie jest mi zupełnie nowe.

Jeśli mogę Ci cokolwiek doradzic; i na prawdę chcesz poznać wszystkie dobre praktyki, to nie zaczynaj od aplikacji webowych (takiej jak ta). Dlatego że to ma bardzo dużo zależności - baza, widok, http, sieć, cache, stan, mnóstwo tego wszystkiego.

Jeśli na prawdę chcesz zrozumieć jak poprawnie zbudować dobry projekt, to zacznij od czegoś prostego - np. aplikacja konsolowa, która jedyne co ma to ten interfejs tekstowy oraz jakąś logikę nic więcej. Napisz ją. Ponieważ jak mówisz masz małe doświadczenie, najpewniej (na 99%) logika będzie połączona widokiem (tzn. z interfejsem konsolowym). Krok pierwszy w dobrej architekturze to oddzielić tylko te dwie rzeczy od siebie, a dokładniej zrobić tak żeby logika nie wiedziała o interfejsie. To jest najprostsze co możesz zrobić - spróbuj, a gwarantuje Ci że się dużo nauczysz.

Jak to zrobisz, to nastepnym krokiem byłoby dodać kolejną zależność do tej aplikacji - np. pliki; zrób jakąś funkcję która wymaga korzystania z plików. Znowu - najpewniej to też będzie połączone z logiką, i zacznie się z tego robić małę spaghetti. Więc teraz znowu możesz wydevelopować swoje skille architektury i oddzielić pliki od logiki i najlepiej od interfejsu też - to będzie trochę trudniejsze, bo już masz 3 moduły, zamiast dwóch. Jak to Ci się uda zrobić dobrze, to na prawdę congrats!

Jak zaczynasz od aplikacji webowej, która ma milion zależności, to można powiedzieć że od startu jesteś spisany na straty, bo musisz nimi żąglować, i musiałbyś też bardzo dobrze znać domenę w której pracujesz.

Dodatkowo - frameworki mają tą cechę że lubią przywiązywać swoich użytkowników do siebie; więc w pewnym sensie walczą z Tobą, jeśli próbujesz zrobić dobrą architekture.

Jeśli na prawdę chcesz zrozumieć jak poprawnie zbudować dobry projekt, to zacznij od czegoś prostego - np. aplikacja konsolowa, która jedyne co ma to ten interfejs tekstowy oraz jakąś logikę nic więcej. Napisz ją. Ponieważ jak mówisz masz małe doświadczenie, najpewniej (na 99%) logika będzie połączona widokiem (tzn. z interfejsem konsolowym). Krok pierwszy w dobrej architekturze to oddzielić tylko te dwie rzeczy od siebie, a dokładniej zrobić tak żeby logika nie wiedziała o interfejsie. To jest najprostsze co możesz zrobić - spróbuj, a gwarantuje Ci że się dużo nauczysz.

Jak to zrobisz, to nastepnym krokiem byłoby dodać kolejną zależność do tej aplikacji - np. pliki; zrób jakąś funkcję która wymaga korzystania z plików. Znowu - najpewniej to też będzie połączone z logiką, i zacznie się z tego robić małę spaghetti. Więc teraz znowu możesz wydevelopować swoje skille architektury i oddzielić pliki od logiki i najlepiej od interfejsu też - to będzie trochę trudniejsze, bo już masz 3 moduły, zamiast dwóch. Jak to Ci się uda zrobić dobrze, to na prawdę congrats!

Jak zaczynasz od aplikacji webowej, która ma milion zależności, to można powiedzieć że od startu jesteś spisany na straty, bo musisz nimi żąglować, i musiałbyś też bardzo dobrze znać domenę w której pracujesz.

Dodatkowo - frameworki mają tą cechę że lubią przywiązywać swoich użytkowników do siebie; więc w pewnym sensie walczą z Tobą, jeśli próbujesz zrobić dobrą architekture.

Riddle dzięki za kolejny treściwy komentarz. Co do zaproponowanego rozwiązania wpadłem na taki pomysł związany trochę z projektem który wrzuciłem do oceny.. Kreator użytkownika "Legal Person". Aplikacja będzie pytać user'a o podanie danych: name, email, phoneNumber, address, krsNumber

Postaram się to ogarnąć jak najszybciej! :)

rocess napisał(a):

Riddle dzięki za kolejny treściwy komentarz. Co do zaproponowanego rozwiązania wpadłem na taki pomysł związany trochę z projektem który wrzuciłem do oceny.. Kreator użytkownika "Legal Person". Aplikacja będzie pytać user'a o podanie danych: name, email, phoneNumber, address, krsNumber

Postaram się to ogarnąć jak najszybciej! :)

Okej, dobry pomysł.

Tylko jeśli aplikacja będzie tylko pytać o dane, i nic potem z nimi nie robić; to ona będzie się składać tylko z interfejsu (jeden moduł), więc nie będzie czego od czego oddzielać.

Jeśli chcesz zacząć ogarniać architekturę, to musisz mieć jakieś dwa moduły które można od siebie oddzielić - mógłbyś np. walidować te dane, sprawdzać czy imię, mail, numer telefonu adres pasują do jakiegoś formatu. Wtedy takie walidowanie to mógłby być jeden moduł (który nie wie nic o widoku, nic o argsach, inputach, etc), i input osobno, który nic nie robi tylko printuje i wczytuje dane, a potem przekazuje je dalej do tej części biznesowej.

Przychodzi mi na myśl taki podział:

• Klasa Main - uruchomienie aplikacji
• LegalPerson - wszystkie dane: name, email, phoneNumber, address, krsNumber - tutaj też będą settery i gettery, tutaj mam walidację robić wprowadzonych danych/inputu ? Wspominał o tym RequiredNickname w 6 punkcie - czy to się robi tylko dla klas encji pracując na bazach danych?
• LegalPersonService - czy może tutaj ta walidacja wszystkich danych wejściowch ?
• UserInterface - tu będą wszystkie instrukcje dla użytkownika.

rocess napisał(a):

Przychodzi mi na myśl taki podział:

• Klasa Main - uruchomienie aplikacji
• LegalPerson - wszystkie dane: name, email, phoneNumber, address, krsNumber - tutaj też będą settery i gettery, tutaj mam walidację robić wprowadzonych danych/inputu ? Wspominał o tym RequiredNickname w 6 punkcie - czy to się robi tylko dla klas encji pracując na bazach danych?
• LegalPersonService - czy może tutaj ta walidacja wszystkich danych wejściowch ?
• UserInterface - tu będą wszystkie instrukcje dla użytkownika.

Nie myśl o tym w kontekście klas. Napisz po prostu kod najpierw, i potem możemy spróbować zrefaktorować to.

Dołączę się do dyskusji. Mam pytanie odnośnie:

RequiredNickname napisał(a):

1. Przechowywanie password w postaci Stringa z uwagi na specyfikę obiektó typu String (string pool) nie jest ani zalecane ani bezpieczne

ponieważ sam do tej pory przechowywałem password w postaci zahaszowanego Stringa. Kontroler rejestracji przyjmuje hasło jako String. Powinienem przekonwertować String w kontrolerze na char[] i potem czyścić to hasło, czy robić to na niższych poziomach? Czy takie podejście będzie ok?

Ornstein napisał(a):

Dołączę się do dyskusji. Mam pytanie odnośnie:

RequiredNickname napisał(a):

1. Przechowywanie password w postaci Stringa z uwagi na specyfikę obiektó typu String (string pool) nie jest ani zalecane ani bezpieczne

ponieważ sam do tej pory przechowywałem password w postaci zahaszowanego Stringa. Kontroler rejestracji przyjmuje hasło jako String. Powinienem przekonwertować String w kontrolerze na char[] i potem czyścić to hasło, czy robić to na niższych poziomach? Czy takie podejście będzie ok?

Nie przyjmuj się tym. Trzymaj na razie normalnie jako string.

Odnośnie struktury projektu z pierwszego posta, aktualnie wyglądającego tak:

com.example.notarycrm
|-- configuration
| |-- RestTemplateConfig.java
|-- controller
| |-- LegalPersonController.java
| |-- MainController.java
| |-- NaturalPersonController.java
| |-- UserController.java
|-- model
| |-- LegalPerson.java
| |-- NaturalPerson.java
| |-- User.java
|-- repository
| |-- LegalPersonRepository.java
| |-- NaturalPersonRepository.java
| |-- UserRepository.java
|-- service
| |-- exceptions
| | |-- DuplicateKrsException.java
| | |-- KrsValidationException.java
| | |-- LegalPersonNotFoundException.java
| | |-- NaturalPersonNotFoundException.java
| | |-- UserNotFoundException.java
| |-- KrsValidatorService.java
| |-- LegalPersonService.java
| |-- NaturalPersonService.java
| |-- UserService.java
|-- NotarycrmApplication.java

"witualny" pomocnik pomógł wygenerować takie coś dla "package per feature" oraz zgodnie z "hexagonal architecture":

com.example.notarycrm
|-- legalperson
| |-- core
| | |-- LegalPerson.java
| | |-- LegalPersonService.java
| |-- ports
| | |-- LegalPersonRepository.java
| |-- adapters
| | |-- persistence
| | | |-- JpaLegalPersonRepository.java
| | |-- web
| | |-- LegalPersonController.java
|-- naturalperson
| |-- core
| | |-- NaturalPerson.java
| | |-- NaturalPersonService.java
| |-- ports
| | |-- NaturalPersonRepository.java
| |-- adapters
| | |-- persistence
| | | |-- JpaNaturalPersonRepository.java
| | |-- web
| | |-- NaturalPersonController.java
|-- user
| |-- core
| | |-- User.java
| | |-- UserService.java
| |-- ports
| | |-- UserRepository.java
| |-- adapters
| | |-- persistence
| | | |-- JpaUserRepository.java
| | |-- web
| | |-- UserController.java
|-- configuration
| |-- RestTemplateConfig.java
|-- exceptions
| |-- DuplicateKrsException.java
| |-- KrsValidationException.java
| |-- LegalPersonNotFoundException.java
| |-- NaturalPersonNotFoundException.java
| |-- UserNotFoundException.java
|-- NotarycrmApplication.java

Taki schemat jak powyżej ma sens ?