HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Is this a complete PDO prepared class?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thiscompletepreparedclasspdo

Problem

Is this class perfect for databases? All I want to know is what I should change to make it perfect.

```
error_reporting(E_ALL);
class RESPETO{
private $_server = 'mysql';
private $_host = '';
private $_database = '';
private $db_username = '';
private $db_password = '';
private $db_charset = 'utf8';
private $conn = NULL;
const FETCH = 0;
const FETCHALL = 3;
const ID = 1;
const ROWCOUNT = 2;
const PARAM_LOB = 'largeObject';
public function SQL( $Query, $value = array(), $req = RESPETO::FETCHALL ){
try{
$pattern = "/^(SELECT)|^(INSERT)|^(UPDATE)|^(DELETE)/";
if(preg_match($pattern, $Query, $matches) === 1):
$ready = $this->conn->prepare($Query);
if( ! empty( $value ) ){
$i = 1;
foreach($value as $key => $keyValue):
if( ( is_string( $key ) ) && ( $key == $this::PARAM_LOB ) ):
$param = PDO::PARAM_LOB;
elseif( is_string( $keyValue ) ):
$param = PDO::PARAM_STR;
elseif( is_int( $keyValue ) ):
$param = PDO::PARAM_INT;
elseif( is_bool( $keyValue ) ):
$param = PDO::PARAM_BOOL;
elseif( is_null( $keyValue ) ):
$param = PDO::PARAM_NULL;
else:
$param = PDO::PARAM_STR;
endif;
$ready->bindValue($i++, $keyValue, $param);
endforeach;
}
$ready->execute();
if( ( $matches[0] === "SELECT" ) && ( $req === $this::FETCHALL ) ):
$value = $ready->fetchall( PDO::FETCH_ASSOC );
elseif( ( $matches[0] === "SELECT" ) && ( $req === $this::FETCH ) ):
$value = $r

Solution

Due to the fact that a lot of the comments should have been answers - I will attempt to say what they are probably thinking.

A jack-of-all-trade function is poor programming. It defeats the SOLID principles and doesn't enable you flexibility.

I will give you a scenario as to why this would fail:

What happens if the error occurs, you get the error message but what is your sql? What happens if this class is being used as a cron - there is no outputs in cron - how are you going to fetch this error? Right there you will need to change your class.

Your constructor (despite logical) is too "busy": 5 variables? If it cross 3 (not a rule of thumb) you should pass an array instead (take a page out of the php.net man itself). You will need to pass these variables to initialize the class and coding and pass numerous variables to do so is tedious. Also your configuration file (typically an array) would be where you fetch these settings from.

WHAT IF we use SQL's ON DUPLICATE KEY query. Its a valid query - but when on duplicate - you will not get a returned value so the rowCount return on insert will give you nil.

Bottom line - nothing is perfect ... don't make things a swiss army knife. Even frameworks evolves.

Context

StackExchange Code Review Q#43780, answer score: 3

Revisions (0)

No revisions yet.