Witaj Gościu! ( Zaloguj | Rejestruj )

Forum PHP.pl

 
Reply to this topicStart new topic
> [www] Prosta sieć reklamowa - Symfony i Framework w PHP
eerie
post 7.04.2020, 10:44:14
Post #1





Grupa: Zarejestrowani
Postów: 81
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?
Go to the top of the page
+Quote Post
nospor
post 7.04.2020, 11:01:06
Post #2





Grupa: Moderatorzy
Postów: 36 440
Pomógł: 6290
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'] questionmark.gif 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 wink.gif 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 wink.gif

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 wink.gif


--------------------

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
viking
post 7.04.2020, 11:02:38
Post #3





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1114
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


--------------------
Go to the top of the page
+Quote Post
nospor
post 7.04.2020, 11:13:32
Post #4





Grupa: Moderatorzy
Postów: 36 440
Pomógł: 6290
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

Go to the top of the page
+Quote Post
viking
post 7.04.2020, 11:45:59
Post #5





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1114
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.


--------------------
Go to the top of the page
+Quote Post
nospor
post 7.04.2020, 11:48:52
Post #6





Grupa: Moderatorzy
Postów: 36 440
Pomógł: 6290
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

Go to the top of the page
+Quote Post
viking
post 7.04.2020, 11:50:04
Post #7





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1114
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


CR są per firma. U mnie by nie przeszło bez nawiasu wink.gif + cs fixer dodający spacje.


--------------------
Go to the top of the page
+Quote Post
nospor
post 7.04.2020, 11:52:05
Post #8





Grupa: Moderatorzy
Postów: 36 440
Pomógł: 6290
Dołączył: 27.12.2004




Cytat
cs fixer dodający spacje.
Przynajmniej w jednym sie zgadzamy smile.gif


--------------------

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
eerie
post 7.04.2020, 15:22:17
Post #9





Grupa: Zarejestrowani
Postów: 81
Pomógł: 0
Dołączył: 3.08.2017

Ostrzeżenie: (0%)
-----


Cytat(nospor @ 7.04.2020, 12:13:32 ) *
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 wink.gif 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? wink.gif

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ć? smile.gif

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ć... wink.gif
Go to the top of the page
+Quote Post
nospor
post 7.04.2020, 15:50:08
Post #10





Grupa: Moderatorzy
Postów: 36 440
Pomógł: 6290
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

Go to the top of the page
+Quote Post
eerie
post 7.04.2020, 16:01:19
Post #11





Grupa: Zarejestrowani
Postów: 81
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'] questionmark.gif 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ć. wink.gif
Go to the top of the page
+Quote Post
nospor
post 7.04.2020, 16:25:50
Post #12





Grupa: Moderatorzy
Postów: 36 440
Pomógł: 6290
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 smile.gif

Cytat
Tego nawiasu używam, bo coś takiego widziałem w dokumentacji.

W dokumetnacji tez widziales == zamiast === wink.gif
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

Go to the top of the page
+Quote Post
viking
post 7.04.2020, 16:50:14
Post #13





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1114
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?


--------------------
Go to the top of the page
+Quote Post
eerie
post 8.04.2020, 10:21:48
Post #14





Grupa: Zarejestrowani
Postów: 81
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ć? wink.gif
Go to the top of the page
+Quote Post
viking
post 8.04.2020, 11:02:09
Post #15





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1114
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


Na przykładzie tego https://github.com/webeeq/framework.eeq/blo...rController.php

  1. class ActivateUserHandler implements RequestHandlerInterface
  2. {
  3. /** @var TemplateRendererInterface */
  4. private $renderer;
  5.  
  6. public function __construct(TemplateRendererInterface $renderer)
  7. {
  8. $this->renderer = $renderer;
  9. }
  10.  
  11. public function handle(ServerRequestInterface $request) : ResponseInterface
  12. {
  13. $activateUserModel = new ActivateUserModel();
  14.  
  15. $activateUserModel->dbConnect(); //głupota, serwis powinien mieć informację o interfejsie storage, połączenie z bazą i tak jest zamykane na koniec requestu
  16.  
  17. $activateUserService = new ActivateUserService($activateUserModel);
  18. $array = $activateUserService->variableAction(
  19. $user,
  20. $code
  21. );
  22.  
  23. return new HtmlResponse($this->renderer->render(
  24. 'app::login',
  25. $array
  26. ));
  27. }
  28. }
  29.  


--------------------
Go to the top of the page
+Quote Post
nospor
post 8.04.2020, 11:09:40
Post #16





Grupa: Moderatorzy
Postów: 36 440
Pomógł: 6290
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:

  1. $db = new Database();
  2. $db->connect(); //choc to, powinno byc zrobiony jako lazy loading jak ci mowiono w innym temacie by bez potrzeby nie laczyc sie z baza.
  3.  
  4. //...
  5. // i na koncu rozlaczenie
  6. $db->disconnect();

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

Go to the top of the page
+Quote Post
eerie
post 8.04.2020, 13:24:10
Post #17





Grupa: Zarejestrowani
Postów: 81
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ść. smile.gif
Go to the top of the page
+Quote Post

Reply to this topicStart new topic
1 Użytkowników czyta ten temat (1 Gości i 0 Anonimowych użytkowników)
0 Zarejestrowanych:

 



RSS Wersja Lo-Fi Aktualny czas: 28.03.2024 - 09:40