Cześć chciałbym prosić o ocenę kodu php, pod kątem przejrzystości i tak dalej. Starałem się zrobić coś w stylu mvc, jak mi to wyszło? Co poprawić?
Na stronce można dodawać posty, które muszą czekać na akceptacje administratora.
https://github.com/Lukasz98/ksiega
0
0
Myśle, że kod jest dobry.
0
To co mi się na szybko rzuciło w oczy: nigdy nie commituj zakomentowanego kodu. Pracując w zespole, który liczy więcej niż 1 osoba takie coś jest zmorą, zakomentowany kod zostaje i nikt nie wie co ma z nim zrobić, robi się coraz większy bałagan. Jeśli już musisz to na górze zostaw adnotację: zakomentowany, testowy do usunięcia dnia 29.03.2017
0
1.Twój kod jest podatny na atak SQL injection.
2.Twój model tak naprawdę reprezentuje wzorzec a'la repozytorium.
3.W modelu (repozytorium) nie powinno być zapisanych danych do łączenia się z bazą.
4.raczejPolaOrazWszystkoNazywajWTakiSpob_niz_taki.
5.Skoro jeden katalog nazywa się controllers (liczba mnoga), dlaczego drugi już model (liczba pojedyncza)?
6.W PHP przyjęło się nazywać pliki MojKontroler.php aniżeli mojKontroler.php.
7.Jeszcze nie rozumiesz dokładnie idei MVC - zacznij pisać w jakimś frameworku (Laravel, Symfony, Zend...), a wejdzie Ci w nawyk szybciej, niż gdybyś miał wynajdować koło od nowa.
8.Nie nazywaj pól od m_ ani innego prefiksu.
9.Twój view.php bardziej przypomina view helper niż rzeczywisty widok - zapoznaj się z pojęciem template engine.
To tak ogólnie rzucając okiem.
0
Na szybko kilka uwag ode mnie:
- brak choćby minimum komentarzy
- brak formatowania
- niezrozumiałe nazwy metod i klas (prefixy przed nazwami?)
- miszmasz php, html, js
- sql injection
Poza tym co napisałem jest jeszcze mnóstwo rzeczy których brak lub jest za dużo.
Tutaj zawsze ktoś Ci pomoże ale doradzam również prosić o weryfikację kodu tutaj: https://codereview.stackexchange.com
0
No to teraz ja :)
- Nie wiem po co te przedrostki
m_, poza tym - Jak będziesz miał więcej modeli,kontrolerów, widoków - przyda się jakiś autoloader
- Jeśli dobrze widzę - to controller.php pełni rolę routera
- Nie stosujesz klamerek przy instrukcjach warunkowych - kiedyś to się zemści :)
- Php już od dawna nie wymaga znacznika domykającego ?> - kiedyś powodowało to problemy z przedwczesnym wysyłaniem nagłówka jesli po domknięciu znajdowały się białe znaki (patrz postController.php)
- Walidacja też troszkę leży - patrz funkcja m_validate - wiąże się to też z dosyć kiepską metodą
m_ShowProblemsz widoku. - Błędy w plikach css (otwórz sobie to np w netbeansie), ale tutaj nie jestem znawcą - po prostu nigdy nie spotkałem się z tym, a ! widziałem wyłącznie przy !important
- Css - div#inp - nie ma potrzeby używania tagu i selektora - wystarczy selektor i umięjętne używanie.
- Metoda m_ShowAdminPanel w widoku - ok... przy takim projekcie.... jeszcze jakoś by uszło.... no ale po co tyle razy te echo?
- Po co te podkreślenia w zmiennych ?
$_row - Początek m_ShowFooter (widok) - pierwszy warunek nie wymaga komentarza.
- Zamiast mysqli - używaj PDO
- Widze podatności na sql injection
- Mimo że nie widzę tam projektu bazy danych - to tabele o nazwie "tab" nic nie mówią.
15.Magiczne liczby - metoda m_ShowPosts ( while ($_row && ($count < 5 || $_allPosts))) - Masz projekt w którym istnieje pseudo mvc (to nie jest zarzut) - korzystasz z klas php (czyli pretendujesz do OOP) - ale w całym kodzie masz raptem 5 returnów - rozsianych po zaledwie 4 metodach.
To tak na szybko pisałem, ale chyba jest co poprawiać.
0
axelbest napisał(a):
- Walidacja też troszkę leży - patrz funkcja m_validate - wiąże się to też z dosyć kiepską metodą
m_ShowProblemsz widoku.- ..a ! widziałem wyłącznie przy !important
- Masz projekt w którym istnieje pseudo mvc (to nie jest zarzut) - korzystasz z klas php (czyli pretendujesz do OOP) - ale w całym kodzie masz raptem 5 returnów - rozsianych po zaledwie 4 metodach.
Kod jest jeszcze ciepły więc zostało troche dziwnych komentarzy i jeśli chodzi o !w css to są właśnie moje "chwilowe" komentarze, tak wiem że to jest złe .
Co jest złego w m_ShowProblems i małej ilości returnów?
Dzięki za komentarze szczególnie o injection, bo o tym nie słyszałem. Bardziej chodziło mi jednak o poprawienie tego w kierunku mvc i ogólnego "flow" (nie pisałem zbyt wiele w php do tej pory), a nie nazewnictwa. Co do mvc to muszę doczytać.
0
-
pobierz sobie np. Symfony, zobacz jak wygląda dokładnie MVC;
-
w MVC nie łączymy php'a z html, css, js etc.;
-
jakieś dziwne taby, nazewnictwo takie sobie;
Ogarnij sobie: https://github.com/mrdoob/three.js/wiki/Mr.doob%27s-Code-Style%E2%84%A2
0
ja to nawet kodu nie widze pod tym linkiem jest tak slaby