Prośba i code review Symfony prosty crud

0

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 :)

1

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 :)

1

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.

0

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

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

0

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.

0
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ść ?

0

Kolejne poprawki dodane na githuba

1

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.

1

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.

1

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.

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