Оцените класс работы с БД на говнокод.

Это говнокод?


  • Всего проголосовало
    17

mr.zherart

Новичок
Здравствуйте, написал собственный велосипед. Стараюсь не писать говнокод. Оцените насколько тут получилось. И конечно же, если это плохой код, укажите на ошибки! Спасибо!

PHP:
class DB extends mysqli  {
    private $typeServer = 0; // 0 - Locale, 1 - Real
    static $instances = 0; // 0 - connection not set, 1 - connection already set
    private $connect;
  
    /* Params for real Server (rServer)*/
    private $rServer = "";
    private $rUser = "";
    private $rPassword = "";
    private $rDatabase = "";
  
    /* Params for Local Server (lServer) */
    private $lServer = "localhost";
    private $lUser = "root";
    private $lPassword = "";
    private $lDatabase = "develophouse";

    function __construct(){

        /* Connect to DB*/
        if($this->typeServer == 0 && DB::$instances == 0){
            $this->connect = new MySQLi($this->lServer, $this->lUser, $this->lPassword, $this->lDatabase);
        }elseif($this->typeServer == 1 && DB::$instances == 0){
            $this->connect = new MySQLi($this->rServer, $this->rUser, $this->rPassword, $this->rDatabase);
        }else{
            echo "Connection already set, close connection";
        }
      
        $this->connect->set_charset("utf8");

        if($this->connect->errno){
            exit('Connection to server unpossible. Error code: '.$this->connect->error.'\n');
        }else{DB::$instances = 1;}
    }

    /*
    *
    * @query - query string to db
    * @arrat_type - type of return:
    *    simple,single - just one string
    *    assoc, row, object - one array
    *    mysqli_* all query data
    */

    function do_query($query, $array_type=''){
        $result = $this->connect->query($query);
        if($result){
            switch ($array_type){
                case 'simple':
                case 'single':
                    return implode($result->fetch_row());
                case 'assoc':
                case 'row':
                case 'object':
                    return $result->{'fetch_'.$array_type}();
                case MYSQLI_ASSOC:
                case MYSQLI_NUM:
                case MYSQLI_BOTH:
                    return $result->fetch_all($array_type);
                default:
                    return $result;
            }
        }else{
            return $this->connect->error;      
        }
    }

    /*
    * @arr - number, string or array to protect from SQL injection
    */

    function protect_query($arr){
        switch ($arr){
            case (is_array($arr)):
                $result = $this->inArray($arr);
                break;
            case (is_numeric($arr)):
                return sprintf('%d', $arr);
            case (is_string($arr)):
                return mysql_real_escape_string($arr);
        }
        return $result;
    }

    private function inArray($arg){
        foreach ($arg as $key => $value) {
            switch ($value){
                case (is_array($value)):
                    $protected[$key] = $this->inArray($value);
                    break;
                case (is_numeric($value)):
                    $protected[$key] = sprintf('%d', $value);
                    break;
                case (is_string($value)):
                    $protected[$key] = mysql_real_escape_string($value);
                    break;
            }
        }
        return $protected;
    }
      
    function closeDB(){
        $this->connect->close();
        DB::$instances = 0;
    }
}
 
Последнее редактирование:
mr.zherart, почитай по psr, phpdoc, неймспейсы, phpunit, а то даже смотреть на оформление неприятно, нее говоря уже о функциональности.
 
private $typeServer = 0; // 0 - Locale, 1 - Real
Какая еще locale?

А float ты, простите, как обрататывать будешь?

Ну и 1 метод простите для того, чтобы обрабатывать все запросы? Удобнее иметь fetch() fetchAll() по крайней мере.
 
c0dex, locale - бред, там должно быть localhost.
Float обязательно обработаю.
Объясните, пожалуйста, чем удобнее иметь fetch() и fetchAll() раздельно? Зачем писать лишнюю функцию. Спасибо!

AmdY, читаю про psr и дико неудобно делать вместо tab 4 пробела. Ну раз так надо. Спасибо. Обязательно исправлюсь и выложу здесь код по приятней.
 
mr.zherart, если бы твое оформление от psr отличалось только табом, никто бы слова не сказал на эту тему.

По теме же, посмотри в результат голосовалки. Польза от такого класса неясна, даже убогий PDO предоставляет намного более удобное api.
 
А зачем было наследоваться от mysqli?

Наследование без причины — признак дурачины.
 
fixxxer, я не спорю у меня куча ошибок по всему и по psr и неясность по функционалу, и PDO конечно круче. Но, но, но... я учусь ведь.
 
mr.zherart, учиться надо на чем-то полезном. Возьми фреймворк популярный и напиши блог/rest-api-под-что-угодно, добавь туда админку на этом фреймворке. А писать сомнительный синглтон под базу - ни разу не полезно.
PS сейчас чтобы тебя не загнобили за синглтон, принято писать нормальную реализацию и заворачивать в "фасад". Типо можно и так и так юзать, эдакий сознательный говнокод.
 
Прям в библиотеке? Заворачивать в "фасад"? Нафига?
 
Как это нафига? Умное слово же. Еще обджект пул есть. Ну или визитор.
 
Зачем нужен пул соединений или визитор в библиотеке - это как раз понятно. Для реализации двухфазных коммитов в рамках пула, например.

"Фасад" же, в том смысле, в каком он в Laravel (а именно это тут и имеется ввиду, я не просто так поставил слово в кавычки) - это в чистом виде dependency management, и этому нечего делать в подключаемой куда угодно библиотеке.
 
https://php.net/manual/en/control-structures.switch.php
Код:
private function inArray($arg){
        foreach ($arg as $key => $value) {
            switch ($value){
                case (is_array($value)):
                    $protected[$key] = $this->inArray($value);
                    break;
                case (is_numeric($value)):
                    $protected[$key] = sprintf('%d', $value);
                    break;
                case (is_string($value)):
                    $protected[$key] = mysql_real_escape_string($value);
                    break;
            }
        }
        return $protected;
    }

придерживайся стандартов OOP и старайся по меньше использовать if, switch там, где можно обойтись без них

у тебя ошибка в логике
 
Я бы посоветовал параметры подключения и логику определения БД вынести за класс. Т.е.:
PHP:
if($a == $b){
  $param = array('host_name1', 'user1', 'passwd1', 'db_name1');
} else {
  $param = array('host_name2', 'user2', 'passwd2', 'db_name2');
}

$db = new DB($param);
 
Сверху