DbM Framework - Autorska aplikacja frameworka opartego na wzorcu MVC |
DbM Framework - Autorska aplikacja frameworka opartego na wzorcu MVC |
20.12.2023, 21:40:29
Post
#1
|
|
Grupa: Zarejestrowani Postów: 529 Pomógł: 6 Dołączył: 21.07.2008 Ostrzeżenie: (0%) |
Witajcie,
miałem chwilę i usiadłem do autorskiego frameworka opartego na wzorcu MVC. Chciałbym go dopracować, utworzyć wersje stabilną. Kod frameworka jest dostępny pod adresem: https://github.com/artimman/dbmframework Zakończyłem pierwszy test aplikacji na serwerze zdalnym - powodzeniem. Wydaje się, że już jest całkiem Ok i ciekawe, czy ktoś się ze mną zgodzi? -------------------- I welcome you on the Internet >>> Design by Malina
|
|
|
21.12.2023, 09:42:45
Post
#2
|
|
Grupa: Moderatorzy Postów: 36 468 Pomógł: 6300 Dołączył: 27.12.2004 |
ok, to na plus:
+ znajomosc jako tako klas + uzywanie typowania w wiekszosci miejsc. + prawie uzywanie najnowszej wersji php a teraz troche krytki: w ogole na poczatek to zaprzyjaznij sie z takmi narzedziami jak : - php-cs-fixer - phpstan - inne do statycznej analizy kodu One wylapia ci na dzien dobry naprawde duzo bledow I generalnie mowie tutaj o bledach ktore nie sa moze jakos krytyczne, ale w dluzszej perspektywie beda uciazliwe do poprawnej pracy z twoim kodem. NIe wiem jak planujesz docelowo dostarczac swoj FW, ale na chwile obecna widze wrzuciles go fo VENDOR a ten katalog jest zastrzezony dla COMPOSER. wywal go wiec stamtad i wsadz nie wiem, np do LIBS. No i jesli twoj kod uzywa np phpmailer, ktory jest zapisany w composer.json to nie zapisuj go w GIT. Katalog VENDOR generalnie w GIT ma sie nie znalezc w ogole. po to jest composer plik config.php rowniez powinien nazywac sie np config.php.dist i dopiero ludzie kopiuja go sobie jako config.php lokalnie. Dlaczego? bo z kazdym updatem z twojego gita, ludzie straca swoje zmiany gdy to zostanie jak teraz declare(strict_types=1); ma byc w kazdym pliku php a nie w co drugim Potworki w stylu IF IF IF IF IF az oczy bola NIe bede omawial wszystkich bo w pyte tego masz, ale przyklad jak to sie poprawia Jesli masz kod IF (costam) { //blabla return 'cos tam' } return null; To zeby uniknac zagniezdzenia duzego to sie robi poprostu negacje na pocatku, wali nullem na poczatku a reszta duzego kodu leci juz bez zagniezdzenia czyli: IF (!costam) { return null; } //blabla return 'cos tam' Jak widzisz w zagniezdzeniu jest tylko return null a nie milion linijek public function __construct() { try { $this->connect = new PDO("mysql:host=" . DB_HOST . ";dbname=" . DB_DATABASE, DB_USER, DB_PASSWORD, array(PDO::ATTR_ERRMODE => PDO::ERRMODE_WARNING)); $this->connect->exec("SET NAMES utf8"); return $this->connect; } catch (PDOException $exception) { throw new DbmException($exception->getMessage(), $exception->getCode()); } } No przeciez konstruktow nigdy nic nie zwraca wiec po grzyba tam return? konstruktor sam w sobie jest "returnem" public function querySql(string $sql, ?string $fetch = null): PDOStatement { if ($fetch == 'assoc') { $stmt = $this->connect->query($sql, PDO::FETCH_ASSOC); } else { $stmt = $this->connect->query($sql); } if (!$stmt) { throw new DbmException($this->connect->errorInfo()[2], $this->connect->errorInfo()[1]); } else { return $stmt; } } po co tu ten fetch jest raz ze stringiem a na dodatek nullem? skoro on odpowiada tylko za dwie mozliwosci, to zrob z niego boola i juz. Dodatkowo ten drugi ELSE na dole jest totalnei zbedny. : public function querySql(string $sql, bool $fetch = false): PDOStatement { if ($fetch) { $stmt = $this->connect->query($sql, PDO::FETCH_ASSOC); } else { $stmt = $this->connect->query($sql); } if (!$stmt) { throw new DbmException($this->connect->errorInfo()[2], $this->connect->errorInfo()[1]); } return $stmt; } Analogicznie cala masa innych funkcji tam public function requestData(string $fieldName) { if ($_SERVER['REQUEST_METHOD'] == "POST" || $_SERVER['REQUEST_METHOD'] == 'post') { if (array_key_exists($fieldName, $_POST)) { return trim($_POST[$fieldName]); } } elseif ($_SERVER['REQUEST_METHOD'] == 'GET' || $_SERVER['REQUEST_METHOD'] == 'get') { if (array_key_exists($fieldName, $_GET)) { return trim($_GET[$fieldName]); } } } To ze ktos wyslal dane POSTem, nie znaczy ze dane nie znajduja sie tez w GET. wlasnie ta funkcja zlikwidowales polowe funkcjonalnosci dla ludzia. public function setDataToDB($value) { $value = strip_tags($value); $value = htmlspecialchars($value); return $value; } A co ty mi tutaj kasujesz tagi z pola co chce wlozyc do bazy? Jak bede chcial skasowac to sam sobie skasuje. poza tym uzywasz bindowania wiec po co htmlspecialchars? To sie uzywa podczas wyswietlania a nie przed wkladaniem do bazy. Ta cala funkcja jest totalnie zbedna public function userPermissions(int $user): string { $database = new DbmDatabase(); $query = "SELECT roles FROM dbm_user WHERE id = ?"; if ($database->queryExecute($query, [$user])) { if ($database->rowCount() > 0) { $data = $database->fetchObject(); return $data->roles; } else { return 'dataNotFound'; } } else { return 'dataQueryError'; } } Jesli funkcja zwraca role, to ma zwracac role a nie komunikaty bledow. Od bledow masz wyjatki.I znowu wpyta zagniezdzen tutaj public static function temp_htmlUser($sessionUserId, $module = null): void { $database = new DatabaseClass(); $userId = (int) $sessionUserId; $query = "SELECT user.login, user.avatar, user_details.fullname FROM dbm_user user" . " INNER JOIN dbm_user_details user_details ON user_details.user_id = user.id" . " WHERE user.id = '$userId'"; if ($database->queryExecute($query)) { $data = $database->fetchObject(); Czemu ta funkcja radosnie tworzy polaczenie do bazy i jeszcze sama z siebie pobiera usera? polaczenie z baza ma byc stworzone rac w calej aplikacji i przekazywane potem do odpowiednich klas. W tym momecnie wczasie jednego request ty generujesz mase polaczen do bazy. Rowniez obiekt usera ma byc pobrany raz i przekazywane do odpowiednich klas. Klasa TranslationClass. Raz ze te Class w nazwie jest zbedne, a dwa raz funkcje z malej raz z z duzej... generalnie maja byc z malej. W klasie DatabaseClass, znowy Class zbedne ale: public function __construct() { try { $this->connect = new PDO("mysql:host=" . DB_HOST . ";dbname=" . DB_DATABASE, DB_USER, DB_PASSWORD, array(PDO::ATTR_ERRMODE => PDO::ERRMODE_WARNING)); $this->connect->exec("SET NAMES utf8"); Ty tutaj znowu tworzyc nowy obiekt PDO, czyli nowe polaczenia. Kazdy model z tego dziedziczy, wiec jak odpalisz 3 modele to masz juz 3 nowe polaczenia do bazy. No tak sie nie robil .Jak pisalem wczesniej, jedno polaczenia do bazy masz tworzyc i ono ma byc przekazywan tam gdzie trzeba public function getSection(int $id): object { $query = "SELECT * FROM dbm_article_sections WHERE id = '$id'"; $this->queryExecute($query); if ($this->rowCount() > 0) { return $this->fetchObject(); } return (object) []; } I znowy strasznie mieszasz style. Raz jak nie ma rekordu to zwracasz NULL a tu radosnie zwracasz pusty obiekt. No sie zdecyduj na jedno i sie tego trzymaj public function userSigninCorrect(array $params, string $password): ?string { $query = "SELECT * FROM dbm_user WHERE (login=:login OR email=:email) AND verified=true LIMIT 1"; if ($this->queryExecute($query, $params)) { if ($this->rowCount() > 0) { $result = $this->fetchObject(); if (password_verify($password, $result->password)) { return $result->id; } else { return self::VALID_PASSWORD; } } else { return self::VALID_LOGIN; } } else { return null; } } I tutaj znowu, funkcja ktora powinna zwracac info czy user sie zalogowal czy nie to zwraca albo jako text id usera albo komunikaty bledow... No prosze cie public const VALID_LOGIN = 'loginNotFound'; public const VALID_PASSWORD = 'passwordNotMatched'; Chyba sie to powinno nazywac INVALID a nie VALID Jeszcze tego tam jest torche ale juz mi sie nie chce sprawdzac. Generalnie idziesz w dobrym kierunku powiedzmy, ale musisz troche popracowac jeszcze -------------------- "Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista "Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer |
|
|
Wersja Lo-Fi | Aktualny czas: 25.05.2024 - 09:32 |