[www] Prosta sieć reklamowa - Symfony i Framework w PHP |
[www] Prosta sieć reklamowa - Symfony i Framework w PHP |
7.04.2020, 10:44:14
Post
#1
|
|
Grupa: Zarejestrowani Postów: 82 Pomógł: 0 Dołączył: 3.08.2017 Ostrzeżenie: (0%) |
Chciałbym prosić o ocenę kodu programowego (PHP + MySQL) mojej bardzo prostej aplikacji internetowej (sieć reklamowa). Całość napisana w dwóch wersjach (Symfony i własny szkielet aplikacji w PHP).
Aplikacja w Symfony + biblioteka do obsługi Api: https://github.com/webeeq/symfony.eeq https://github.com/webeeq/sieciq Własny szkielet tej samej aplikacji w PHP: https://github.com/webeeq/framework.eeq Obydwa projekty poddawałem już ocenie jakieś dwa lata temu, ale nie widzę nigdzie tamtego wątku, więc tworzę nowy. Pewnie został skasowany. Obecne wersje zawierają liczne poprawki. Prosiłbym zwrócić szczególną uwagę na zagadnienia: 1. Tworzenie klas encji tabel bazy danych w Symfony. Czy dobrze to robię? Chodzi mi głównie o brak deklaracji opcji "default". 2. Jakie wzorce projektowe poza Dependency Injection powinienem użyć w moich projektach. Jakie wzorce warto jeszcze znać? 3. Czy zapytania SQL (w cudzysłowie '...') mogą być w wielu liniach. Nie powoduje to błędu, ale czy to ładnie tak programować? 4. Czy z takim programowaniem, jak moje, jest sens w ogóle starć się o pracę? Bądź lepiej będzie, jeśli dam sobie z tym spokój? |
|
|
7.04.2020, 11:01:06
Post
#2
|
|
Grupa: Moderatorzy Postów: 36 523 Pomógł: 6309 Dołączył: 27.12.2004 |
$code = ($_GET['code']) ? (int) $_GET['code'] : 404;
po co ten pierwszy nawias? $code = $_GET['code'] ? (int) $_GET['code'] : 404; A jakbys przesiadl sie na php7 to juz w ogole bajka: $code = (int) ($_GET['code'] 404); nie $code == 403 a $code === 403 naucz sie uzywac === Plik index.php i ten zajebiscie duzy switch...case no wola o pomste do nieba Nie tworz takich kobyl, robijaj rzeczy na serwisy, klasy user_admin` TINYINT(1) NOT NULL DEFAULT '0', Jak cos jest liczba, to nie ustalaj temu czemus domyslnej wartosci jako tekst. Liczby to liczby, tekst to tekst. Wywal te (1) z tinyint i inne podobne przy typach liczbowych bo to nie sluzy ograniczeniu zakresu liczby... ten kod if (!$editUserModel->isUserId($id, $user)) { $editUserModel->dbClose(); header('Location: ' . $config->getUrl() . '/logowanie'); exit; } powielasz w wielu kontrolerach po co takie // src/Controller/ShowSiteController.php wstawki w kontrolerach? Czemu to sluzy procz faktu ze niczemu? W kazdym kontrolerze masz ->dbConnect() ->dbClose(); Cala masa zbednego i zduplikowanego kodu I wiele wiele wiecej, nie mam czasu przegladac Cytat Czy z takim programowaniem, jak moje, jest sens w ogóle starć się o pracę? Bądź lepiej będzie, jeśli dam sobie z tym spokój? Ludzie sie staraja o prace z gorszymi kodami i mniejsza wiedza public function getSiteData( int $site, ?int &$site_visible, ?string &$site_name, ?string &$site_url Unikaj taki potworkow z referencjami. Jak funkcja ma zwracac dane, to zwracaj je jako tablica a nie przez referencje return ($this->dbFetchArray($result)) ? true : false; wystarczy return (bool) $this->dbFetchArray($result); CityListModel ktory jest katalagu model/Ajax Nie tworz modeli pod akcje. Model to model, akcja to akcja. Model moze byc odpalony w kazdej akcji nie "<?php echo $array['www']; ?> a: "<?= $array['www'] ?> Mamy juz XXi wiek -------------------- "Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista "Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer |
|
|
7.04.2020, 11:02:38
Post
#3
|
|
Grupa: Zarejestrowani Postów: 6 375 Pomógł: 1116 Dołączył: 30.08.2006 Ostrzeżenie: (0%) |
Mnie się jeszcze w oczy rzuciła sama struktura. Wszystko wrzucone do folderu / wliczając cssy i src. Ani to bezpieczne, ani uporządkowane. W src jest Config/cośtam. Dlaczego to zapisałeś w ten sposób zamiast stworzyć oddzielny folder poza strukturą src? Osobiście nie podoba mi się definiowane w każdej akcji layout itd (które oczywiście odnosi się do src).
PS. $code = ($_GET['code']) ? (int) $_GET['code'] : 404; Ja też tak piszę. Ma to zapobiec przypadkowemu złemu warunkowaniu w operatorze ternary. Ten post edytował viking 7.04.2020, 11:03:50 -------------------- |
|
|
7.04.2020, 11:13:32
Post
#4
|
|
Grupa: Moderatorzy Postów: 36 523 Pomógł: 6309 Dołączył: 27.12.2004 |
Klasa Param to cala prawie do wyrzucenia. Jaki jest jej sens w ogole?
Czemu konfig bazy danych znajduje sie w repo? A co jak inna osoba ma inne dane do bazy niz ty? Niby uzywasz jquery a potem i tak do ajaxa masz function ajaxData(id, file) { if (window.XMLHttpRequest) { xmlhttp = new XMLHttpRequest(); } else { xmlhttp = new ActiveXObject("Microsoft.XMLHTTP"); } xmlhttp.onreadystatechange = function() { if (xmlhttp.readyState == 4 && xmlhttp.status == 200) { document.getElementById(id).innerHTML = xmlhttp.responseText; } } xmlhttp.open("GET", file, true); xmlhttp.send(); } to po co te jquery? Cytat PS. $code = ($_GET['code']) ? (int) $_GET['code'] : 404; Ja też tak piszę. Ma to zapobiec przypadkowemu złemu warunkowaniu w operatorze ternary. Jakis przyklad takowego przypadkowego zlego warunku?zas co do TINYINT(1) to najlepsze jest to ze miesiac temu o to pytales. Wyciagasz w ogole jakas nauke z naszych odpowiedzi czy walimy w powietrze wszystko? -------------------- "Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista "Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer |
|
|
7.04.2020, 11:45:59
Post
#5
|
|
Grupa: Zarejestrowani Postów: 6 375 Pomógł: 1116 Dołączył: 30.08.2006 Ostrzeżenie: (0%) |
Nawet z dokumentacji echo (true?'true':false?'t':'f'); Ogólnie wyrobiłem sobie nawyk nawiasowania zawsze chociaż to nadmiarowe. Zresztą w dokumentacji też używają nawiasu do przykładu z empty.
-------------------- |
|
|
7.04.2020, 11:48:52
Post
#6
|
|
Grupa: Moderatorzy Postów: 36 523 Pomógł: 6309 Dołączył: 27.12.2004 |
Mimo wszystko podany przez ciebie przyklad z dokumentacji to jednak troche inna sytuacja niz omawiany przypadek z kodu autora. O ile to podane przez ciebei tu echo jestem jeszcze w stanie strawic, to tamten zbedny nawias pierwszego kodu, by nie przeszedl code review.
-------------------- "Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista "Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer |
|
|
7.04.2020, 11:50:04
Post
#7
|
|
Grupa: Zarejestrowani Postów: 6 375 Pomógł: 1116 Dołączył: 30.08.2006 Ostrzeżenie: (0%) |
CR są per firma. U mnie by nie przeszło bez nawiasu + cs fixer dodający spacje.
-------------------- |
|
|
7.04.2020, 11:52:05
Post
#8
|
|
Grupa: Moderatorzy Postów: 36 523 Pomógł: 6309 Dołączył: 27.12.2004 |
Cytat cs fixer dodający spacje. Przynajmniej w jednym sie zgadzamy
-------------------- "Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista "Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer |
|
|
7.04.2020, 15:22:17
Post
#9
|
|
Grupa: Zarejestrowani Postów: 82 Pomógł: 0 Dołączył: 3.08.2017 Ostrzeżenie: (0%) |
zas co do TINYINT(1) to najlepsze jest to ze miesiac temu o to pytales. Wyciagasz w ogole jakas nauke z naszych odpowiedzi czy walimy w powietrze wszystko? Myślałem, że dla typów logicznych ten (1) ma znaczenie. W całej reszcie zrobiłem bez liczb w nawiasach. W Symfony 4.4 używają TINYINT(1) dla logicznych: https://github.com/webeeq/symfony.eeq/blob/...00326095423.php Cytat nie $code == 403 a $code === 403 naucz sie uzywac === Czy używanie dokładnego przyrównania jest bezwzględnie wymagane, jeśli nie przyrównuję do zera lub wartości logicznej? W dokumentacji chyba robią zamiennie. Cytat Plik index.php i ten zajebiscie duzy switch...case no wola o pomste do nieba Nie tworz takich kobyl, robijaj rzeczy na serwisy, klasy Ten plik index.php odpala kontrolery w oparciu o parametr wywołanej strony. Kontrolery natomiast odpalają serwisy aplikacji. Jak to mogę zrobić lepiej i prościej? Cytat po co takie // src/Controller/ShowSiteController.php wstawki w kontrolerach? Czemu to sluzy procz faktu ze niczemu? Takie komentarze dodawali w dokumentacji Symfony, więc też tak robiłem. W sumie nie sprawdzałem, czy robią to nadal. I faktycznie nie ma to większego sensu. Cytat W kazdym kontrolerze masz ->dbConnect() ->dbClose(); Cala masa zbednego i zduplikowanego kodu Wzorowałem się na Symfony z tymi kontrolerami. Jedna podstrona = jeden kontroler. Jak to mogę zrobić lepiej, aby nie duplikować tego kodu w kontrolerach? Cytat Unikaj taki potworkow z referencjami. Jak funkcja ma zwracac dane, to zwracaj je jako tablica a nie przez referencje Co zrobić, gdy metoda zwraca tablicę z wynikiem zapytania do bazy i dodatkowo chcę zwrócić (zmienić) jakąś wartość dla parametru, który i tak muszę podać? Cytat CityListModel ktory jest katalagu model/Ajax Nie tworz modeli pod akcje. Model to model, akcja to akcja. Model moze byc odpalony w kazdej akcji Czyli jak w Symfony? Oddzielny model dla zapytań do tabeli `users` i oddzielny do `sites`? Tylko jak realizować połączenia z bazą danych? W konstruktorach? Kurczę. Napisałem się jak głupi, a tu błąd, że zbyt dużo cytatów. I nie mogę reszty dodać... |
|
|
7.04.2020, 15:50:08
Post
#10
|
|
Grupa: Moderatorzy Postów: 36 523 Pomógł: 6309 Dołączył: 27.12.2004 |
Przestan tak bezgranicznie patrzec na dokumentacje a juz szczegolnie przestan patrzec na elementy z tla w dokumentacji gdy czytasz o rozwiazaniu X. Dokumentacja moze byc troche nie swieza, moze miec blad itp.
tinyint to nie jest typ logiczny. Nie sluzy tylko do przechowywania 0 i 1. (LICZBA) - obowiazuje kazdego typu INT. Miales to wyjasnione miesiac w temu w oddzielnym temacie. Wiec tak, przestan stosowac te liczby w nawiasie bo one nic nie robia. Robia dopiero jak uzyjessz ZEROFILL - tyle w temacie === masz uzywac zawsze. Nie wazne ze gdzies tam przy opisie funkcji X uzyli ==. Oni opisywali funkcje X a nie dzialanie ==/=== === zaoszczedzi ci mase problemow w przyszlosci index.php i duzy switch - ok, ale index.php nie musi wiedziec, ze kontroler x potrzewbuje parametroe x,y,z by dzialac. To kontroler/service ma to wiedziec i o oto dbac a nie index.php W dokumnetacji symfony dodaja nazwe pliku jako komentarz bys wiedzial gdzie ten kod wstawic. W swoim edytorze czy na git juz wiesz w jakim pliku jestes. Informacje ze plik x.php to plik x.php jest zbedna Cytat Co zrobić, gdy metoda zwraca tablicę z wynikiem zapytania do bazy i dodatkowo chcę zwrócić (zmienić) jakąś wartość dla parametru, który i tak muszę podać? Zasad jednej odpowiedzialnosci. Jak funcja zwraca dane z bazy to ma zwracac dane z bazy i tyle.Cytat Czyli jak w Symfony? Oddzielny model dla zapytań do tabeli `users` i oddzielny do `sites`? Tak. I model nie powinieniec dziedziczyc po database. On powinien to miec wstrzykniete i jego nie powinno obchodzic startowanie i zamykanie polaczenia
-------------------- "Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista "Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer |
|
|
7.04.2020, 16:01:19
Post
#11
|
|
Grupa: Zarejestrowani Postów: 82 Pomógł: 0 Dołączył: 3.08.2017 Ostrzeżenie: (0%) |
Cytat Mnie się jeszcze w oczy rzuciła sama struktura. Wszystko wrzucone do folderu / wliczając cssy i src. Ani to bezpieczne, ani uporządkowane. W src jest Config/cośtam. Dlaczego to zapisałeś w ten sposób zamiast stworzyć oddzielny folder poza strukturą src? W sumie faktycznie mógłbym umieścić kod css i js w oddzielnym katalogu np. public. Tylko że ostatnia linijka kodu w pliku .htaccess zabezpiecza przed dziurami. https://github.com/webeeq/framework.eeq/blo...aster/.htaccess Cytat Osobiście nie podoba mi się definiowane w każdej akcji layout itd (które oczywiście odnosi się do src). Wzorowałem się na framework'u Symfony. Tam jest, dla każdej akcji, renderowany szablon w oparciu o plik twig. Jak to mogę uprościć lub zrobić jeszcze lepiej? https://github.com/webeeq/symfony.eeq/blob/...mController.php https://github.com/webeeq/symfony.eeq/blob/...FormService.php Cytat Klasa Param to cala prawie do wyrzucenia. Jaki jest jej sens w ogole? Zabezpiecza przed atakami. Uniemożliwia wklejenie do zapytania SQL fragmentu niechcianego kodu. Bez metod klasy Param zmienne byłyby niezabezpieczone. Cytat Czemu konfig bazy danych znajduje sie w repo? A co jak inna osoba ma inne dane do bazy niz ty? W Symfony jest dobrze. Tu nie chciało mi się usuwać. Miał to być przykładowy skrypt. Ustawiłem default'owo user i hasło root. Poprawię przy najbliższej okazji. Cytat $code = ($_GET['code']) ? (int) $_GET['code'] : 404; po co ten pierwszy nawias? $code = $_GET['code'] ? (int) $_GET['code'] : 404; A jakbys przesiadl sie na php7 to juz w ogole bajka: $code = (int) ($_GET['code'] 404); Tego nawiasu używam, bo coś takiego widziałem w dokumentacji. Ten skrypt powstawał, gdy nie było jeszcze php7. Nie chciało mi się wszystkiego poprawiać. |
|
|
7.04.2020, 16:25:50
Post
#12
|
|
Grupa: Moderatorzy Postów: 36 523 Pomógł: 6309 Dołączył: 27.12.2004 |
Cytat Zabezpiecza przed atakami. Uniemożliwia wklejenie do zapytania SQL fragmentu niechcianego kodu. Bez metod klasy Param zmienne byłyby niezabezpieczone. Do zabezpieczania zapytan uzywa sie BINDowania a nie jakis pseudu zabezpieczen sprzed 30 lat Cytat Tego nawiasu używam, bo coś takiego widziałem w dokumentacji. W dokumetnacji tez widziales == zamiast === Zreszta, jak widziales po naszej rozmowie z vikingiem, nawet takie zbedne nawiasy maja swoich fanow. Cytat W sumie faktycznie mógłbym umieścić kod css i js w oddzielnym katalogu np. public. Tylko że ostatnia linijka kodu w pliku .htaccess zabezpiecza przed dziurami. Kiedys rozwiniesz sie jeszcze bardziej, przesiadziesz sie np. na NGINX i tyle z twojego zabezpieczenia. ROb porzadnie od razu. PUBLIc to public, PRIVATE to private
-------------------- "Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista "Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer |
|
|
7.04.2020, 16:50:14
Post
#13
|
|
Grupa: Zarejestrowani Postów: 6 375 Pomógł: 1116 Dołączył: 30.08.2006 Ostrzeżenie: (0%) |
Jedna strona jeden kontroler. W różnych fw różnie się to zwie (handlers, actions, invokable controller) ale w takim razie zastosuj __invoke. Myślałeś o psr15?
-------------------- |
|
|
8.04.2020, 10:21:48
Post
#14
|
|
Grupa: Zarejestrowani Postów: 82 Pomógł: 0 Dołączył: 3.08.2017 Ostrzeżenie: (0%) |
Cytat Tak. I model nie powinieniec dziedziczyc po database. On powinien to miec wstrzykniete i jego nie powinno obchodzic startowanie i zamykanie polaczenia Nasuwa mi się pytanie... Gdzie i jak powinienem startować i zamykać połączenie z bazą danych? Cytat Jedna strona jeden kontroler. W różnych fw różnie się to zwie (handlers, actions, invokable controller) ale w takim razie zastosuj __invoke. Myślałeś o psr15? Będę szczery... Poczytałem i wiem, jak ten __invoke działa, ale nie mam pojęcia, jak go w moim projekcie użyć. Jeśli chodzi o PSR15, to muszę się wgryźć, bo na szybkiego średnio zrozumiałem. Mógłbym prosić o jakąś podpowiedź, jak tego mam użyć? |
|
|
8.04.2020, 11:02:09
Post
#15
|
|
Grupa: Zarejestrowani Postów: 6 375 Pomógł: 1116 Dołączył: 30.08.2006 Ostrzeżenie: (0%) |
Na przykładzie tego https://github.com/webeeq/framework.eeq/blo...rController.php
-------------------- |
|
|
8.04.2020, 11:09:40
Post
#16
|
|
Grupa: Moderatorzy Postów: 36 523 Pomógł: 6309 Dołączył: 27.12.2004 |
Cytat Nasuwa mi się pytanie... Gdzie i jak powinienem startować i zamykać połączenie z bazą danych? Wszystko zalezy jak rozwiazesz tworzenie modeli, serwisow itp, ale generalna idea: gdzies tam, niech dla przykladu bedzie to index.php, tworzysz obiekt bazy danych:
Taki obiekt bazy danych jest potem wstrzykiwany wszedzie tam gdzie jest potrzebny -------------------- "Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista "Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer |
|
|
8.04.2020, 13:24:10
Post
#17
|
|
Grupa: Zarejestrowani Postów: 82 Pomógł: 0 Dołączył: 3.08.2017 Ostrzeżenie: (0%) |
Dzięki serdeczne za trafne uwagi. Wyciągnę z nich wnioski na przyszłość.
|
|
|
Wersja Lo-Fi | Aktualny czas: 31.10.2024 - 23:58 |