• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Pierwsza aplikacja Symfony - prośba o ocenę kodu [to do list]

0 głosów
317 wizyt
pytanie zadane 20 lipca 2017 w PHP przez demotywatorking Obywatel (1,210 p.)
edycja 20 lipca 2017 przez demotywatorking

Skończyłem właśnie swoją pierwszą aplikację w Symfony. Jest to prosta aplikacja typu lista rzeczy do zrobienia. Starałem się upchnąć jak najwięcej rzeczy do niej, dlatego proszę o ocenę - nie chcę powtarzać swoich błędów przy kolejnych projektach.

Link do projektu: To-Do List

3 odpowiedzi

+4 głosów
odpowiedź 21 lipca 2017 przez Ehlert Ekspert (205,710 p.)
edycja 21 lipca 2017 przez Ehlert
  1. Masz komentarze dodane przez PHPStorma. Z tym oddajesz kod do oceny?
  2. Standard PSR-2 wymaga \n po namespace,  i po dyrektywach use. Ogólnie mało \n co zmniejsza czytelność. 
  3. Controller jest warstwą dość bliską dla użytkownika. U Ciebie jest w nim QueryBuilder. Używaj serwisów!
  4. Nie używasz serwisów? Ok, ale puste klasy repo i query w kontrolerze to jakaś pomyłka. Po to są klasy repo żeby tam definiować kwerendy itp.
  5. Dokumentacja niezgodna z PHPDocs.
  6. Dlaczego nie php7?
  7. Column name lower case please? 
  8. Zero testów. Niewybaczalne. 
  9. W szablonie nazywasz body blok który nie tyczy się znacznika body. 
  10. /todo dostępne dla użytkownika, /todo/setlang dla każdego? Trzymaj się jakiś założeń. 
  11. Writing useless git commit messages? 

To było tak na szybko. Jak znajdę coś więcej dam znać. Sorry za błędy, niepisane na trzeźwo blush

1
komentarz 21 lipca 2017 przez Fenix Nałogowiec (26,850 p.)
". Sorry za błędy, niepisane na trzeźwo " i to rozumiem ^^
komentarz 21 lipca 2017 przez demotywatorking Obywatel (1,210 p.)

Dzięki za ocenę.

A tu kilka rzeczy ode mnie, na wszelki wypadek, żeby mieć pewność, że wszystko dobrze zrozumiałem.

Ad.1. Zupełnie o tym nie pomyślałem.

Ad.2. To może być wina źle ustawionego PHPStorma:

https://image.prntscr.com/image/rKQ2LwMGTYaCZS73fIrs4A.png

lub gita: https://image.prntscr.com/image/KxlyHi2wQpSODKsx9bYiEg.png ? Nie do końca wiem jak to ustawić.

Ad. 3. http://symfony.com/doc/current/service_container.html Chodzi o to?

Ad.4. Czyli wszędzie gdzie mam korzystać z findBy... to wypadałoby to wrzucić do repozytorium? Dobrze rozumiem?

Ad.8. Nie wiedziałem, że to aż tak ważne, ja zawsze wszystko ręcznie testowałem, tak jak i w tym przypadku.

Ad.11. Pierwszy raz używam gita, w dodatku nie używałem go od początku tego projektu i nie wiem do końca co tam napisać. Muszę się z tego podszkolić :)

+3 głosów
odpowiedź 21 lipca 2017 przez Assasz Nałogowiec (30,490 p.)

To ja trochę napiszę o Twoim kontrolerze TodoController:

/**
     * @Route("/todo/add", name="add")
     */

Te nazwy routów mało mówią, raczej powinno być np. 'add_todo'. W przypadku małej aplikacji to jeszcze nie problem, ale wydaje mi się, że lepiej wyrabiać sobie dobre nawyki.

if (!$todo) {
            return $this->render('details.html.twig', [
                'todo' => ''
            ]);
        }

Osobiście rzuciłbym tutaj błąd 404, zamiast przekazywać pustą zmienną.

$request->getSession()
                ->getFlashBag()
                ->add('success', 'Z powodzeniem edytowano zadanie.')

Można to zrobić krócej, $this->addFlash('success', 'Z powodzeniem edytowano zadanie.');

Oprócz tego wydaje mi się, że setLangAction oraz indexAction powinny być w oddzielnym kontrolerze, bo akcje te nie mają za dużo wspólnego z todo. addChoicesToForm już zupełnie tu nie pasuje.

komentarz 21 lipca 2017 przez demotywatorking Obywatel (1,210 p.)
Dzięki za ocenę.

Będę pamiętać o nazwach routów.

Co do wyjątku z błędem 404, to o nim myślałem na początku.

addChoicesToForm to powinna być w repozytorium w ogóle, teraz to widzę jak powinno być.

A co w wypadku jakby mi była potrzebna taka metoda, która coś ma zrobić, ale być niedostępna z zewnątrz, czyli być prywatną? Mam ją dać do innego kontrolera czy mam to napisać jako serwis jak napisał Ehlert na górze?
komentarz 21 lipca 2017 przez Assasz Nałogowiec (30,490 p.)
To w sumie zależy od tego, co dokładnie robi ta metoda i jak jest wykorzystywana. W Twoim przypadku wydaje mi się, że można by taką metodę umieścić w TodoType jako metodę prywatną i używać jej bezpośrednio w TodoType przy tworzeniu formularza. W każdym razie takie coś nie powinno się znajdować w kontrolerze - kontroler ma za zadanie wyłącznie przetworzyć żądanie, a następnie przygotować i wysłać odpowiedź do klienta.
0 głosów
odpowiedź 1 sierpnia 2017 przez demotywatorking Obywatel (1,210 p.)

Kod został przeze mnie sfaktoryzowany na tyle ile umiałem.

Mógłby ktoś zajrzeć jeszcze raz już po zmianach? :)
to-do-list

Podobne pytania

+1 głos
1 odpowiedź 166 wizyt
pytanie zadane 20 sierpnia 2017 w PHP przez Smatix Obywatel (1,030 p.)
0 głosów
2 odpowiedzi 177 wizyt
pytanie zadane 29 sierpnia 2017 w HTML i CSS przez DraveS Początkujący (300 p.)
+1 głos
4 odpowiedzi 846 wizyt
pytanie zadane 1 lipca 2017 w HTML i CSS przez DraveS Początkujący (300 p.)

86,541 zapytań

135,291 odpowiedzi

300,649 komentarzy

57,288 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto dwie polecane książki warte uwagi. Pełną listę znajdziesz tutaj.

...