[PHP][Symfony3] Ocena kodu gry przeglądarkowej

Question

Cześć wszystkim,
mam do was prośbę - mianowicie chodzi ocenę mojego kodu. Tworzę grę przeglądarkową przy użyciu Symfony 3, póki co dodałem tylko część rozwiązań, ale zanim zabrnę dalej, dobrze by było gdyby ktoś bardziej doświadczony rzucił na to okiem.
Jako, że to gra - sporo rzeczy dzieje się po stronie klienta (JS), ale na razie do oceny przesyłam tylko kod PHP:

Answer 1

Co to za pomysł, aby trzymać w repozytorium sam AppBundle? Jak dla mnie bez sensu. Gdzie reszta aplikacji?

Nie wiem pod jaką wersję PHP piszesz (ani nawet jaka wersja Symfony, nie ma composer.json), ale myślę ze niżej niż 7 w większości przypadków nie ma już sensu. Możesz więc wtedy zastąpić @return z phpDocumentora typem zwracanym z metody. A wszystkie array() zastąpić []. Routing moim zdaniem czytelniejszy jest w yamlu niż adnotacjach.

Wszelkie zapytania jakie są w Utils powinny być moim zdaniem w repozytoriach, od tego one są.

Nazwy commitów "code sample", "code sample no2" i "code sample no3" to raczej nie jest dobry pomysł.

Comments

Hej, dzięki za odpowiedź.

Już się tłumaczę ;)

Co to za pomysł, aby trzymać w repozytorium sam AppBundle? Jak dla mnie bez sensu. Gdzie reszta aplikacji?

Cały projekt nad którym pracuje siedzi w innym, niepublicznym repozytorium. Do publicznego, którego tu udostępniłem wrzuciłem to co uważałem za najważniejsze.

Nazwy commitów "code sample", "code sample no2" i "code sample no3" to raczej nie jest dobry pomysł.

Dlatego też samo repo nazwałem code-sample, a w samych nazwach commitów się zbytnio nie rozwijałem.
Wersję Symfony podałem w poście i tytule wątku.

Możesz więc wtedy zastąpić @return z phpDocumentora typem zwracanym z metody.

Rozumiem, że chodzi Ci np, o coś takiego:

public function example($a, $b):  array 
{
    ...
}

Wszelkie zapytania jakie są w Utils powinny być moim zdaniem w repozytoriach, od tego one są.
 

Faktycznie, wygooglałem i nawet jest to opisane w dokumentacji:
https://symfony.com/doc/3.3/doctrine/repository.html

Dzięki za radę :)

Co do routingu, dla mnie osobiście bardziej intuicyjny jest ten w adnotacjach.

Dzięki za wszystkie uwagi. Jeżeli masz jakieś jeszcze, np. odnośnie formatowania kodu itp. to wal śmiało, pozdrawiam :)

---

Cały projekt nad którym pracuje siedzi w innym, niepublicznym repozytorium. Do publicznego, którego tu udostępniłem wrzuciłem to co uważałem za najważniejsze.

Rozumiem, szło to wywnioskować po tym co napisałeś w temacie, ale to jednak trochę mało, nawet do oceny. 

Dlatego też samo repo nazwałem code-sample, a w samych nazwach commitów się zbytnio nie rozwijałem. 

Czego byś nie wrzucał to takie nazwy commitów aż rażą w oczy :p

Wersję Symfony podałem w poście i tytule wątku.

Fakt, moje niedopatrzenie, wybacz. Co nie zmienia faktu że i tak composer.json czasem wiele może powiedzieć.

Rozumiem, że chodzi Ci np, o coś takiego:

Dokładnie. Od PHP 7 można określać typ zwracany jak i typy argumentów.

Faktycznie, wygooglałem i nawet jest to opisane w dokumentacji: 

Tak, to to. W dokumentacji Symfony jest chyba wszystko.

Więcej uwag specjalnie na razie nie mam. Formatowanie kodu wygląda ok, z tego co widzę PSR-2 przestrzegasz, więc spoko.

---

Rozumiem, szło to wywnioskować po tym co napisałeś w temacie, ale to jednak trochę mało, nawet do oceny. 

To wynika z tego, że to nie jest projekt "open-source" - dlatego właśnie staram się podawać najmniej info jak mogę :)

Czego byś nie wrzucał to takie nazwy commitów aż rażą w oczy :p

Jak tak teraz na to patrzę, to chyba masz rację :P

Dokładnie. Od PHP 7 można określać typ zwracany jak i typy argumentów.

Tylko z tego co wygooglałem na szybko, można określić jeden zwracany typ (ew. dwa, ale z null), a niektóre moje metody mogą zwrócić jeden z dwóch typów, np. array lub int.

Więcej uwag specjalnie na razie nie mam. Formatowanie kodu wygląda ok, z tego co widzę PSR-2 przestrzegasz, więc spoko.

Code Sniffer robi robotę ;)

Dzięki za odpowiedź.

---

Ok, rozumiem.

Tylko z tego co wygooglałem na szybko, można określić jeden zwracany typ (ew. dwa, ale z null), a niektóre moje metody mogą zwrócić jeden z dwóch typów, np. array lub int.

Tak, to prawda. Może trzeba popatrzeć czy da się tak przebudować kod, aby zwracał wartość jednego typu ;)

---

Tak, to prawda. Może trzeba popatrzeć czy da się tak przebudować kod, aby zwracał wartość jednego typu ;)

No właśnie tak pomyślałem teraz, że to może być kwestia złej budowy metod. Popracuję nad tym :>

Answer 2

1. Nie widzę encji. Nie dołączyłeś do repo folderu Entity? 
2. Korzystaj z QueryBuildera. 
3. Nazwa Trait dla elementu logiki jest bardzo myląca, biorąc pod uwagę fakt, że jest to konstrukcja języka.
4. Korzystaj z tego co już jest np. Fosuserbundle. Jeśli masz bardzo rozwiniętą logikę z potencjałem do rozwoju, to polecam MigrationsBundle.
5. json_decode ma drugi argument. 

6. $item_for_sale. To nie wygląda dobrze. 

7. Bez PHP7? ​​​​​​
8. Ten switch ze stringami wygląda smutno. Instanceof byłoby bardziej przyszłościowe.

Ogólnie bardzo spoko, daj znać jak będzie jakaś alfa/beta.  

Comments

Nie widzę encji. Nie dołączyłeś do repo folderu Entity? 

Nie dodałem. To wynika z tego, że to nie jest projekt "open-source" - dlatego właśnie staram się podawać najmniej info jak mogę :)

Korzystaj z QueryBuildera. 

Korzystam z obu funkcji. Z createQuery() przy prostych zapytaniach np. proste selecty, a z createQueryBuilder() przy bardziej złożonych, np. zapytania typu join czy złożone update. Jaki jest powód aby przerzucić się tylko na createQueryBuilder()?

Nazwa Trait dla elementu logiki jest bardzo myląca, biorąc pod uwagę fakt, że jest to konstrukcja języka.

Faktycznie, niezła wpadka :P

Korzystaj z tego co już jest np. Fosuserbundle. Jeśli masz bardzo rozwiniętą logikę z potencjałem do rozwoju, to polecam MigrationsBundle.

Na początku używałem FOSUserBundle, ale gdy chciałem go zmodyfikować wg. własnych potrzeb (dodawanie rekordów podczas rejestracji do trzech różnych tabel jednocześnie) to mi się odechciało :) MigrationsBundle nie znam, ale się zapoznam.

json_decode ma drugi argument. 

Ale czy jego podawanie na pewno jest konieczne? Bo nawet w przykładach w dokumentacji jest często pomijany:  http://php.net/manual/en/function.json-decode.php

$item_for_sale. To nie wygląda dobrze. 

O kurde, jakim cudem to się uchowało? :) Myślałem, że wszystkie kwiatki tego typu poprawiłem.
 

Bez PHP7? ​​​​​​

Przepiszę kod na PHP 7 ;)

Ten switch ze stringami wygląda smutno. Instanceof byłoby bardziej przyszłościowe.

Hmm, rozumiem jak użyć instanceof wraz ze switchem. Natomiast nie wiem jak to ugryźć z innej strony. Tzn. musiałbym stworzyć klasy "armorer", "blacksmith" itd. których nie ma, a nie widzę potrzeby ich tworzenia dla samego switcha. Czy źle Cię zrozumiałem?

Ogólnie bardzo spoko, daj znać jak będzie jakaś alfa/beta.  

Dzięki, na pewno dam znać :)