Prośba i code review Symfony prosty crud

Witam, od niedawna zacząłem się uczyć symfony 3. Stworzyłem sobie jakiś tam kod ale nie wiem czy nie robię rażących błędów. Wiec zwracam się do was o pomoc.
Kod: https://github.com/zawiszaty/symfonyBooks
Z góry dziękuje wszystkim osobom za pomoc :)
Trochę poprawiona wersja oddana na githuba :)

tak po kilku sekundach spojrzenia na kod :

PSR-y
jaki php ? jeśli 7,x to typowanie , bez annotacji
akcje dotyczące jednej encji w jednym kontrolerze

reszta jak poprawisz :)

Zmorą początków oop w php było pakowanie wszystkiego do 1/2 kontrolerów. Teraz widzę tendencja odwórciła się, jak mawiają politycy, o 360st.

Kontrolery mógłbyś ograniczyć do 5.
PSR w wielu miejscach leży.
Używasz php 7.1 a z typowaniem słabo.
Ubogie adnotacje dla routingu.
Używanie translacji od samego początku może być w przyszłości pomocne.
Wyrzuć .idea z projektu

Pozbijaj te kontrolery bo ludziom nie będzie się chciało otwierać kilkunastu plików żeby zrobić review jednej akcji w każdym z nich.

Skąd pomysł, żeby robić klasy z jedną metodą pod tytułem indexAction?

1. Jak już rozbijasz to umieść je w odpowiednim namespace np. zamiast AppBundle/Controller/AddAuthorController.php lepiej będzie AppBundle/Controller/Author/AddController.php
2. Brak testów.
3. Do CRUD jest easyadmin - wtedy wystarczą same Entity w src.

Markuz napisał(a):

1. Jak już rozbijasz to umieść je w odpowiednim namespace np. zamiast AppBundle/Controller/AddAuthorController.php lepiej będzie AppBundle/Controller/Author/AddController.php
2. Brak testów.
3. Do CRUD jest easyadmin - wtedy wystarczą same Entity w src.

3 easy admin nie zawsze jest pomocny.. zwłaszcza, jak chcesz wykonać coś niestandardowwego albo dodać jakąś opcję, wtedy przerabianie templatu i nadpisywanie kontrolerów jest bardzo męczące. O dziwo Easy admin nadal nie ma bulkAction co jest niewiarygodne przy crudzie...
z drugiej strony jest sonata która konfiguracyjnie i dokumentacją leży i kwiczy. Tragedia.

wg mnie kontrolery robią za dużo. Wyobraź sobie, że chcesz zrobić dodawanie autora z linii komend albo poprzez API. Co wtedy zrobisz? Kopiuj-wklej :) a co jeśli później będziesz chciał dodać jakieś pole do autora? będziesz miał zmianę w 3 miejscach zamiast w 1.

Dziedziczenie po jakimś innym Bundle, w Twoim wypadku FOSUserBundle powinno robić się w ostateczności. Wyobraź sobie, że teraz z jakichś względów MUSISZ zrobić dziedziczenie po innym bundle? Wtedy *** zbita. Najlepiej tego w ogóle nie rób.

no_solution_found napisał(a):

wg mnie kontrolery robią za dużo. Wyobraź sobie, że chcesz zrobić dodawanie autora z linii komend albo poprzez API. Co wtedy zrobisz? Kopiuj-wklej :) a co jeśli później będziesz chciał dodać jakieś pole do autora? będziesz miał zmianę w 3 miejscach zamiast w 1.

Dziedziczenie po jakimś innym Bundle, w Twoim wypadku FOSUserBundle powinno robić się w ostateczności. Wyobraź sobie, że teraz z jakichś względów MUSISZ zrobić dziedziczenie po innym bundle? Wtedy *** zbita. Najlepiej tego w ogóle nie rób.

@SekretarzGeneralnyONZ: @no_solution_found zacząłem rozdzielać logikę od kontrolerów. I teraz nie wiem czy to dobrze robię. Wyciągam logikę i rejestruję ją jako nowy serwis. Do tego zauważałem ze np doctrine muszę przekazać do tego serwisu , wiec przekazuje to jako obiekt. Jednak wydaje mi sie ze jak bede chcial coś jeszcze dodać to w końcu dojdzie ze będę musiał przekazywać po 10 obiektów, wiec pewnie trzeba użyć jakiegoś kontenera którego będę sobie przekazywać miedzy serwisami. Tylko nie wiem jak to zrobić ? Sam muszę zrobić taki jeden zbiorczy serwis czy symfony ma wbudowana taka funkcjonalność ?

Kolejne poprawki dodane na githuba

Takie rozbicie na klasy z pojedynczymi metodami w katalogu Utils chyba nie za bardzo jest ok. Np. AuthorLogic. Wydaje mi sie, ze powinieneś mieć jedną klasę, np. User.php, w ktorej masz metody add(), delete(), etc.., a nie osobne klasy do każdej metody.

Nie wiem jakiego IDE używasz ale polecam zintegrować je z Php Code Snifferem. Przykładowo PhpStorm jest z takimi narzedziami fajnie zintegrowany.
PhpCS zwróci Ci uwagę na wiele niezgodności ze standardami kodowania, które masz w kodzie.

Kolejna uwaga, bardziej rzecz kosmetyczna.
Chodzi mi o nazwy klas encji. Mianowicie nazwa klasy encji nie powinna być w liczbie mnogiej, a pojedynczej.
W bazie danych tabela books przechowuje wszystkie książki. Ale jedna encja klasy Book jest instancją tylko jednej z tych książek.
O wiele czytelniej wygląda to w sytuacji, gdy w jakiejś metodzie wskazujesz, że zmienna ma mieć typ Book, np. $someMethod(Book $someBook) {...}

Jeszcze jedna uwaga odnośnie commitów. Jak robisz commit to daj sobie do niego jakiś sensowny opis, żebyś później sam widział np. w PhpStormie lub SourceTree (czy jeszcze czym innym) historię zmian od razu z opisem co w konkretnym commicie się zmieniło.
Takich rzeczy najlepiej uczyć się od razu, nawet jeżeli uważasz, że teraz to pierdoła i się nie przyda.