CodeReview jakby ktoś miał chwilkę :)

0

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.

https://github.com/kamil161g/FootbalLand

1

Miej litość i zrób coś z tym formatowaniem Screenshot_20190311-211848_Chrome.jpg

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.

  1. Wywal z gita foldery od IDE - w tym przypadku .idea
  2. 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.
  3. Folder templates wpakowalbym do src i to co tam siedzi starał się opakowac w foldery. Np articleModule, commentModule i tam logicznie to porozmieszczal.
  4. 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.
  5. Co to za kontroler? CreateNewCommentController
  6. 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?
1

Tak na szybko:

  1. Wrzuć folder .idea do .gitignore.
  2. Po co .gitignore w folderze z encjami?
  3. Zamiast ręcznie tworzyć pola createdAt i updatedAt możesz użyć: https://github.com/KnpLabs/DoctrineBehaviors#timestampable
0

Patrząc pobieżnie:

  1. 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.

  2. 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.

  3. Z tego co widzę brakuje np. https://symfony.com/doc/current/reference/constraints/Length.html nad encjami

  4. 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.

  5. Tak jak wspomniano te komentarze bym pousuwał

  6. 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?

  7. 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.

  8. 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).

1 użytkowników online, w tym zalogowanych: 0, gości: 1