Witam napisałem projekt i jakby ktoś miał chwilkę czasu na CodeReview to byłbym wdzięczny za uwagi i podpowiedzi. Duży minus nie napisałem testów do projektu.
Miej litość i zrób coś z tym formatowaniem
Używasz Phpstorma? Jak nie to zacznij i skorzystaj z opcji formatowania kodu.
Ooo i po githubie kojarzę Cie z grup symfony na fb, sporo tam pytasz :) dobrze dobrze :)
W sumie fajnie by było byś opisał co robi ten projekt, na czym skupić uwagę. Możesz to wszystko opisać w jakim readme.md, bo teraz to trzeba zaglądać w Composera by zobaczyć na jakiej wersji symfony to stoi itp itd.
- Wywal z gita foldery od IDE - w tym przypadku .idea
- Node modules jest zbędny - tak samo jak vendor, którego na szczęście nie masz. Zależności powinny się dociagac z package.json i composer.json. W sumie to nawet nie wiem po co Ci ten folder w katalogu głównym i w public.
- Folder templates wpakowalbym do src i to co tam siedzi starał się opakowac w foldery. Np articleModule, commentModule i tam logicznie to porozmieszczal.
- W kodzie też bym dał trochę oddechu między metodami, bo teraz w jednej linijce kończy się metoda, a w następnej zaczyna nowa. Daj tam jednego enterka - kod będzie się czytało przyjemniej.
- Co to za kontroler? CreateNewCommentController
- Wiem że Ci tam na fb nawsadzali różnej maści porad, ale moim zdaniem nie ma sensu rozbijac się na 1 kontroler-> 1akcja. W dużych systemach masz po kilkanaście albo i więcej akcji w kontrolerach, których mogą być setki. Chcialoby Ci się potem grzebać w setkach/tysiącach plików?
Tak na szybko:
- Wrzuć folder
.idea
do.gitignore
. - Po co
.gitignore
w folderze z encjami? - Zamiast ręcznie tworzyć pola
createdAt
iupdatedAt
możesz użyć: https://github.com/KnpLabs/DoctrineBehaviors#timestampable
Patrząc pobieżnie:
-
https://github.com/kamil161g/FootbalLand/blob/master/templates/Article/article.html.twig#L43
Wstawiłbym pętle dla flash messages do bazowego szablonu, żeby w razie co pokazywało wiadomość na każdej stronie np. w divie, który będzie z position absolute i wyświetlający się na środku strony. -
Nazwy plików w template ja u siebie nazywam np. profile_show w .../Profile/, ewentualnie samo show.html.twig, edit.html.twig itp wydaje mi się ciut czytelniejsze.
-
Z tego co widzę brakuje np. https://symfony.com/doc/current/reference/constraints/Length.html nad encjami
-
Kontrolery moim zdaniem lepiej tworzyć pod dane przeznaczenie typu ArticleController z routingami do wyświetlenia, edycji itp.CommentController do edycji komentarzy, wstawiania nowych itp. Wchodzisz w src/Controller i widzisz jaki kontroler jakie ma przeznaczenie. Przy bardziej zaawansowanych projektach mozna dzielić na podfoldery np Controller/Profile i tam AdminController, UserController czy inne.
-
Tak jak wspomniano te komentarze bym pousuwał
-
https://github.com/kamil161g/FootbalLand/blob/master/src/Controller/EditArticleController.php#L30 Ciekawi mnie czemu stosujesz tu konstruktor, kiedy można przez DI wstrzyknąć do editArticle obok Request $request dodać ', UserService $userService'? Jakieś przeciwwskazanie jest ku temu?
-
https://github.com/kamil161g/FootbalLand/blob/master/src/Service/UserService.php#L32 Czemu w ogóle masz ten serwis, jak on tylko wywołuje wbudowane w doctrine find i nic poza tym? Więcej kodu, tworzenia plików niż samego działania. Dodatkowo w konstruktorze masz EntityManagera zamiast UserRepository? Po co, jak korzystasz tylko z encji Usera, nie masz operacji typu persist, flush na $em.
W przypadku serwisu taki jak wyżej to lepiej już mieć custom query poprzez 'customQueryBuilder' w repository dla odpowiedniej encji, jeśli pobranie jest jakieś bardziej skomplikowane i rzeczywiście nie chcesz używać find, findBy itd. -
Gdybyś miał w template jakiś kod, który się powtarza, warto korzystać z rozwiązań include, embedd itp w Twigu, żeby potem móc szybko coś poprawić, zamiast zmieniać w x plikach ;)
Na githubie jak masz stare projekty, jakieś mało wartościowe rzeczy itd to można korzystać z prywatnych repo zamiast trzymać to na widoku (np. masz jakiś lipny projekt sprzed 2 lat z tego co widziałem).