patternphpMinor
PDO connector class
Viewed 0 times
classconnectorpdo
Problem
I have created a DB connector class with PDO.
Do I need to add, delete or edit anything in this code?
```
startConf();
return self::$instance;
}
public function startConf(){
if (empty($this->configState)){
$this->resetState();
}
return $this;
}
/**
* Stores datas in the Config.
* Example: $instance->foo = 'bar';
* @param $key
* @param $val
*/
public function __set( $key , $val ){
$this->configState[$key] = $val;
}
/**
* Gets datas from the Config.
* Example: echo $instance->foo;
*
* @param $key
* @return bool
*/
public function __get( $key ){
return (isset($this->configState[$key]))? $this->configState[$key]: false;
}
public function __isset( $key ){
return isset($this->configState[$key]);
}
public function __unset( $key ){
unset( $this->configState[$key] );
}
public function destroy(){
$this->configState = array();
}
public function resetState(){
$this->configState = array();
$this->configState['show_php_errors'] = true;
$this->configState['SSL_security'] = true;
$this->configState['XFO_security'] = true;
$this->configState['XSS_security'] = true;
$this->configState['CSRF_security'] = true;
$this->configState['db_type'] = "mysql";
$this->configState['db_host'] = "localhost";
$this->configState['db_port'] = 3306;
$this->configState['db_name'] = "demo";
$this->configState['db_user'] = "root";
$this->configState['db_pass'] = "";
$this->configState['db_path'] = "";
}
}
class PDOConnection{
private $instance;
private $dsn;
private $username;
private $password;
private $options = [];
/**
* constructor
*
* @param $dsn
* @param $username
* @param $passwor
Do I need to add, delete or edit anything in this code?
```
startConf();
return self::$instance;
}
public function startConf(){
if (empty($this->configState)){
$this->resetState();
}
return $this;
}
/**
* Stores datas in the Config.
* Example: $instance->foo = 'bar';
* @param $key
* @param $val
*/
public function __set( $key , $val ){
$this->configState[$key] = $val;
}
/**
* Gets datas from the Config.
* Example: echo $instance->foo;
*
* @param $key
* @return bool
*/
public function __get( $key ){
return (isset($this->configState[$key]))? $this->configState[$key]: false;
}
public function __isset( $key ){
return isset($this->configState[$key]);
}
public function __unset( $key ){
unset( $this->configState[$key] );
}
public function destroy(){
$this->configState = array();
}
public function resetState(){
$this->configState = array();
$this->configState['show_php_errors'] = true;
$this->configState['SSL_security'] = true;
$this->configState['XFO_security'] = true;
$this->configState['XSS_security'] = true;
$this->configState['CSRF_security'] = true;
$this->configState['db_type'] = "mysql";
$this->configState['db_host'] = "localhost";
$this->configState['db_port'] = 3306;
$this->configState['db_name'] = "demo";
$this->configState['db_user'] = "root";
$this->configState['db_pass'] = "";
$this->configState['db_path'] = "";
}
}
class PDOConnection{
private $instance;
private $dsn;
private $username;
private $password;
private $options = [];
/**
* constructor
*
* @param $dsn
* @param $username
* @param $passwor
Solution
PDOConnection doesn't seem to serve any purpose. The only thing it seems to do is hold an instance of PDO, so why not use PDO itself?database also only seems to be a wrapper of PDO:queryis simply a renamedprepare
binda renamedbindValue
executeisexecute
FetchAllisfetchAll, except that it also executes (same withFetchColumn)
beginTransactionisbeginTransaction, butendTransactioniscommitandcancelTransactionisrollBack.
So you have a wrapper that slightly renames the functions of PDO, adds very minor changes (
FetchOne can be fetchObject or fetch), and limits its power (lots of functions are missing). Additionally, your comments are not nearly as in-depth as the official PDO documentation. The PHP documentation is often not great, but still, for eg fetchAll, I can read what parameters it accepts, what it returns, etc. Yours only says
fetching all result. So now I have to look into your actual code (because I can't trust that it does exactly what fetchAll does).This may easily create confusion. At a minimum, I would stick to the original names (doesn't really matter if yours are better or worse, PDO is quite well established, so most people will be familiar with it).
But really, I'm not sure that it is worth it to have a class like this. Everyone who is newly added to your project will have to get familiar with your not-quite-PDO class. If it's just this one class, that may be acceptable, but if you handle similar situations the same way, and always create thin wrappers that slightly change how the details of something work, introducing new developers to your project will be difficult because they will have to put a lot of work into getting to know your code base, and they may introduce bugs (because they assume that your wrappers will work like the original thing, while you slightly change the behaviour).
Misc
- be more consistent with your naming. Classes should start with an upper-case character, methods with a lower-case character.
I need to be sure that this classes are secure for injections: This really depends on how you use the class. You still provide the possibility to bind parameters, so just make sure that you always make use of that. Never pass any string that contains variable content (user input, values from the database, ...) toquery, and you will be fine.
Context
StackExchange Code Review Q#121982, answer score: 3
Revisions (0)
No revisions yet.