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