3
\$\begingroup\$

I'm trying to code a simple and scalable PHP framework for my own use. Could you please offer some critiques or mandatory improvements for these two classes?

<!-- language: PHP -->
<?php
class Xmysqli extends mysqli
{
 public function __construct($dataDB = array())
 {
 global $config, $debug, $err;
 if (empty($dataDB['host']))
 $dataDB['host'] = $config['db']['host'];
 if (empty($dataDB['user']))
 $dataDB['user'] = $config['db']['user'];
 if (empty($dataDB['pass']))
 $dataDB['pass'] = $config['db']['pass'];
 if (empty($dataDB['base']))
 $dataDB['base'] = $config['db']['base'];
 parent::__construct($dataDB['host'],$dataDB['user'],$dataDB['pass'],$dataDB['base']);
 if (mysqli_connect_error())
 {
 $error_message = "mysqli::connect(): Connect Error (' . mysqli_connect_errno() . ') ' . mysqli_connect_error() . ";
 if ($config['core']['debug'])
 $debug['db']['connect'] = $error_message;
 else
 $err->set($error_message);
 }
 if(!$this->set_charset("utf8"))
 $err->set('mysqli::set_charset(): Error while loading utf8 charset');
 }
 public function query($q)
 {
 global $config, $debug, $err;
 $start = microtime(true);
 $res = parent::query($q);
// my_dump($q,false);
 $end = microtime(true);
 if ($config['core']['debug'])
 {
 $time = ($end - $start);
 $debug['db']['counter']++;
 $debug['db']['timer'] += $time;
 $debug['db']['qry'][$debug['db']['counter']]['query'] = $q;
 $debug['db']['qry'][$debug['db']['counter']]['time'] = $time;
 $debug['db']['qry'][$debug['db']['counter']]['rows'] = $this->affected_rows;
 }
 if (!$res)
 {
 $error = $this->error;
 $iid = $_SERVER["REMOTE_ADDR"]."-".$_SERVER["REMOTE_PORT"]."-".$_SERVER["SERVER_ADDR"]."-".substr(md5($error), 0, 8);
 $name = 'SqlLog-'.date('Y_m_d') . '.err';
 $errtxt = "\n$iid\n\n".
 "QUERY: $q\n".
 "Error: $error\n".
 "Date: ".date("r")."\n".
 "path: ".$_SERVER["REQUEST_URI"]."\n".
 "backtrace: ".backtrace().
 "-------------\n";
 if ($fd = fopen('../tmp/logs/' . $name, 'a'))
 {
 fputs($fd,$errtxt);
 fclose($fd);
 }
 else
 saveAccessLog("ERR", "Echec ouverture fichier " .$name);
//die($errtxt);
 $err->set("An error occured please send this code to <a href='mailto:".$config['core']['contact']."'>".$config['core']['contact']."</a>#".$iid);
 }
 return ($res);
 }
 public function prepare($data)
 {
 return $this->real_escape_string($data);
 }
 public function fetch_all($query)
 {
 $res = $this->query($query);
 $all = array();
 if (!is_object($res))
 die('Query ERROR: ');
 while($row = $res->fetch_array(MYSQLI_ASSOC))
 {
 $all[] = $row;
 }
 return $all;
 }
 public function fetch_array($query)
 {
 $res = $this->query($query);
 return $res->fetch_array(MYSQLI_ASSOC);
 }
 public function num_rows($q)
 {
 $res = $this->query($q);
 return $res->num_rows;
 }
}
<!-- language: PHP --> 
abstract class Model
{
protected $link; // (resource) link for mysqli connection
public $insert_id; // 
protected $affected_rows; // 
protected $fields; // (array) list of table fields
protected $primaryKey; // (string) PK name 
protected $tableName; // (string) Table name
protected $listfields; // (array) list of table fields setted by child class
public function __construct()
{
 global $mysqli_link, $err;
 $this->link = $mysqli_link;
 if (empty($this->tableName))
 $err->set('Cannot create new Model: $this->tableName is not defined');
 if (empty($this->primaryKey))
 $err->set('Cannot create new Model: $this->primaryKey is not defined');
 $this->setFields();
}
protected function setFields()
{
 global $err;
 if (!empty($this->listfields))
 $this->fields = $this->listfields;
 else
 {
 $q = "SHOW COLUMNS FROM `".$this->tableName."`";
 $result = $this->link->fetch_all($q);
 foreach($result as $column)
 {
 if($column['Key'] == 'PRI')
 $this->primaryKey = $column['Field'];
 $this->fields[] = $column['Field'];
 }
 }
 if (empty($this->fields))
 $err->set('setFields(): no fields defined for table '.$this->tableName);
}
protected function format_opt($opt)
{
 $clauses = '';
 $nb_clauses = 0;
 if (isset($opt['related']) AND is_array($opt['related']))
 {
 foreach ($opt['related'] AS $clause)
 {
 if (!empty($clause['type']))
 $clauses .= ' '.strtoupper($clause['type']). ' JOIN ';
 else
 $clauses .= ' LEFT JOIN ';
 $clauses .= '`'.$clause['table_to'].'`';
 $clauses .= ' ON `'.$clause['table_to'].'`.`'.$clause['field_to'].'` = `'.$clause['table_from'].'`.`'.$clause['field_from'].'`';
 }
 }
 $nb_clauses = 0;
 if (isset($opt['where']) AND is_array($opt['where']))
 {
 foreach ($opt['where'] AS $clause)
 {
 if ($nb_clauses == 0)
 $clauses .= ' WHERE ';
 else
 $clauses .= ' AND ';
 if (!empty($clause['table']))
 $clauses .= "`".$clause['table']."`.";
 else
 $clauses .= "`".$this->tableName."`.";
 $clauses .= "`".$clause['field']."` ". $clause['op'];
 $clauses .= ($clause['op'] !== 'NOT IN' AND $clause['op'] !== 'IN' AND $clause['value'] !== 'NOW()') ? " '".$this->link->prepare($clause['value'])."'" : " ".$this->link->prepare($clause['value']);
 $nb_clauses++;
 }
 }
 if (isset($opt['groupby']) AND is_array($opt['groupby']))
 {
 $clauses .= ' GROUP BY ';
 if (!empty($opt['groupby']['table']))
 $clauses .= '`'.$opt['groupby']['table'].'`';
 else
 $clauses .= '`'.$this->tableName.'`';
 $clauses .= '.`'.$opt['groupby']['field'].'`';
 }
 $nb_clauses = 0;
 if (isset($opt['orderby']) AND is_array($opt['orderby']))
 {
 foreach ($opt['orderby'] AS $clause)
 {
 if ($nb_clauses == 0)
 $clauses .= ' ORDER BY ';
 else
 $clauses .= ', ';
 if (!empty($clause['table']))
 $clauses .= "`".$clause['table']."`.";
 else
 $clauses .= "`".$this->tableName."`."; 
 $clauses .= "`".$clause['field']."`";
 if (!empty($clause['sort']))
 $clauses .= " ".$clause['sort']; 
 $nb_clauses++;
 }
 }
 if (isset($opt['limit']) AND is_array($opt['limit']))
 {
 $clauses .= ' LIMIT ';
 if (!empty($opt['limit']['start']))
 $clauses .= $opt['limit']['start'].',';
 $clauses .= $opt['limit']['len'];
 }
 return $clauses;
}
public function hydrate()
{
 global $err;
 if (empty($this->{$this->primaryKey}))
 $err->set(get_called_class().': cannot hydrate without primary key value');
 $q = "SELECT * FROM `".$this->tableName."` WHERE `".$this->primaryKey."` = ".intval($this->{$this->primaryKey});
 $data = $this->link->fetch_array($q);
 foreach ($this->fields as $key => $name)
 {
 $setter = 'set_'.$name;
 $this->$setter($data[$name]);
 }
}
public function delete($opt = array())
{ 
 if (empty($opt)) 
 {
 $q = "DELETE FROM `".$this->tableName."`";
 $count = 0;
 foreach ($this->fields as $k =>$name)
 {
 $getter = 'get_'.$name;
 $value = $this->$getter();
 if (!empty($value))
 {
 if ($count == 0)
 $q .= " WHERE ";
 else
 $q .= " AND ";
 $q .= "`".$this->tableName."`.`".$name."` = '".$value."'";
 $count++;
 }
 }
 }
 else 
 $q = "DELETE FROM `".$this->tableName."`" . $this->format_opt($opt);
 $this->link->query($q);
 $this->affected_rows = $this->link->affected_rows;
}
public function save()
{
 $count = 0;
 $nb_fields = count($this->fields);
 if($this->{$this->primaryKey} != null)
 {
 $q = "UPDATE `".$this->tableName."` SET ";
 foreach ($this->fields as $key => $name)
 {
 if($count < ($nb_fields - 1))
 $q .= "`".$name."` = '".$this->link->prepare($this->$name)."',";
 else
 $q .= "`".$name."` = '".$this->link->prepare($this->$name)."'";
 $count++;
 }
 $q .= " WHERE `".$this->primaryKey."` = '".intval($this->{$this->primaryKey})."'";
 $this->link->query($q);
 $this->affected_rows = $this->link->affected_rows;
 }
 else
 {
 $q = "INSERT INTO `".$this->tableName."` (";
 foreach ($this->fields as $key => $name) 
 {
 if($count < ($nb_fields - 1))
 $q .= "`".$name."`,";
 else
 $q .= "`".$name."`)";
$count++;
}
$count = 0;
$q .= " VALUES (";
 foreach ($this->fields as $key => $name) 
 {
 if($count < ($nb_fields - 1))
 $q .= "'".$this->link->prepare($this->$name)."',";
 else
 $q .= "'".$this->link->prepare($this->$name)."')";
$count++;
}
$this->link->query($q);
$this->insert_id = $this->link->insert_id; 
}
}
public function format_select($opt)
{
 $out = '';
 if (!empty($opt['constraint']))
 $out .= ' ' .strtoupper($opt['constraint']);
 if (!empty($opt['fields']))
 {
 $nb_field = 0;
 foreach ($opt['fields'] AS $field)
 {
 if ($nb_field !== 0)
 $out .= ', ';
 if (!empty($field['function']))
 $out .= $field['function'].'(';
 if (!empty($field['table']))
 $out .= ' `'.$field['table'].'`.';
 $out .= $field['field'];
 if (!empty($field['function']))
 $out .= ')';
if (!empty($field['as']))
 $out .= ' AS "'.$field['as'].'"';
$nb_field++;
}
}
return $out;
}
public function getAll($opt = array(), $first_only = FALSE)
{
 if (empty($opt)) 
 {
 $q = "SELECT `".$this->tableName."`.`id` FROM `".$this->tableName."`";
 $count = 0;
 foreach ($this->fields as $k =>$name)
 {
 $getter = 'get_'.$name;
 $value = $this->$getter();
 if (!empty($value))
 {
 if ($count == 0)
 $q .= " WHERE ";
 else
 $q .= " AND ";
 $q .= "`".$this->tableName."`.`".$name."` = '".$value."'";
 $count++;
 }
 }
 if ($first_only === TRUE)
 $q .= " LIMIT 1";
 }
 else 
 if (!empty($opt['select']))
 $q = "SELECT ".$this->format_select($opt['select'])." FROM `".$this->tableName."`" . $this->format_opt($opt);
 else
 $q = "SELECT `".$this->tableName."`.`id` FROM `".$this->tableName."`" . $this->format_opt($opt);
 $ids = $this->link->fetch_all($q);
 $objs = array();
 if (!empty($opt['select']))
 return $ids;
 else
 {
 $child_class_name = get_called_class();
 foreach($ids as $id)
 {
 $obj = new $child_class_name();
 $obj->set_id($id['id']);
 $obj->hydrate();
 if ($first_only === TRUE) 
 return $obj;
 $objs[] = $obj;
 }
 }
 return $objs;
 }
} 
?>
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 18, 2013 at 14:59
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

Sorry, but I would not use this class. Please check the SOLID design.

  1. Instead of passing an array in the xmysqli construct and expecting some array keys, I would just pass the variables directly:

    public function __construct($host,$username,$password,$database)
    

    but then it would be the same as a MySQLi base class. I would instead create a database configuration class and use typehints in construct.

    public function __construct(XmysqliConfig $config)
    

    If someone passes something different into your database class, he/she will see an error.

  2. Please remove all your globals. You have a class which contains properties, so just don't use globals.

  3. Don't use profiling and logging within a function. Create a method to add a profiler class and a method to add a logger.

  4. Your table creation methods or table modifications don't belong to a user model. You instead require that a Schema class doctrine is using a schema manager for this. Your model has just the responsibility of the data inside the table, not the table itself.

This was just an overview of the points, but there are more.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
answered Oct 22, 2013 at 10:49
\$\endgroup\$
1
\$\begingroup\$

My opinion:

@BlackScorp said:

instead of passing an array in xmysqli construct und expect some array keys, i would just pass the variables directly

I'd keep it as is, since personally, I don't really like methods with long argument signatures, where you need to split your code to match the signature, and inside the class, the code is just build together again. However, I would optimize the options handling by merging the given options with default ones, eg.:

$this->config = array_merge($defaultConfig, $givenConfig)

I can more or less +1 the statements from @BlackScorp. Don't use globals, and make profiling and logging optional by moving it somewhere else.

answered Oct 22, 2013 at 11:36
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.