Правильно ли я строю свое приложение ?

[So-cold]

Вот уже второй день не покидает мысль о том, что я не правильно делаю логику в приложение:
Сделал класс соединения с бд и с методами выборки :

<?php
namespace Modules;

use PDO;

class MySqlDatabase
{
    protected $_db;
    public $last_query;
    const DB_DSN = 'mysql:host=localhost;dbname=photo_gallery';
    const DB_USER = "admin";
    const DB_PASS = "123qwe";

    //Connection to DB
    public function __construct()
    {
        try {
            $this->_db = new PDO(self::DB_DSN, self::DB_USER, self::DB_PASS);
            $this->_db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        } catch (\PDOException $e) {
            echo "Connection error: " . $e->getMessage();
            exit;
        }
    }

    //Close connection
    public function __destruct()
    {
        unset($this->_db);
    }

    //Our query without result
    public function exec($sql)
    {
        try {
            $res = $this->_db->exec($sql);
            return $res;
        } catch (\PDOException $e) {
            echo $e->getMessage() . "<br/>";
            echo $this->last_query = $sql;
        }
    }

    //Our query from Select
    public function query($sql)
    {
        try {
            $stmt = $this->_db->query($sql);
            $row = $stmt->fetch(PDO::FETCH_ASSOC);
            return $row;
        } catch (\PDOException $e) {
            echo "Query Select error: " . $e->getMessage() . "<br/>";
            echo $this->last_query = $sql;
        }
    }

    //PDO number of rows affected by last SQL
    public function rowCount()
    {
        $row = $this->_db->prepare();
        $row->execute();
        $count = $row->rowCount();
        return $count;
    }

    //PDO last insert
    public function lastInsert()
    {
        return $this->_db->lastInsertId();
    }

    //Prepare statement Insert
    public function prepareInsert($sql, $username, $pass, $fname, $lname)
    {
        try {
            $stmt = $this->_db->prepare($sql);
            $stmt->bindParam(":username", $username, PDO::PARAM_STR);
            $stmt->bindParam(":password", $pass, PDO::PARAM_STR);
            $stmt->bindParam(":first_name", $fname, PDO::PARAM_STR);
            $stmt->bindParam(":last_name", $lname, PDO::PARAM_STR);
            $stmt->execute();
        } catch (\PDOException $e) {
            echo "Error prepare: " . $e->getMessage() . "<br/>";
            echo $this->last_query = $sql;
        }
    }

    //Prepare statement SelectALL
    public function prepareSelectAll($sql, $class)
    {
        try {
            $stmt = $this->_db->prepare($sql);
            $stmt->execute();
            $data = $stmt->fetchAll(PDO::FETCH_CLASS, $class);
            return $data;
            } catch (\PDOException $e) {
            echo "Error prepareALL: " . $e->getMessage() . "<br/>";
            echo $this->last_query = $sql;
        }
    }

    //Prepare statement By ID
    public function prepareSelectById($sql, $var1, $class)
    {
        try {
            $stmt = $this->_db->prepare($sql);
            $stmt->bindParam(":id", $var1, PDO::PARAM_INT);
            $stmt->execute();
            $data = $stmt->fetchObject($class);
            return $data;
        } catch (\PDOException $e) {
            echo "Error prepareById: " . $e->getMessage() . "<br/>";
            echo $this->last_query = $sql;
        }
    }
    public function prepareSelectByIdAuthenticate($sql, $username,$password, $class)
    {
        try {
            $stmt = $this->_db->prepare($sql);
            $stmt->bindParam(":username", $username, PDO::PARAM_STR);
            $stmt->bindParam(":password", $password, PDO::PARAM_STR);
            $stmt->execute();
            $data = $stmt->fetchAll(PDO::FETCH_CLASS, $class);
            return $data;
        } catch (\PDOException $e) {
            echo "Error prepareByIdAuthenticate: " . $e->getMessage() . "<br/>";
            echo $this->last_query = $sql;
        }
    }
}

И второй класс юзер, который наследуют класс работы с бд и использует у себя некоторые его методы:

<?php
namespace Modules;
class User extends MySqlDatabase
{
    public $id;
    public $username;
    public $password;
    public $first_name;
    public $last_name;

    //Select all
    public function findAll()
    {
        $sql = "SELECT * FROM users";
        $result_set = $this->prepareSelectAll($sql, __CLASS__);
        return $result_set;
    }

    //Find by id
    public function findById($id = 0)
    {
        $sql = "SELECT * FROM users WHERE id =:id LIMIT 1";
        $resuls_set = $this->prepareSelectById($sql, $id, __CLASS__);
        return $resuls_set;

    }

    // Function return full name
    public function full_name()
    {
        if (!empty($this->first_name) && !empty($this->last_name)) {
            return $this->first_name . " " . $this->last_name;
        } else {
            return "";
        }
    }

    //
    public function authenticate($username = "", $password = "")
    {
        $sql = "SELECT * FROM users ";
        $sql .= " WHERE username = :username ";
        $sql .= " AND password = :password ";
        $sql .= " LIMIT 1";
        $resuls_set = $this->prepareSelectByIdAuthenticate($sql, $username, $password, __CLASS__);
        if (!empty($resuls_set)) {
            return array_shift($resuls_set);
        } else {
            return false;
        }

    }

}

 

[WMix]

так се, не очень, но для пробы пера годится...
выкини наследование для начала

class User{

  private $db;

  // на этом месте лучше иметь Database-interface (не MySqlDatabase), но с твоими голыми sql-запросами смысла большого не имеет
  public function __construct( MySqlDatabase $db){
    $this->db = $db;
  }
  public function findAll()
  {
      $sql = "SELECT * FROM users";
      $result_set = $this->db->prepareSelectAll($sql, __CLASS__); // __CLASS__ это не дурь, выкинь
      return $result_set;
    }
//...
}

class MySqlDatabase
{
    protected $_db;
    public $last_query;


    //Connection to DB
    public function __construct(\PDO $pdo)
    {
            $this->_db = $pdo;
    }
// ...
}

//это вырезать, чтоб несколько баз иметь
$DB_DSN = 'mysql:host=localhost;dbname=photo_gallery';
$DB_USER = "admin";
$DB_PASS = "123qwe";
new MySqlDatabase( new PDO($DB_DSN, $DB_USER, $DB_PASS));

 

[So-cold]

так се, не очень, но для пробы пера годится...
выкини наследование для начала

class User{

  private $db;

  // на этом месте лучше иметь Database-interface (не MySqlDatabase), но с твоими голыми sql-запросами смысла большого не имеет
  public function __construct( MySqlDatabase $db){
    $this->db = $db;
  }
  public function findAll()
  {
      $sql = "SELECT * FROM users";
      $result_set = $this->db->prepareSelectAll($sql, __CLASS__); // __CLASS__ это не дурь, выкинь
      return $result_set;
    }
//...
}

class MySqlDatabase
{
    protected $_db;
    public $last_query;


    //Connection to DB
    public function __construct(\PDO $pdo)
    {
            $this->_db = $pdo;
    }
// ...
}

//это вырезать, чтоб несколько баз иметь
$DB_DSN = 'mysql:host=localhost;dbname=photo_gallery';
$DB_USER = "admin";
$DB_PASS = "123qwe";
new MySqlDatabase( new PDO($DB_DSN, $DB_USER, $DB_PASS));

Спасибо , еще один вопрос:

но с твоими голыми sql-запросами смысла большого не имеет

Не совсем понял что вы этим хотели сказать. Я запросы кидаю через prepare в классе (MySqlDatabase). Или я не в ту степь подумал?
И второе хотел спросить:

  $result_set = $this->db->prepareSelectAll($sql, __CLASS__); // __CLASS__ это не дурь, выкинь

prepareSelectAll - просто имя класса User указать ?

 

[AnrDaemon]

Зачем тебе вообще mysqldatabase? Используй PDO напрямую.

 

[antson]

@So-cold, слабое подобие доктрины получиться.
И вот такое чувство, что без твоих классов писать будет быстрее и понятнее.

 

[So-cold]

Зачем тебе вообще mysqldatabase? Используй PDO напрямую.

Я поэтому и спросил как лучше делать и как правильнее?. Я думал мб наследовать PDO в свой класс BD и там если нужны будут правки исправлять. Но подумал что это плохой вариант вот и спросил у Вас знающих .

 

[Фанат]

Для начала: https://phpdelusions.net/pdo/common_mistakes
Если по-английски непонятно, то гуголь дает довольно внятный перевод
После прочтения переписать код и предоставить на проверку.

 

[So-cold]

Для начала: https://phpdelusions.net/pdo/common_mistakes
Если по-английски непонятно, то гуголь дает довольно внятный перевод
После прочтения переписать код и предоставить на проверку.

Хорошо, спасибо

 

[WMix]

Не совсем понял что вы этим хотели сказать.

QueryBuilder

prepareSelectAll - просто имя класса User указать ?

немножко грязновато даже не пойму, что с таким object делать, там не будет $db к примеру..

 

[Фанат]

В итоге от класса должен остаться только 1 метод для выполнения запроса с параметрами. все остальное либо не нужно, либо относится к юзеру.
То есть можно спокойно унаследоваться от ПДО:

class MyPDO extends PDO

{
    public function __construct($dsn, $username = NULL, $password = NULL, $options = [])
    {
        $default_options = [
            PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
            PDO::ATTR_EMULATE_PREPARES => false,
            PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        ];
        $options = array_replace($default_options, $options);
        parent::__construct($dsn, $username, $password, $options);
    }
    public function run($sql, $args = NULL)
    {
        if (!$args)
        {
             return $this->query($sql);
        }
        $stmt = $this->prepare($sql);
        $stmt->execute($args);
        return $stmt;
    }
}

создаешь ОДИН инстанс этого класса и дальше передаешь его в конструктор

сlass User
{
    public function __construct(\MyPDO $pdo)
    {
            $this->_db = $pdo;
    }

    //Select all
    public function findAll()
    {
        $sql = "SELECT * FROM users";
        return $this->_db->run($sql)->fetchAll(PDO::FETCH_CLASS, __CLASS__);
    }

    //Find by id
    public function findById($id = 0)
    {
        $sql = "SELECT * FROM users WHERE id =? LIMIT 1";
        return $this->_db->run($sql, [$id])->fetchObject(__CLASS__);
    }
}

Хотя если задуматься, то метод findAll будет болтаться без дела. Нет ни одного места в приложении, где понадобилось бы выбирать всех юзеров подряд.

 

[AnrDaemon]

array_replace
Нельзя использовать aray_merge с числовыми ключами.
Вернее, в данном случае нельзя.

 

[Фанат]

Хотя

array_replace
Нельзя использовать aray_merge с числовыми ключами.
Вернее, в данном случае нельзя.

Вот спасибо. Знатно я затупил. И ведь знал же пр эту особенность, но ничего в голове не зазвенело, когда копировал код из safemysql

 

[So-cold]

В итоге от класса должен остаться только 1 метод для выполнения запроса с параметрами. все остальное либо не нужно, либо относится к юзеру.
То есть можно спокойно унаследоваться от ПДО:

class MyPDO extends PDO

{
    public function __construct($dsn, $username = NULL, $password = NULL, $options = [])
    {
        $default_options = [
            PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
            PDO::ATTR_EMULATE_PREPARES => false,
            PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        ];
        $options = array_replace($default_options, $options);
        parent::__construct($dsn, $username, $password, $options);
    }
    public function run($sql, $args = NULL)
    {
        if (!$args)
        {
             return $this->query($sql);
        }
        $stmt = $this->prepare($sql);
        $stmt->execute($args);
        return $stmt;
    }
}

создаешь ОДИН инстанс этого класса и дальше передаешь его в конструктор

сlass User
{
    public function __construct(\MyPDO $pdo)
    {
            $this->_db = $pdo;
    }

    //Select all
    public function findAll()
    {
        $sql = "SELECT * FROM users";
        return $this->_db->run($sql)->fetchAll(PDO::FETCH_CLASS, __CLASS__);
    }

    //Find by id
    public function findById($id = 0)
    {
        $sql = "SELECT * FROM users WHERE id =? LIMIT 1";
        return $this->_db->run($sql, [$id])->fetchObject(__CLASS__);
    }
}

Хотя если задуматься, то метод findAll будет болтаться без дела. Нет ни одного места в приложении, где понадобилось бы выбирать всех юзеров подряд.

Спасибо я понял как теперь нужно, но у меня вопрос есть(где-то час уже пытался решить сам но )

$myPdo= new \Modules\MyPDO(DB_DSN,DB_USER,DB_PASS,DB_OPT);
$user = new \Modules\User1($myPdo);
var_dump($user->findById(2));
//bool(false)

А во второй

var_dump($user->findAll());
////////
Fatal error: Uncaught Error: Class 'Modules\PDO' not found in

 return $this->_db->run($sql)->fetchAll(PDO::FETCH_CLASS, __CLASS__);

 

[Фанат]

Спасибо я понял как теперь нужно, но у меня вопрос есть

Ну здесь я просто слеш забыл перед PDO::FETCH_CLASS

return $this->_db->run($sql)->fetchAll(\PDO::FETCH_CLASS, __CLASS__);

 

[So-cold]

Ну здесь я просто слеш забыл перед PDO::FETCH_CLASS

return $this->_db->run($sql)->fetchAll(\PDO::FETCH_CLASS, __CLASS__);

Fatal error: Uncaught ArgumentCountError: Too few arguments to function Modules\User1
Через класс MyPDO только что вытаскивал данные из БД. Даже не знаю что ему не нравится тут ТТ

 

[Фанат]

Fatal error: Uncaught ArgumentCountError: Too few arguments to function Modules\User1
Через класс MyPDO только что вытаскивал данные из БД. Даже не знаю что ему не нравится тут ТТ

Судя по ошибке, она не имеет отношения к этому коду.
На какую точно строку оно указывает?

 

[So-cold]

Судя по ошибке, она не имеет отношения к этому коду.
На какую точно строку оно указывает?

return $this->_db->run($sql)->fetchAll(\PDO::FETCH_CLASS, __CLASS__);

 

[Фанат]

странно.
А если так написать, то на какую строчку укажет?

return $this->_db
              ->run($sql)
              ->fetchAll(\PDO::FETCH_CLASS, __CLASS__);

 

[So-cold]

странно.
А если так написать, то на какую строчку укажет?

return $this->_db
              ->run($sql)
              ->fetchAll(\PDO::FETCH_CLASS, __CLASS__);

->fetchAll(\PDO::FETCH_CLASS, __CLASS__);

 

[So-cold]

Вообще такой вопрос, когда пишу:

$myPdo= new \Modules\MyPDO(DB_DSN,DB_USER,DB_PASS,DB_OPT);
$sql="SELECT * FROM users WHERE id=? LIMIT 1";
$vv=$myPdo->run($sql, [1])->fetchObject(__CLASS__);
var_dump($vv);
//Fatal error: Class '' not found in

И так если пишу тоже не пашет

$vv=$myPdo->run($sql, [1])->fetchObject("MyPDO");

Подключаю классы composerom. Не понимаю почему он не видит эти классы.