patternphpMinor
Evaluation of insert method from within a class using PDO
Viewed 0 times
evaluationinsertmethodwithinusingfromclasspdo
Problem
I am learning PDO. I have a class - User...
When I run this...
It works fine. I know it's a bit odd posting when things are working, but I have suspicions that this is not the best approach. I am still learning PDO and about the convert my MySQLi site over to PDO. I want to make sure the PDO veterans think this is a reasonable approach.
class User {
protected static $table_name="users";
public $id;
public $first_name;
public $last_name;
public function pdo_create() {
global $handler;
$sql = " INSERT INTO users (first_name, last_name)";
$sql .= " VALUES (?, ?)";
$query = $handler->prepare($sql);;
$query->execute(array($this->first_name ,$this->last_name));
}
}When I run this...
$user= new User();
$user->first_name = "test";
$user->last_name = "test";
$user->pdo_create();It works fine. I know it's a bit odd posting when things are working, but I have suspicions that this is not the best approach. I am still learning PDO and about the convert my MySQLi site over to PDO. I want to make sure the PDO veterans think this is a reasonable approach.
Solution
Your design is flawed in the following way:
DO NOT USE GLOBAL.
It is not OO programming and you shouldn't be calling it if you are using PDO - pass your PDO through your function or as part of your user constructors. (see SOLID design patterns)
your function pdo_create
is too couple with pdo - if you decide to switch off from pdo - you will need to re-write that function call to reflect what you change to - use convention a bit more general:
Now as for the function itself:
+1 for using prepare statement however ? is more used for mysqli - it better to use place handler instead. such as VALUES(:first_name, :last_name). When you pass the array you can then use the place handler to effectively assign to which variable its value. It is best for readability later.
Finally: you defeat the purpose of the class if your attributes are PUBLIC. Ideally functions receives parameters by the function itself OR by the constructor of the class which initializes the attributes of the class.
DO NOT USE GLOBAL.
It is not OO programming and you shouldn't be calling it if you are using PDO - pass your PDO through your function or as part of your user constructors. (see SOLID design patterns)
class User
{
//private attributes
private $_db;
public function __construct(PDO $pdo)
{
$this->_db = $pdo;
}
}your function pdo_create
is too couple with pdo - if you decide to switch off from pdo - you will need to re-write that function call to reflect what you change to - use convention a bit more general:
creatingUser(PDO $pdo) for example or even better creatingUser(DBInterface $db) (as part of SOLIDNow as for the function itself:
+1 for using prepare statement however ? is more used for mysqli - it better to use place handler instead. such as VALUES(:first_name, :last_name). When you pass the array you can then use the place handler to effectively assign to which variable its value. It is best for readability later.
Finally: you defeat the purpose of the class if your attributes are PUBLIC. Ideally functions receives parameters by the function itself OR by the constructor of the class which initializes the attributes of the class.
Code Snippets
class User
{
//private attributes
private $_db;
public function __construct(PDO $pdo)
{
$this->_db = $pdo;
}
}Context
StackExchange Code Review Q#48045, answer score: 5
Revisions (0)
No revisions yet.