DbM Framework - Autorska aplikacja frameworka opartego na wzorcu MVC Opcje

[Malinaa]

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?

[nospor]

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

[Malinaa]

Dzięki bardzo za sprawdzenie kodu.

Narzędzia były tu zaprzyjaźnione:
php-cs-fixer - przeleciał kod bez błędu

jako że fixer naprawdę nie pokazał żadnego błędu to phpstan sobie odpuściłem
nie wiem co w tym uciążliwego, chyba że mowa tu o doskonałym kodzie, ale o takim nie słyszałem,
co najwyżej jest idealny, gdzie po jakiejś chwili pojawia się kolejna wersja jakże doskonałego kodu.

Z katalogiem vendor ta mam taką rozkminkę co, jak? Z tego co widzę Symfony jest w /vendor, więc dlaczego chcesz wsadzić tego frameworka do /libs? Jak idealnie poukładać te katalogi?
Tutaj pojawia się więcej znaków zapytania, co kiedy zaktualizuje silnik aplikacji, przydałby się jakiś mechanizm dla aktualizacji, tylko jak go zrobići tak, aby świetnie współpracował z Composer itd.?
Katalogu vendor generalnie nie powinno być w ogóle na Gicie. W takim przypadku jak załadować silnik frameworka najlepiej do katalogu vendor? Podobnie jak w Symfony.

Plik config.php na config.php.dist - faktycznie, racja, jakoś jeszcze nie pomyślałem o tym (nawet jeszcze nie testowałem, czy Composer działa na zdalnym, skopiowałem projekt z Gita i wykonałem pierwsze uruchomienie)

declare(strict_types=1); - no to muszę jeszcze poprzeglądać plik po pliku

Milion linijek if'ów już nie dawało mi dostać się do snu w nocy - zajrzę na to, może teraz coś mi się lepszego przyśni

Z $this->connect i dalej $database->queryExecute($query) tu coś się zamotałem, kur.de jak poprawić te połączenia? Ma być jedno połączenie i pozostałe... mają z takiego korzystać.

Aby przejść dalej to muszę poprawić powyższe... na tym połączeniu z bazą danych itp. zamotałem się! Jak to poprawić?

P.S. Czy musisz rozpier.dzielać mi serducho z tym radośnie zaś?
Czuje się jak Wesoły Romek, może to lepiej niż jakiś Zenek, albo Ziutek, ale generalnie to jeden czort,
może by tak chociaż od Święta zamiast Jasiu pomyśleć Jan Kowalski, chyba widać tu różnicę

Ten post edytował Malinaa 21.12.2023, 11:38:23

[nospor]

phpstan sprawdza innego rodzaju rzeczy wiec polecam mimo wszystko

nie wiem co w tym uciążliwego, chyba że mowa tu o doskonałym kodzie, ale o takim nie słyszałem,
co najwyżej jest idealny, gdzie po jakiejś chwili pojawia się kolejna wersja jakże doskonałego kodu.

Jesli wywoluje twoja metode ktora powinny mi zwrocic string, a ona jako string zwraca mi jeszcze bledy i ja musze znac kod tych bledow i je obslugiwac, to jest to super uciazliwe.
Jesli odpale x modeli i one wszystkie tworza mi nowe polaczenie i mam wielu klientow na raz, to nagle baza mi zacznie mulic. znowu uciazliwe.
Jesli bede chcial z toba wspolpracowac i musial poprawiac cos w twoim kodzie, to sie zachlaszcze po dwoch godzinach
Ja sam nie pisze idealnego kodu, zawsze jest cos do poprawki. Ale jak ktos pokazuje sensowne bledy to nie lece po ambicji.

Jak juz mowilem: VENDOR jest zarezerwowany dla COMPOSER. Twoj lib nie idzie przez composer, ty go wsadzasz na sile w GIT do VENDOR i to jest zle. Tak, Symfony jest w vendor, ale on tam wpada z composer.
Ja VENDOR moge skasowac, potem odpalic composer install i mi sie wszystko odbuduje samo. W twoim wypadku, ja skasuje VENDOR i szlag trafil projekt.
Jak mowilem, VENDOR ma nie byc w GIT. VENDOR sam sie buduje jak odpalisz composer install. Na tym to wlasnie polega a ty tego nie rozumiesz.

Co do polaczen to poczytaj o DI (dependency injection). Ewentualnie zastosuj SINGLETON - generalnie sie tego nie poleca, ale w twoim wypadku poprawi funkcjonalnosc

P.S. Czy musisz rozpier.dzielać mi serducho z tym radośnie zaś?

Sluchaj, ja ci tego nie pisze ze zlosci, z mysla "o, kolejny koles ktoremu dopieke"...
Poswiecilem troche czasu by przejrzec twoj kod i ci pomoc przez konstruktywna krytyke.
Ale jesli zle to odbierasz to skasuj temat, zapomnij o sprawie i pisz dalej po swojemu. Tylko nie pisz wiecej ze napisales juz zajebisty kod i kto sie z toba zgadza

[Malinaa]

Kod

Na tym to wlasnie polega a ty tego nie rozumiesz.

Oj, rozumiem, co nie oznacza że już wiem jak prawie idealnie zakodować.

Trochę obracasz kota ogonem. Napisałem: "Wydaje się, że już jest całkiem Ok", czy znaczy to samo jak piszesz: "ze napisales juz zajebisty kod". Nie, ale rozumiem.
Pewnie po prostu tak masz. Wierzę, że nie piszesz po złości, gdyby było inaczej nie prosiłbym o pomoc w napisaniu bezbłędnego kodu.
Z lekka irytuje mnie radośnie jak Wesoły Romek, ale nikt nie musi o tym wiedzieć, pewnie moja zasługa/wina, że jest tak radośnie i coś dziwnie odbieram.

Dzisiaj to już nie za bardzo jest czas, przed świętami raczej nie usiądę do poprawiania.

Wesołych Świąt.
Dziękuje za pomoc.

[nospor]

A ,teraz kumam do czego piles piszac te "radosnie" .Nie zakumalem ze to do mnie
poprostu tak pisze. Jesli ktos robi cos w jednym schemacie, a potem nie z gruszki ni z pietruszki zmienia na cos innego to pisze po prostu "radosnie" . Bez zlosliwosci, ot takie powiedzonko.Tak samo pisze, gdy oceniam komus kod w firmie.
Tak jak ty zawsze zwracales NULL gdy nie bylo rekordu, a potem w jednej funkcji radosnie zwrociles pusty OBJECT.

Jak nie chcesz swojego liba wrzucac do katalug LIB, tylko twardo sie trzymasz VENDOR, to zajrzyj do composera dokumentacji, tam masz podane jak sie importuje prywatne repozytoria z github przez composer. I juz. Ale wywal ten folder VENDOR i wszystko co w nim masz, tak samo jak phpmailer. On jest w composer.json i juz go nie nie powiniinies trzymac w swoim git.

[netir]

Jak zobaczyłem w model raw query bez bindowania to już przestałem dalej sprawdzać (model nie powinien wiedzieć tyle o implementacji warstwy danych) Na pewno jest to ciekawe ćwiczenie do zrozumienia wielu aspektów budowania frameworka i chyba jako ćwiczenie należy to oceniać

[PHP] pobierz, plaintext 
$query = "SELECT article.id AS aid, article.image_thumb, article.page_header, article.page_content, section.id AS sid, section.section_name, details.user_id AS uid, details.fullname"
            ." FROM dbm_article article"
            ." JOIN dbm_article_sections section ON section.id = article.section_id"
            ." JOIN dbm_user_details details ON details.user_id = article.user_id"
            ." ORDER BY article.created DESC LIMIT $limit";
 
        $this->queryExecute($query);
[PHP] pobierz, plaintext 

Ten post edytował netir 26.12.2023, 16:08:13

[Malinaa]

1. w ogole na poczatek to zaprzyjaznij sie z takmi narzedziami jak :
- php-cs-fixer
- phpstan
- inne

2. 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.

3. 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

4. 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

5. declare(strict_types=1); ma byc w kazdym pliku php a nie w co drugim

6. Potworki w stylu IF...

7. public function __construct()

8. public function querySql(string $sql, ?string $fetch = null): PDOStatement

9. public function requestData(string $fieldName)

10. public function setDataToDB($value)

11. public function userPermissions(int $user): string

12. public static function temp_htmlUser($sessionUserId, $module = null): void

13. W klasie DatabaseClass
Ty tutaj znowu tworzyc nowy obiekt PDO, czyli nowe polaczenia.

14. public function getSection(int $id): object

15. public function userSigninCorrect(array $params, string $password): ?string

16. public const VALID_LOGIN = 'loginNotFound';

1.
a) php-cs-fixer - jest zainstalowany - pokazuje bez błędów
b ) phpstan - doinstalowałem - pokazuje błąd

Line Controller\IndexController.php
------ ------------------------------------------------------------------------------------
:45 Access to constant BLOG_INDEX_ITEM_LIMIT on an unknown class App\Config\Constants.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
------ ----------------------------------------------------------------------------

nie wiem o co chodzi z tym błędem?

c) inne - jakie to jeszcze inne narzędzia

2 i 3. FW dałem do katalogu library.
Usunąłem vendor -> wykonałem composer update i się załadował nowy vendor, czyli powinno być Git (do przetestowania na zdalnym).

Z katalogiem vendor jest pytanie.
Nie jest potrzebny na Gicie, composer go tworzy - Ok, ale co kiedy kod jest w wersji CMS czyli dla zielonych, kiedy zostawię vendor uruchomią aplikacje bez problemu, ale jeśli usunę to nie wykonają polecenia composer i aplikacja nie zadziała. Nie odpalą composera tym bardziej na zdalnym serwerze, ba zwykle taki user nie ma na serwerze SSH, a na pewno nie wie po co to. Zostawiam vendor i problem z głowy, zadziała nawet bez SSH.

4. Dodałem config.php.dist teraz zastanawiam się, czy to coś dało kiedy po zmianie nazwy z config.php.dist na config.php Git i tak będzie widzieć taką zmianę itd. Tu raczej jakiś git ignore config.php trzeba by wstawić?

5. declare(strict_types=1); dopisałem, ale ma być w każdym pliku .php, chyba nie w tempates np. base.html.php ?

6. IFy, może np. tak:

Kod

public function getSections(): ?array
    {
        $query = "SELECT * FROM dbm_article_sections ORDER BY id ASC";

        $this->queryExecute($query);

        if ($this->rowCount() > 0) {
            return $this->fetchAllObject();
        }

        return null;
    }

ponieważ według tego co napisałeś:

Kod

public function getSections(): ?array
    {
        $query = "SELECT * FROM dbm_article_sections ORDER BY id ASC";

        $this->queryExecute($query);

        if ($this->rowCount() == 0) {
            return null;
        }

        return $this->fetchAllObject();
    }

z taką negacją na początku - jasne, że tak się robi, tylko w tym przypadku jakoś mi nie pasuje i nie wygląda najlepiej if count == 0 ? Tu zawsze było if count > 0 !
Jak powinna wyglądać więc ta metoda, tak aby było super git?

7. Nie wiem skąd ten return w konstruktorze - usunąłem.

class DatabaseClass {}

Konstruktor zmieniony, ale czy nie będzie bardziej poprawnie kiedy zamiast DB_HOST odznaczę private $dbHost = DB_HOST; itd. czy bez różnicy, tak i tak jest ok?
Przypomniało mi się, że gdzieś czytałem o exec(); że należy do metod niebezpiecznych i nie raz jest wyłaaczone na serwerze, więc raczej lepiej napisać w wersji z $options = []; ?

8. Ten $fetch wolałbym zostawić, ponieważ przy rozbudowie to nie będzie boolean, można dodać tu więcej opcji niż dwie true/false.

9. public function requestData(string $fieldName) może jakiś pomysł jak poprawić, aby było na 100%?

10. public function setDataToDB($value) czyli do usunięcia.
Czy to tak można zezwalać na zapisywanie wszystkiego do bazy danych co user wklepie w formularz?

11. public function userPermissions(int $user): string - tylko rola, więc komunikaty usuwam.

12. public static function temp_htmlUser($sessionUserId, $module = null): void
Jest oznaczone "temp" czyli tymczasowe do napisania od nowa.
Nie wiem jak wykonać tu połączenie z bazą danych w inny sposób, tzn. pojedyncze?

13. Nie za bardzo wiem jak poprawić całość na takie jedno połączenie z bazą danych?

14. Widzę, że return (object) [] to już kombinacja alpejska, ale

if ($this->rowCount() == 0) {
return null;
}
return $this->fetchObject();

też świetne nie będzie, czy można jeszcze inaczerj (świetnie)?

15. Ta funkcja nie sprawdza czy user zalogowany, a zwraca jesli wystąpi bład komunikat do formularza logowania, czyli jest tu potrzebny string.

16. Miało nazywać się VALIDATION_LOGIN walidacja loginu, ale skróciłem na VALID_LOGIN, zmienione na VALIDATION_LOGIN itd.
Short -> full name - to kosmetyka, czy jest uciążliwe?

Ten post edytował Malinaa 28.12.2023, 17:59:49

[nospor]

ad1) Nie widzi klasy App\Config\Constants
Jeszcze inne? np. psalm

ad2,3
No niestety, na super zielonych i swiety boze nie pomoze.
Jak bardzo chcesz to dodaj instrukcje skad wziasc biblioteki takie jak phpMailer i zeby wgrac je do katagu vendor. No ale i wowczas caly szlag trafi autoloading itp. wiec generalnie nie przejmuj sie tym.

ad4 tak, config.php ma byc w .gitignore.

ad5) no tylko w plikach klas

ad6 ja podalem ci negacje IFow gdy masz duzy blok kodu w IF. Przyklad co tu pokazales nie lapie sie do tego

ad7. Troche sie zgubilem. Co ma do tego exec() ?

ad9 No nie zakladaj, ze skoro metoda poszla POST, to ze tytko masz dane w post. Jesli metoda POST, to szukaj najpierw w POST, a jak nie ma to zajrzyj tez do GET.

ad10
Jak pisalem, uzywasz bindowania wiec baza jest bezpieczna.
htmlspecialchars() sie uzywa przed wywsietleniem danych a nie przed wlozeniem ich do bazy.
Jesli zas bedziesz mial jakies krytyczne pola, gdzie nie moga pojsc konkretne rzeczy, to tam przed zatwierdzeniem forma, dodajesz VALIDACJE, czy dane sa w porzadku.

ad12,13
Uzyj wzorca SINGLETON na klasie DatabaseClass. I wywal te "Class" z tych wszystkich nazw

ad14
No a co jest zlego w tym?
if ($this->rowCount() == 0) {
return null;
}
return $this->fetchObject();

Jak nic nie ma to zwracasz NULL a jak jest to zwracasz obiekt co chciales. Wiec chyba ok tak?

ad15
No ale to albo zwraca userId albo zwraca komunikat bledu. Wszystko to jest tekstem. Skad ja mam wiedziec czy otrzymany tekst to id uzytkownika i tym samym ok, czy moze to jednak komunikat bledu? No to nie ma sensu. A co oznacza NULL?
Przerob to na funkcje ktore zwraca ID zalogowanego usera albo wyjatek gdy cos nie tak

[PHP] pobierz, plaintext 
<?php
public function getSignedUserId(array $params, string $password): int
{
    $query = "SELECT * FROM dbm_user WHERE (login=:login OR email=:email) AND verified=true LIMIT 1";
 
    $res = $this->queryExecute($query, $params);
    if (!$res) {
        throw new \Exception('Something went wrong');
    }
 
    if ($this->rowCount() == 0) {
        throw new \Exception('Invalid login or email');
    }
 
    $result = $this->fetchObject();
 
    if (!password_verify($password, $result->password)) {
        throw new \Exception('Invalid password');
 
    } 
 
    return (int) $result->id;    
}
?>
[PHP] pobierz, plaintext 

Zwroc tez uwage jak pozbylem sie tych duzych zagniezdzen co miales. u mnie masz tylko jedno zagniezdzenie a u ciebie byly az 3 podzagniezdzenia. Kod teraz wyglada o niebo czytelniej

ad16
Twoj skrot jest uciazliwy bo tak go skrociles ze nadale przeciwne znaczenie.

[Malinaa]

1. Jak sprawić, aby widział klasę App\Config\Constants, czy takie umieszczenie stałych w tym pliku jest ok?
Czy plik może zostać w katalogu, w którym się znajduje, czy należy przenieść do katalogu application (autoloader). Wówczas będzie widoczny i wszystko ok.

2 i 3. w katalogu /vendor/ zostawię tylko autoloading i dodałem informacje co jak w README.md

4 - 9.. zrobione

10. sprawdziłem metoda nie była w ogóle używana, więc została usunięta

12 i 13 Jeżeli SINGLETON nie jest zalecane, zróbmy tak jak powinno być z Dependency Injection
Próbowałem zrobić, ale coś nie za bardzo idzie, zacząłem poprawiać od:

Kod

class DatabaseClass extends PDO
{
    //private $connect;
    private $result;
    //public $pdo;
    
    public function __construct() // __construct(PDO $pdo)
    {
        try {
            $dbDSN = "mysql:host=" . DB_HOST . ";dbname=" . DB_DATABASE;
            $dbOptions = [
                PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
                PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8",
            ];

            //$this->connect = new PDO($dbDSN, DB_USER, DB_PASSWORD, $dbOptions);
            parent::__construct($dbDSN, DB_USER, DB_PASSWORD, $dbOptions);
        } catch (PDOException $exception) {
            throw new DbmException($exception->getMessage(), $exception->getCode());
        }
    }
...
}

Jedno połączenie wszędzie gdzie tylko jest potrzebne wykonanie zapytania.
Pytanie jak byś to zrobił, aby wstrzyknąć połączenie w plikach i metodach:

Kod

class FrameworkClass
{
...

    // User permissions
    public function userPermissions(int $id): ?string
    {
        $database = new DbmDatabase(); // TODO! Jak zmienic, aby bylo OK?

        $query = "SELECT roles FROM dbm_user WHERE id = ?"; // TODO! Jak to jest z :id lub znakiem zapytania, czy tak samo jest bezpieczne?

        $database->queryExecute($query, [$id]);

        if ($database->rowCount() == 0) {
            return null;
        }

        $data = $database->fetchObject();

        return $data->roles;
    }
}

Kod

class TemplateClass
{
...

    /* TODO! Temporary. Change to full htmlUserNavigation() ? */
    public static function temp_htmlUser($sessionUserId, $module = null): void
    {
        $database = new DatabaseClass();
        $userId = (int) $sessionUserId;
        ...
    }
}

Te Class to trochę tak celowo zostawiłem, aby nad tym chwilę pomyśleć jak powinny nazywać się te klasy oraz folder(y) w obecnym /library/dbmframework/ (folder classes, methods) mając na względzie, że może dodałbym zaciąganie tych klas i kodu do katalogu /vendor/. Poza tym np. ExceptionClass.php nie pasuje zmienić na Exception.php -> class Exception {} podobnie też class Database {} i pozostałe miałby zbyt ogólne nazwy, nie za bardzo to mi się podoba, ponieważ trzeba wówczas dodawać aliasy np. Exception as MyException;. Dodatkowo, czy trzymać wszystko w jednym katalogu, czy może lepiej podzielić te pliki i klasy do subkatalogów i dalej co z namespace też zmienić odpowiedni w zależności, czy dzielimy klasy i kod na subkatalogi? Trochę zastanawiające jak poukładać te pliki i klasy w dobrą całość.

14, Jakoś miałem wrażenie, że
if ($this->rowCount() > 0) { }
jest Ok, bynajmniej logicznie nadal mam takie wrażenie, chociaż bez zagnieżdżenia wygląda lepiej.

15. Metoda ma zwrócić string, nie int i służy do wyświetlania komunikatów w formularzu, więc nie może być throw.
Pozbyłem się tych zagnieżdżeń i wygląda lepiej.

16. Dla jasności zmienione na bez skrótów, które jak się okazuje mogą być mylące.

Mam prośbę o pomoc w ogarnięciu Dependency Injection w projekcie, bo się przy tym już pogubiłem.

Ten post edytował Malinaa 8.01.2024, 09:30:55

[nospor]

ad1 Klasy maja byc widoczne w autoloading i wtedy powinny byc widoczne przez analizatory.
Swoja droga pisales ze zadnych innych bledow ci nie pluly. Troche dziwne, bo przegldajac twoj kod powinno wypluc wiecej bledow. Na pewno dobrze to skonfigurowales by patrzylo na wszystkie katalogi/pliki?

ad15
Jesli twoja metoda jako tekst zwraca albo id uzytkownika, albo komunikat bledu, to jest to zle zaprojektowana metoda. juz ci to pisalem. wiec sie zdecyduj co ona ma zwraca, albo komunikat bledu, albo id uzytkownika.
Zas co do throw to po to masz TRY CATCH by to wlasnie tam zlapac

ad13:
no Singleton w twoim miejscu bylby najlepszy. Najszybciej bys go wdrozyl. Ale jesli nie chcesz to o to:
$database = new DbmDatabase();
masz zrobic tylko raz w swojej aplikacje, w jakims glownym core, chociazby w index.php i potem te $database masz przekazywac do kazdych innych klas (w konstruktorze) ktore tego potrzebuja. Ale twoj kod nie jest na to przygotowany wiec dlatego juz lepiej bys uzyl Singleton

[Malinaa]

1. Klasa przeniesiona i widoczna - całość bez błędów.

Narzędzia sprawdzające jakość kodu i błędy nie pluły mi błędów poza tym jednym co się powtarzał x-razy o klasie która jest nie widoczna
oraz drobiazg, o którym nie pisałem, bo ogólnie wystarczyło przenieść echo() do nowej linii.

15. Ok, ale zostawiam tę metodę, aby nie musieć pisać dwóch, ponieważ to id jest używane tylko w session value i jest stringiem.
Taka nie typowa metoda, na pewno słyszałeś 2 in 1

13. Rozumiem, że byłby lepszy, ale to powinno być po prostu super dobrze.

Czyli w pliku głównym index.php tu gdzie mam kod Routa dodaję Database:

Kod

### RENDER PAGE ###
use Dbm\Classes\RoutClass;
use Dbm\Classes\DatabaseClass; // as DbmDatabase;

new RoutClass();

$database = new DatabaseClass();

i jak dalej przygotować ten mój kod pod DI, ponieważ nie pomyślałem o tym na początku, a teraz przy tym DI to już się zamotałem.

Ten post edytował Malinaa 8.01.2024, 13:34:06

[nospor]

ale to powinno być po prostu super dobrze.

Z jednej strony nalegasz na super dobrze, a z drugiej strony piszesz metode ktora zwraca albo ID albo komunikat bledu, wszystko jako tekst i czort wie czy to co zwrocilo to id czy moze jednak error i sie upierasz ze tak ma byc

Singleton u ciebie bedzie OK bo caly kod nadal nie jest super dobrze.

No ale jak sie upierasz to w FrameworkClass w metoddzie model() masz
return new $modelNamespace();
tam wiec do constructora masz przekazac $database
A skad FrameworkClass ma miec $database? No to widze kontroller dziedziczy po FC wiec w RoutClass masz przekazac do kontrolera w konstruktorze $database.
A skad RoutClass ma miec $database? No widze ze w index.php tworzysz obiekt RoutClass wiec tam masz przekazac jeszcze $database

I tak z kazda sciezka gdzie $database jest wykorzystywane

[PHP] pobierz, plaintext 
 
$database = new DatabaseClass();
new RoutClass($database);
[PHP] pobierz, plaintext 

Albo zaimplementuj Registry, wloz raz $database do registry i mozesz pominac pare krokow.
Albo zastosuj singleton

edit: wlasnie sobie uswiadomilem ze i tak ta cala sciezka z $database co ci teraz podalem jest o kant 4 liter skoro u ciebie kazdy model dziedzicy po DatabaseClass a ta w konstruktorze na dzien dobry tworzy nowe polaczenie z baza
Dlatego nic juz niekombinuj, tylko utworz ten singleton na klasie DatabaseCLass i problem z glowy
Tylko zeby zrobic z tego Singleton, to wywal to dziedziczenie po PDO co widze dodales ostatnio w klasie DatabaseClass rob normalnie new PDO itp. Oczywiscie okraszone juz Singletonem


edit2:
A tu masz pokaze ci jeszcze jak sie pozbywac tych nieszczesnych zagniezdzen
Aktualnie masz taka metode (przeczytaj komentarze co tam wlozylem)

[PHP] pobierz, plaintext 
    public function queryExecute(string $query, ?array $params = [], bool $reference = false): bool
    {
        try {
            $this->result = $this->prepare($query);
 
            if (empty($params)) {
                return $this->result->execute();
            } else {
                // PO CO TEN ELSE? Przeciez IF mial RETURN wiec ELSE NIEPOTRZEBNY
                $first = array_key_first($params);
 
                if (is_string($first)) {
                    // PO CO ten dlugi kod w tym IF? Nie lepiej zrobic negacje i w IF tylko wlozysz jeden prosty return?
                    foreach ($params as $key => &$value) {
                        is_int($value) ? $type = PDO::PARAM_INT : $type = PDO::PARAM_STR;
 
                        if (!$reference) {
                            $this->result->bindValue($key, $value, $type);
                        } else {
                            $this->result->bindParam($key, $value, $type);
                        }
                    }
 
                    return $this->result->execute();
                }
 
                return $this->result->execute($params);
            }
        } catch (PDOException $exception) {
            throw new DbmException($exception->getMessage(), $exception->errorInfo[1]);
        }
    }
[PHP] pobierz, plaintext 

A tu po poprawkach, Zniknely dwa zbedne zagniezdzenia i od razu czytelniej.

[PHP] pobierz, plaintext 
    public function queryExecute(string $query, ?array $params = [], bool $reference = false): bool
    {
        try {
            $this->result = $this->prepare($query);
 
            if (empty($params)) {
                return $this->result->execute();
            }
 
            $first = array_key_first($params);
 
            if (!is_string($first)) {
                return $this->result->execute($params);
            }
 
            foreach ($params as $key => &$value) {
                is_int($value) ? $type = PDO::PARAM_INT : $type = PDO::PARAM_STR;
 
                if (!$reference) {
                    $this->result->bindValue($key, $value, $type);
                } else {
                    $this->result->bindParam($key, $value, $type);
                }
            }
 
 
            return $this->result->execute();
 
        } catch (PDOException $exception) {
            throw new DbmException($exception->getMessage(), $exception->errorInfo[1]);
        }
    }
[PHP] pobierz, plaintext 

[Malinaa]

Z jednej strony nalegasz na super dobrze, a z drugiej strony piszesz metode ktora zwraca albo ID albo komunikat bledu...

Kod znajdujący się w folderze /library/dbmframework/ oraz bezpośrednio z nim powiązany nalegam, aby napisać super dobrze.
Metoda o której piszesz bardziej jest związana z wersją demo (cms), nie jest jakaś super kluczowa, to tylko demo, jak coś będzie tu zakombinowane w stylu 2w1 - no problem.
Element demo cms - na luzie. Jak to wytłumaczyć, może spójrz na to w ten sposób, że przy kolejnym projekcie opartym o dbmframework folder /application/ będzie pusty, wszystko co jest w tym folderze będzie od nowa tworzone w nowej wersji (nie cms).
P.S. Przykładowo mam skrypt płatności, który napisałem 10 lat temu, przydałoby się go napisać od nowa i chciałbym oprzeć projekt od library ->dbmframework. Czy teraz kaman? Kaman beauty

Uparłeś się na Singleton, więc zacząłem pisać pod ten Singleton:

Kod

class DatabaseClass // implements SingletonInterface - * Przyszły mi tu na myśl interfejsy, fajnie byłoby dodać takie do aplikacji np. w /library/dbmframework/interfaces/ pytanie jak tworzyć interfejsy, które później można fajne wstrzykiwać np. do konstruktora itp. itd.
{
    private static $instance = null; // static - po dodaniu pierwszego elementu dla Singletona zaczyna się już jakaś katastrofa !?
    private $connect;
    private $statement;

    //private function __construct() // po zmianie na private - wysypie się
    public function __construct()
    {
        try {
            $dbDSN = "mysql:host=" . DB_HOST . ";dbname=" . DB_DATABASE;
            $dbOptions = [
                PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
                PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8",
            ];

            $this->connect = new PDO($dbDSN, DB_USER, DB_PASSWORD, $dbOptions);
        } catch (PDOException $exception) {
            throw new DbmException($exception->getMessage(), $exception->getCode());
        }
    }

    public static function getInstance() //: Result ?
    {
        if (!self::$instance) {
            self::$instance = new DatabaseClass();
        }

        return self::$instance;
    }

    public function getConnection() //: Result ?
    {
        return $this->connect;
    }
}

Przy Singleton o ile widzę tworzymy tu zmienne globalne - to nie może tak być, okropnie wygląda, nie róbmy tego.

Modele, o których piszesz jak widzę i tak muszą zostać poprawione (przerobione), coś nie sposób w nich wstawić chociażby konstruktora.

Kod

class BlogModel extends DatabaseClass // TODO! Remove extends DatabaseClass
{
    /* TODO! private $database;

    public function __construct(DatabaseClass $database)
    {
        $this->database = $database;
    } */
..
}

W kontrolerach jest podobnie lipa, brak możliwości wstrzykiwania do konstruktora

Kod

class IndexController extends FrameworkClass
{
    private $blogModel;
    private $translation;

    /*
     * TODO! public function __construct(TranslationClass $translation)
     */
    public function __construct()
    {
        $this->blogModel = $this->model('BlogModel'); // TODO! Zmienic sposob ladowania ClassModel ?!

        $translation = new TranslationClass();
        $this->translation = $translation;
    }
...
}

gdzie np. w Symfony jest AbstractController i można do konstruktora wstrzykiwać co dusza zapragnie - super.

Prośba, nalegam zróbmy Dependency Injection.

Dzięki za poprawki i komentarze w queryExecute().
Przy if ($this->rowCount() == 0) {} miałem mieszane wrażenia, zawsze bardziej pasowało mi tu if ($this->rowCount() > 0) {} ale bez zagnieżdżeń jest lepiej.
W przesłanym przykładzie akurat bez dwóch zdań po co te Ify teraz jest dobrze

Ten post edytował Malinaa 9.01.2024, 13:25:44

[nospor]

NIe wiem o co ci chodzi z zmiennymi globalnymi w SINGLETON, skoro SINGLETON nie tworzy zmiennych globalnych


Olej singleton, popraw sobie troche te DatabaseClass na

[PHP] pobierz, plaintext 
class DatabaseClass
{
    protected static $connect;
 
 
    public function __construct()
    {
        try {
 
            if (!self::$connect) {
                $dbDSN = "mysql:host=" . DB_HOST . ";dbname=" . DB_DATABASE;
                $dbOptions = [
                    PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
                    PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8",
                ];
 
                self::$connect = new PDO($dbDSN, DB_USER, DB_PASSWORD, $dbOptions);
            }
        } catch (PDOException $exception) {
            throw new DbmException($exception->getMessage(), $exception->getCode());
        }
    }
// reszta co miales gdzies tam
 
}
[PHP] pobierz, plaintext 

I juz. Teraz sobie mozesz robic i milion razy new DatabaseClass() i Modeli z tego dzieciczacych a i tak ci zrobi tyklo jedno polaczenie PDO. I juz

Co do prawidlowego DI to trzeba by u cibie w pyte zmieniac a ja nie mam czasu teraz nad tym myslec. Chyba ze inna dobra dusza sie zdecuduje za to brac

[Malinaa]

Z protected static $connect to takie zakombinowane, lepiej bo jak rozumiem mamy teraz jedno połączenie, ale czy jest dobrze?!

Innych dusz kumatych to coś nie widać, więc daj znać jakbyś znalazł trochę czasu.

[nospor]

Z protected static $connect to takie zakombinowane, lepiej bo jak rozumiem mamy teraz jedno połączenie, ale czy jest dobrze?!

No jest troche zakombinowane, ale czasami trzeba podejsc do tematu pragmatycznie. Na chwile obecna to bedzie dobrze, bo dzieki temu unikasz tworzenia kilku polaczen do tej samej bazy danych wiec maszz poprawie o 470%* w porownaniu do tego co bylo

nnych dusz kumatych to coś nie widać, więc daj znać jakbyś znalazł trochę czasu.

ok



*tak wiem, wartosc wzialem z powietrza ale poprawa i tak jest

[viking]

https://github.com/artimman/dbmframework/bl...config.php.dist
Dlaczego zamiast użyć https://github.com/vlucas/phpdotenv to masz tego potwora?

w .gitignore masz vendor a jednak jest na serwerze.
Trochę mi się nie podoba że kontrolery nic nie zwracają, a sama metoda view robi include i na tym koniec. Takie wrażenie niezamkniętego requestu.

[Malinaa]

No jest troche zakombinowane, ale czasami trzeba podejsc do tematu pragmatycznie...

Podejście pragmatyczne w programowaniu, brzmi dość filozoficznie, być może przekracza moje doświadczenia. Wystarczy, że skoncentrujemy się na wspomnianej abstrakcji

* z powietrza? To chyba jakiś chwyt marketingowy, 470% kiedy 100% to max, może 47%, ewentualnie 4,7 tzn. 5 razy lepiej
i poprawa widoczna gołym okiem

config.php.dist - potwór?

1. Przy instalacji należy zmienić nazwę na config.php - nie wiem dlaczego to jest potwór?
2. O ile rozumiem chcesz zamienić config.php.dist na .env
Podałeś jednak od razu link na jakąś petardę, może jakiś przykład dla zielonych (nie git gotowiec, a czysty kod), bo mam mieszane uczucia, nie wiem co chcesz zrobić.
3. Tak racja (musiałem podejść do tematu pragmatycznie), z katalogiem vendor rzecz zrobiła się bardziej złożona, mogę próbować wytłumaczyć jeśli trzeba, ale przeczytaj README.md co powinno rozjaśnić kłopot. którym teoretycznie można by się nie przejmować... ale odbiorca (klient) jest bardzo istotny.

Też mi się trochę nie podoba rozwiązanie z kontrolerami i metodami (do poprawki), ale nie mam coś pomysłu na lepsze.
Jak masz pomysł na poukładanie kodu w stabilną całość - podaj koniecznie rozwiązanie problemu, a będę mówił Ci Królu przez cały tydzień.

Ten post edytował Malinaa 11.01.2024, 12:21:45

[nospor]

Podejście pragmatyczne w programowaniu, brzmi dość filozoficznie,

A ja jakos w robocie dosc czesto musze do tematu podchodzic pragmatycznie bo zrobienie czegos w sposob podrecznikowy, poprostu zajmie 10x dluzej czasu, albo bedzie wolniejsze albo to albo tamto.Takze wiedza wiedzą ale umiejetnosc adaptacji do panujacych warunkow tez wazna