Prośba o code review - chat napisany w Symfony

Prośba o code review - chat napisany w Symfony
WB
  • Rejestracja:ponad 7 lat
  • Ostatnio:ponad 5 lat
  • Postów:7
0

Proszę o code review, jest to moja druga aplikacja napisana w Symfony.
Jest to prosty chat, początkowo miałem używać w nim PHP 7.1, ale ostatecznie musiałem użyć wersji 7. (Jedyna różnica to, że stałe w konfiguracji byłyby zapisane jako private)
chat
Starałem się napisać go najlepiej jak potrafię i nie popełniać tych samych błędów co przy pierwszym projekcie, ale na pewno jakieś uwagi ktoś znajdzie.
Z góry dzięki ;)

edytowany 1x, ostatnio: Wojciech Basiński
Burdzi0
  • Rejestracja:około 9 lat
  • Ostatnio:10 dni
  • Lokalizacja:Futurama
  • Postów:887
1

Nie piszę w PHP, ale rzuciłem okiem na przykład. Błędy:

  • screenshot-20170925131617.png
  • brak HTTPS przy rejestracji (nie wiem jak logowanie) - hasło można podejrzeć zwykłym snifferem
  • screenshot-20170925131903.png Troszku się rozjechało

Poza tym layout trochę ascetyczny, ale nawet przyjemny.


Bite my shiny metal ass!
Life throws you an error code like that, you don't have the luxury of a ZnVja2luZw== pop-up explanation *Robię projekty studenckie, pisz priv ;) *
WB
Co do recaptchy, to próbowałem ją zaimplementować z marnym skutkiem, widocznie nie wyczyściłem dokładnie pliku przed wrzuceniem na serwer, nie powinno jej być.
axelbest
  • Rejestracja:ponad 17 lat
  • Ostatnio:dzień
  • Lokalizacja:Warszawa
  • Postów:2252
1
  1. Wywal folder .idea z repo
  2. Dlaczego masz w encji message userId oraz userInfo?
  3. Metoda createArrayToJson - zwraca arraya - to co w nazwie robi JSon?
  4. O tym czy wiadomość jest skasowana mówi jej treść? - w repository na podstawie tego pobierasz. Nie lepiej jest dać flagę is_deleted?
  5. Co się stanie jak wielokrotnie wywołam metodę addOnlineUserAction z securityControllera?
  6. W ww. metodzie posługujesz się metodą getUser() - to jakaś domyślna metoda? (nie robię często przy SF)
WB
2. Do relacji - zapisuję sobie dane użytkownika, który napisał daną wiadomość. 3. Zwraca tablicę, która jest potem wysyłana do odpowiedzi w formacie json. To akurat może być trochę mylące, zmienię nazwę. btw. jest możliwość wysłania entity do jsona bezpośrednio bez zamiany na tablicę? 4. Wiadomość jest kasowana z bazy, a dodatkowa wiadomość jest po to, aby u każdego użytkownika online dana wiadomość się skasowała.
WB
5. Przekieruje Cię z powrotem na chat, ponieważ jest tam sprawdzenie czy dany użytkownik jest zapisany do tabeli w bazie. W bazie mam ustawienie, żeby id użytkownika był unikalny, gdyby nie sprawdzanie, to wyskoczyłby wyjątek, że dany id już istnieje. 6. To jest metoda dziedziczona z Controllera, a ta z kolei pobiera usera z TokenStorage.
axelbest
2. No ale skoro masz usera do relacji - to po co Ci jeszcze raz trzymać jego id? 3. Czyli ta metoda nie robi nic z JSON'em Pobaw się lepiej chatem na socketach :)
grzesiek51114
grzesiek51114
Wszedłem ale ni ma z kim pogadać.
SG
  • Rejestracja:ponad 8 lat
  • Ostatnio:ponad 5 lat
  • Postów:103
1

createArrayToJson() ma rzeczywiście niefortunną nazwę. Dodatkowo ta w Message Entity nie jest najlepiej zrobiona. 5 z 7 elementów tej tablicy ma takie same wartości. Nie ma potrzeby pisania ich dwa razy. Dodatkowo ten warunek if ($textSplitted[0] == '/delete') też najlepiej nie wygląda.

W MessageRepository::getIdFromLastMessage() użyłeś getResult() dlaczego nie getSingleResult? W tym samym pliku operacje na dacie mógłbyś przenieść do czegoś osobnego (może trait?) rzuca się w oczy, że jeszcze się to przyda w tym pliku i szkoda się tak powtarzać.

SG
jeszcze jedno. lada moment wychodzi Symfony 3.4 lifecycle wygląda bardzo interesująco.
WB
"dlaczego nie getSingleResult" A jest jakakolwiek różnica, skoro i tak będzie tylko jeden rekord z bazy? W jakim folderze trzymać traity? Nie używałem ich jeszcze w Symfony.
SG
Twoją intencją jest pobranie jednego rekordu a następnie operacja na obiekcie nie na kolekcji. Po to właśnie jest getSingleResult. Wyciąganie zawesze tylko pierwszego eleementu tablicy wygląda słabo.
SG
Co do lokalizacji traitów. Warto zrobić sobie osobny katalgo Traits i tam je trzymać. Takie minimum to osobny katalog z traitami dla każdego bundla czyli u Ciebie AppBundle\Traits\DefaultTrait.php Niektórzy praktykują również osobny katalog z traitami dla poszczególnych sekcji w bundlu. Czyli np. AppBundle\Entity\Traits\DefaultTrait.php - trait używany tylko w encjach.
1

Xss,
niepotrzebne komentarze np.

Kopiuj
class AdminPanel
{
    /**
     * AdminPanel constructor.
     *
     * @param EntityManagerInterface $em
     */
    public function __construct(EntityManagerInterface $em)
    {
        $this->em = $em;
    }

Zarejestruj się i dołącz do największej społeczności programistów w Polsce.

Otrzymaj wsparcie, dziel się wiedzą i rozwijaj swoje umiejętności z najlepszymi.