Оцените класс для работы с БД?

Question

[pornolio]

Вот собственно класс

<?php

class DB{

    private $dsn;
    private $settings;
    private $connection = false;

    public function __construct($options = null) {
        $this->settings = parse_ini_file(CORE_PATH . DS . 'config' . DS . 'db.ini.php');
        $this->dsn = $this->settings["db_driver"] . ':host=' . $this->settings["db_host"] . ';dbname=' . $this->settings["db_name"];
        $this->connection = new PDO($this->dsn, $this->settings["db_user"],$this->settings["db_pass"], $options);
        $this->connection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        $this->connection->exec("set names utf8");
    }

    public function select($db, $cols, $where = null, $order = null, $limit = null) {
        $sql = "SELECT {$cols} FROM `{$db}` {$where} {$order} {$limit}";
        $result = $this->connection->query($sql);
        $result->execute();
        if($where != null){
            $row = $result->fetchAll(PDO::FETCH_ASSOC);
            return $row;
        } else {
            $row = $result->fetchAll(PDO::FETCH_ASSOC);
            return $row;
        }
    }

    public function insert($db, $cols = "", $values = "") {
        if($cols == "") echo "Ошибка!";
        $sql = "INSERT INTO `{$db}`({$cols}) VALUES ({$values})";
        $this->connection->query($sql);
    }

    public function update($db, $what, $value, $where) {
        $sql = "UPDATE `{$db}` SET {$what}={$value} {$where}";
        $this->connection->query($sql);
    }

    public function delete($db,$where = "") {
        $sql = "DELETE FROM  `{$db}` {$where}";
        return $this->connection->query($sql);
    }

    public function __destruct() {
        $this->connection = null;
    }
}

Answer 1

[FanatPHP]

В двух словах - бессмысленный и небезопасный.

Класс не использует главные преимущества PDO - подготовленные выражения и возможность возвращать результат в десятках различных форматов. В итоге мы имеем SQL инъекцию в каждом запросе, а результат всегда возвращается только в виде списка.

При этом написана куча функций, чтобы сэкономить себе два слова при написании SQL запроса, ценой создания нечитаемой абракадабры.
Попробуй написать пример использования функции select() для запроса "SELECT access FROM user WHERE id=1" , а потом посмотри на него и честно скажи - можно ли без документации понять, что и откуда эта функция должна выбрать и с какими условиями.

Про бессмысленные телодвижения в select() я уж и не говорю.
То, что запрос зачем-то выполняется по два раза, а после этого идет странное условие с одинаковым кодом в каждой ветке - это уже мелочи.

Answer 2

[Василий Назаров]

Тактически:
Как минимум, передавать в каждый метод $db - криво, $db лучше сделать приватным и инициировать в конструкторе.
Ну и запросы небезопасные, выше отмечали.
Стратегически:
Такой класс пишет почти любой начинающий программист. И он никакой пользы не несёт обычно, ибо непонятно, зачем вообще этот класс.
Если синтаксический сахарок - спасибо, мне такого не надо.
Если какой-то уровень абстракции, то нужен не класс "для работы с БД", а ORM.
Если ORM - имело бы смысл хотя бы чтобы insert и update принимали одинаковый массив [field => value], а update и delete - PK изменяемой/удаляемой записи вместо $where.
А ещё, почитайте доку по Laravel 5 и/или Yii2, узнаете много полезного и, видимо, нового для вас.

Comments

[FanatPHP]

По коду видно, что $db - это на самом деле имя таблицы. Тут скорее претензия к именованию.

[Василий Назаров]

Я вижу, что это имя таблицы. А как это влияет на моё замечание? Кроме того, что и ваше замечание о [не том] названии справедливо?

[FanatPHP]

Не канает. Делать имя таблицы приватным в этом классе так же бессмысленно :)

[Василий Назаров]

Вполне канает, хотя это вопрос дизайна.
Я имел в виду что-то типа такого:
$user = new DB('tbl_user');
$user->delete(101);

[FanatPHP]

Ну так это уже ORM, которым, по вашим собственным словам этот класс не является.
Не говоря уже о том, что в текущей реализации это убьет сервер по too many connections