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

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


  • Всего проголосовало
    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;
    }
}
 
Последнее редактирование:

AmdY

Пью пиво
Команда форума
mr.zherart, почитай по psr, phpdoc, неймспейсы, phpunit, а то даже смотреть на оформление неприятно, нее говоря уже о функциональности.
 

c0dex

web.dev 2002-...
Команда форума
Партнер клуба
private $typeServer = 0; // 0 - Locale, 1 - Real
Какая еще locale?

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

Ну и 1 метод простите для того, чтобы обрабатывать все запросы? Удобнее иметь fetch() fetchAll() по крайней мере.
 

mr.zherart

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

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

fixxxer

К.О.
Партнер клуба
mr.zherart, если бы твое оформление от psr отличалось только табом, никто бы слова не сказал на эту тему.

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

Вурдалак

Продвинутый новичок
А зачем было наследоваться от mysqli?

Наследование без причины — признак дурачины.
 

mr.zherart

Новичок
fixxxer, я не спорю у меня куча ошибок по всему и по psr и неясность по функционалу, и PDO конечно круче. Но, но, но... я учусь ведь.
 

hell0w0rd

Продвинутый новичок
mr.zherart, учиться надо на чем-то полезном. Возьми фреймворк популярный и напиши блог/rest-api-под-что-угодно, добавь туда админку на этом фреймворке. А писать сомнительный синглтон под базу - ни разу не полезно.
PS сейчас чтобы тебя не загнобили за синглтон, принято писать нормальную реализацию и заворачивать в "фасад". Типо можно и так и так юзать, эдакий сознательный говнокод.
 

fixxxer

К.О.
Партнер клуба
Прям в библиотеке? Заворачивать в "фасад"? Нафига?
 

HraKK

Мудак
Команда форума
Как это нафига? Умное слово же. Еще обджект пул есть. Ну или визитор.
 

fixxxer

К.О.
Партнер клуба
Зачем нужен пул соединений или визитор в библиотеке - это как раз понятно. Для реализации двухфазных коммитов в рамках пула, например.

"Фасад" же, в том смысле, в каком он в Laravel (а именно это тут и имеется ввиду, я не просто так поставил слово в кавычки) - это в чистом виде dependency management, и этому нечего делать в подключаемой куда угодно библиотеке.
 

accido

Новичок
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 там, где можно обойтись без них

у тебя ошибка в логике
 

che

Новичок
Я бы посоветовал параметры подключения и логику определения БД вынести за класс. Т.е.:
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);
 
Сверху