code review php – Estructura de envoltura de DOP

Leave a Comment / code review php / By

Pregunta:

¿Es dinámica esta estructura de envoltura de DOP? ¿Puede el uso de self:: lugar de $this causar problemas?

class Database
extends PDO
{
/*@var  mixed $stmt     Used as a temporary variable to hold queries*/
private $stmt = Null;

/*@var $scopeSelectVar  Used as a temporary variable to hold queries*/
private $scopeSelectVar = Null;

/*@Constructor __construct
 *
 * Database __construct ( mixed $db, [ string $serverHost = Null], [ string $dbName = Null],
 * [ string $dbUser = Null], [ string $password = Null], [ bool $persistent = Null])
 *
 *  @access-public
 */
public function __construct($db,
    $serverHost = Null,
    $dbName = Null, $dbUser = Null,
    $password = Null, $persistent = true)
{
    try
    {
        if(is_array($db))
        {
        /*
        * parent::_construct allows easy instantiation and multiple database connections.
        * @param    mixed $db   When array(), var holds all parameters required to connect.
        *                       When string, var holds the type of DB to connect to. E.g. mysql
        *
        * @param        string $serverHost      the host of the DB connection
        * @param        string $dbName          the name of the database
        * @param        string $dbUser          the respective name of the user
        * @param        string $password        the password of the user connecting
        * @param        bool   $persistent      if true, allows connection to persist through the app
        *
        */
            parent::__construct("{$db['dbType']}:host={$db['Host']};
                        dbname={$db['dbName']}",
                        $db['dbUser'],
                        $db['dbPassKey'],
                        array(
                            PDO::ATTR_PERSISTENT => $persistent,
                        ));

        }//if Var $db == array() then connect using array parameters
        else
        {
            //else connect using all the other function parameters
            parent::__construct("{$db}:host={$serverHost};
                        dbname={$dbName}", $dbUser, $password,
                        array(
                            PDO::ATTR_PERSISTENT => $persistent,
                        ));
        }
    /*Uncomment if you have a proper log class*/
        Log::write('../AppLogs/databaseConn.log', 'Connection
            Established at:');
    }
    catch(PDOException $e)/*catching pdo exception and writing it to a file*/
    {

    /*Uncomment if you have a proper log class*/
        Log::write('../AppLogs/databaseErrors.log',
            'Error:'
            .$e->getMessage());
    }
}


/*
 * @access private
 *
 * void defineParamType(string $val)
 *
 * */
private function defineParamType($val)
{
    /*@param string $val    the update or insert query data passed*/
    /*@return const $param  the param type constant returned from the function*/
    switch($val):

        case (is_int($val)):
            $param = PDO::PARAM_INT;
            break;

        case (is_string($val)):
            $param = PDO::PARAM_STR;
            break;

        case (is_bool($val)):
            $param = PDO::PARAM_BOOL;
            break;

        case (is_Null($val)):
            $param = PDO::PARAM_Null;
            break;

        default:
            $param = Null;
    endswitch;

    return $param;
}


/*
 * @access private
 *
 * void insertPrepare(array $bindData)
 * */
private function insertPrepare($bindData)
{
    /* @param array $bindData       Data to be prepared for binding
     * @return array $insertArray
     * */
    ksort($bindData);

    $insertArray = array(
                'fields' => implode("`,`", array_keys($bindData)),
          'placeholders' => ':'.implode(',:',array_keys($bindData)),
    );

    return $insertArray;
}


/*
 * @access private
 *
 * void updatePrepare(array $bindData)
 * */
private function updatePrepare($bindData)
{
    /*
     * @param array $bindData   Data to be prepared for binding
     * @return string $placeHolders
     * */
    ksort($bindData);

     $placeHolders = Null;
    foreach($bindData as $key => $val)
    {
        $placeHolders .= "`$key`=:$key, ";
    }

    $placeHolders = rtrim($placeHolders, ', ');
    return $placeHolders;
}


private function where($arguments = array(), $joinKeyword = 'AND')
{
    ksort($arguments);

        $whereClause = Null;
    foreach($arguments as $key => $val)
    {
        if(is_int($val))
        {
            $whereClause .= "`$key` = $val {$joinKeyword} ";
        }
        else
        {
            $whereClause .= "`$key` = '$val' {$joinKeyword} ";
        }
    }

    $whereClause = rtrim($whereClause, ' '.$joinKeyword.' ');
    $whereClause = "WHERE {$whereClause}";
    return $whereClause;
}


/*
 * @access public
 *
 * void query(string $sql)
 * */
public function query($sql)
{
    /*
     * @param string $sql       the sql command to execute
     * */
    $this->scopeSelectVar = NULL;
    $this->stmt = parent::query($sql);
}


/*
 * @access public
 *
 * void insertRow(string $tableName, string $bindData)
 *
 * $array = array('field1'=>'field1Value')<-Notice the abscence of ":"
 *      $handler->insertRow('table', $array)
 *
 * */
public function insertRow($tableName, $bindData)
{
    /*
     * @param string $tableName     Name of the table that is inserted into
     * @param   array $bindData     array holding the set of column names
     *                              respective data to be inserted
     * */
    try
    {
        $insertData = self::insertPrepare($bindData);

        $this->stmt = parent::prepare("INSERT INTO
                        `{$tableName}` (`{$insertData['fields']}`)
                        VALUES({$insertData['placeholders']})");

        foreach($bindData as $key => $val)
        {
            $param = self::defineParamType($val);

            $this->stmt->bindValue(":$key", $val, $param);
        }

        $query = $this->stmt->execute();
    }
    catch(PDOException $e)
    {

    }
}


/*
 * @access public
 *
 * void updateRow(string $tableName, array $bindData, string $target)
 *
 * Way of use: to update
 *  $array = array('field1'=>'field1Value')<-Notice the abscence of ":"
 *      $handler->updateRow('table', $array, '`field2`='Val'')
 * */
public function updateRow($tableName, $bindData, $target, 
    $targetClause = 'AND')
{
    /*
     * @param string $tableName     The name of the table you're updating
     * @param array $bindData       array of the values to be inserted.
     *                              includes $_POST and $_GET
     * @param string $target        The exact update target. I.e.
     *                              WHERE id='?'
     * */
    try
    {
        $updateData = self::updatePrepare($bindData);
        if(isset($target))
        {
            $target = self::where($target, $targetClause);
        }
        $this->stmt = parent::prepare("UPDATE {$tableName}
                        SET {$updateData} {$target}");
        foreach($bindData as $key => $val)
        {
            $param = self::defineParamType($val);

            $this->stmt->bindValue(":$key", $val, $param);
        }

        $this->stmt->execute();
    }
    catch(PDOException $e)
    {

    }
}

/*
 * @access public
 *
 * void deleteRow(string $tableName, string $target)
 * */
public function deleteRow($tableName, $target)
{
    /*
     * @param string $tableName table to be deleted from
     * @param string $target  target of the delete query
     * */
    try
    {
        return parent::exec("DELETE FROM {$tableName} WHERE
                                {$target}");
    }
    catch(PDOException $e)
    {

    }
}

/*
 * @access public
 *
 * void selectRow(string $fields, string $tableName, string $target)
 * */
public function selectRow($fields, $tableName, array $target = NULL
    , $targetClause = 'AND')
{
    /*
     * @param  string $fields   the fields of selection. E.g. '`field`,`field2`'...
     * @param  string $tableName The name of the target table
     * */
    if(isset($target))
    {
        $where = self::where($target, $targetClause);
    }else
    {
        $where = Null;
    }
    self::query("SELECT {$fields} FROM {$tableName} {$where}");

}

/*
 * @access public
 *
 * void fetch([string $singleReturn = false], [constant $fetchMode = PDO::FETCH_OBJ])
 * */
public function fetch($singleReturn = false,
    $fetchMode = PDO::FETCH_OBJ)
{
    /*
     * @param string $singleReturn  the name of the "single" field to be fetching
     * @param constant $fetchMode   The fetch mode in which the data recieved will be stored
     * @return mixed    Null when conditions are not met, stdClass(object) or string when
     *                  conditions are met.
     * */
    if(!isset($this->stmt)){return false;}

    if($singleReturn == true)
    {
        if($this->scopeSelectVar == false)
        {
            $this->scopeSelectVar = $this->stmt->fetch($fetchMode);

            if(isset($this->scopeSelectVar->$singleReturn))
            {
                return $this->scopeSelectVar->$singleReturn;
            }else
                return false;
        }
    }else
    {
        $this->scopeSelectVar = $this->stmt->fetch($fetchMode);
        return (isset($this->scopeSelectVar)) ?
            $this->scopeSelectVar: new StdClass;
    }
}

/*
 * @access public
 * 
 * void fetchAll([constant $fetchMode = PDO::FETCH_ASSOC])
 * */
public function fetchAll($fetchMode = PDO::FETCH_ASSOC)
{
    /*
     * @param constant $fetchMode Default is PDO::FETCH_ASSOC the mode of fetching data
     * */
    if(!isset($this->stmt)){return false;}

    $fetchVar = $this->stmt->fetchAll($fetchMode);

    return (!empty($fetchVar)) ? $fetchVar: new StdClass;

}

/*
 * @TODO    set a convenient method to quicly setup nested queries
 * */
public function setSubQuery($target, $subQuery,
    $mysqlSubQueryHandler)
{
    //mysql nested query handler
}

/*
 * @access public
 * 
 * void rowCount()
 * */
public function rowCount()
{
    /*
     * @return numeric $this->stmt->rowCount()
     * */
    if(isset($this->stmt))
    {
        return $this->stmt->rowCount();
    }
}

/*
 * @access public
 * 
 * void lastId()
 * 
 * */
public function lastId()
{
    if(isset($this->stmt))
    {
        return parent::lastInsertId();
    }
}

/*
 * @access public
 * 
 * void truncateTable(string $tableName)
 * */
public function truncateTable($tableName)
{
    /*
     * @param string $tableName     The name of table to be truncated
     * Notice: truncation will reset the table and delete the data
     * */
    return self::query("TRUNCATE TABLE {$tableName}");
    echo "Table {$tableName} Truncated on:".date("d-m-Y h:i:s")
        ."\n";
}

/*
 * @access public
 * void debugDumpParams()
 *
 * */
public function debugDumpParams()
{
    return $this->stmt->debugDumpParams();
}


public function rankTable($tableName, $rankableColumn, 
    $orderByColumn, $ascDesc='DESC', $sqlVarName = 'rankNum')
{
    self::query("SET @{$sqlVarName}:= 0");
    self::query("UPDATE `{$tableName}` SET {$rankableColumn}
        =@{$sqlVarName}:=@{$sqlVarName}+1 
        ORDER BY `{$orderByColumn}` {$ascDesc}");
}


public function 


public function __destruct()
{

}
}

Sé que probablemente no debería extenderme a PDO, pero se rediseñará toda la arquitectura. Solo necesito asegurarme de que las otras funciones estén bien codificadas.

Este es mi primer diseño de código OOP pesado. Por favor critique.

Respuesta:

Cuando se trata de hacer cosas como esta, es sorprendente lo rápido que se puede complicar y la cantidad de ideas y principios de programación orientada a objetos que pueden estar involucrados.

De todos modos, hay mucho que cubrir aquí, así que solo voy a resumir algunas cosas y puedo volver y editar más detalles más adelante.

Para un "primer diseño de código OOP pesado", esto es bastante bueno (probablemente mejor que el típico primer intento real), pero hay algunas cosas que se destacaron:

  • self:: es casi siempre una mala señal
    • Significa que una variable es básicamente un espacio de nombres global
    • Significa que un método no tiene absolutamente nada que ver con el estado de un objeto de la clase a la que pertenece. Esto puede tener sentido en algunas situaciones, pero es una decisión que debe considerarse cuidadosamente a menos que sea uno de los usos obvios.
    • Su código es incorrecto desde un punto de vista técnico porque no puede, bueno, no debería, ya que PHP es demasiado indulgente, llamar a un método no estático de forma estática. No tiene sentido $this y self no son intercambiables en absoluto.
  • Tu clase es específica de MySQL
    • Las comillas inversas son específicas de MySQL
    • Algunas otras cosas que soy demasiado vago para mirar hacia atrás a través del código y volver a encontrar 🙂
  • Está mezclando preocupaciones con la creación de consultas y la conexión a la base de datos.
    • prepare , query , query operaciones de nivel bastante bajo
    • crear declaraciones con métodos como where es una operación de nivel bastante alto
      • Si desea un DBAL, cree un DBAL. Un DBAL debe tener una conexión, no una conexión.
    • La gente, por alguna razón, siempre quiere extender la DOP. Hay situaciones en las que eso tiene sentido, sin embargo, el 99% de las veces no lo tiene.
      • Si desea una clase auxiliar de SQL, tenga una clase que use una instancia de PDO detrás de escena. No juntes tu clase de ayuda y tu clase de conexión.
  • Estás abusando de las excepciones
    • ¿Cómo sabe un consumidor de su clase que se produjo una PDOException? Casi nunca deberías comer una excepción. Si sus métodos fallan, es probable que el código consumidor deba saberlo.
  • Tu registro es defectuoso
    • Si alguna vez utiliza una clase como C::a(); C::b(); o si alguna vez se encuentra creando un nuevo objeto en un método, es probable que sea una señal de que la persona que llama debe proporcionar la dependencia.
      • Proporciona flexibilidad (¿qué pasa si alguien quiere tener dos instancias de su clase y usar dos registradores diferentes?)
      • Facilita el acoplamiento (la Database depende secretamente de PDO ; no es suficiente decirle a los consumidores que descomenten el código si tienen un registrador llamado Log )
    • Pase un registrador en su lugar que implemente una determinada interfaz
      • Permite flexibilidad como un registrador nulo (un registrador que simplemente descarta todo) o como tener un registro de instancia en una tabla de base de datos y otro en un CSV
    • Dependiendo del nivel bajo de su clase, es posible que un registrador no pertenezca allí.
      • Lo que quiero decir con esto es que hay un cierto nivel en el que el registro simplemente no tiene sentido. Las llamadas al sistema no se registran. Incluso PDO no se registra. Probablemente debería manejar el registro en el terreno de la aplicación donde la aplicación puede tener una mejor idea de cómo se debe manejar el registro.
  • Tu constructor mata la flexibilidad
    • SQL Server o Postgres, por ejemplo, tendrán DSN muy diferentes a MySQL
      • al tratar de ser demasiado útil, su constructor ha eliminado esta flexibilidad: hay una razón por la que PDO toma un DSN y no los parámetros que toma su constructor.
    • Abordan la persistencia, pero ¿qué pasa con los otros atributos?
  • Algunas notas de estilista (nota: estas son 100% de opinión)
    • Es bastante estándar aplicar sangría al primer nivel dentro de las clases.
    • switch: y switch: endswitch; se utilizan muy raramente. Mucha gente va a discutir con vehemencia conmigo sobre esto, pero creo que al endwhile; endif; y así sucesivamente, las construcciones son una mancha espantosa en el lenguaje C-esque.
    • Null casi siempre se escribe NULL o null en PHP.

En resumen, este es un buen comienzo, pero probablemente esté yendo por el camino equivocado con esta clase. Aquí hay muchas fallas técnicas y de diseño que deben abordarse. Sugeriría aprender un poco más la sintaxis de OO y tratar de familiarizarse mejor con las filosofías de bajo nivel y las ideas de programación orientada a objetos (cuando tenga más tiempo, volveré y proporcionaré algunos enlaces).

Leave a Comment

Your email address will not be published. Required fields are marked *

web tasarım