I’ve learned much from the recent PDO v MySQL thread. I, too, find that converting my classes to PDO from mySql (NOT mySqli) seems overly verbose for little benefit.
Then again, most of the discussion in that thread is way over my head.
So let me ask if my approach is OOP / PDO appropriate.
My root class, lessons_template_db, determines if I’m local
if($currentHost == "127.0.0.1") { // no place like home
or on my host’s site and instantiates the connection [$this->db]. (I also check “if isset($this->db)” to avoid re-instantiation – but it never is … hm-m-m-m).
2) All other classes that EXTEND are unique to a MySQL table. They, of course, handle the CRUD, returning an associative array for the Retrievals or a status / db-key for the CREATE, UPDATE, DELETE components.
If this is the proper approach, setting up the named procedures seems overly cumbersome given that I lose the db objects every time I send a form to the user.
Can you provide an example of your code that you are finding more verbose than you’d like?
Please provide a code example, and its execution path, so we can help you out on this.
I’d like to see your code example for this too, as I think I know what you are saying, but I can’t be certain.
Couple of things,
[list=1][]In my opinion PDO is broken (on some level; but this is mainly because I’m comparing it to other languages that have a more versatile solution), but it is a good step in the right direction, the problem is, is it is trying to solve a problem allowing you to change your data source easily, but is under the assumption that the syntax for each data source is the same (which is a flawed assumption). But don’t focus solely on that, PDO does have optimizations that you can benefit from and security enhancements (granted both of these are available to MySQLi as well).
[]DTO (Data Transfer Objects) are you friend when you are abstracting your presentation from your data access. By having a very simple Product class, that contains, id, name, price, stock, etc. and no business/data access logic, you have an object you can pass to your CRUD repository for Products and and object you can pass back after a SELECT is performed in your Product repository.
[*]Your repository classes (the ones that handle your CRUD) should receive an object that holds your data access connections in their constructor or as a method parameter so you can pass your single connection on to each repository (thus reusing the same connection throughout your whole page execution), then at the end of your page execution you should close the connection.
[/list]
Errr PDO is not a database abstraction, the only thing it abstracts is the data-access. Issuing queries and fetching data that is all PDO abstracts, by design. Its not meant for one to swap from one database to another.
Okay, you are right on that, and I miss-stated that (my bad). I still think it doesn’t achieve as much as I would want it to from an abstraction stand point and that there are not many benefits to over the native MySQLi (if using MySQL), when using SQL Server there are some benefits as there isn’t an OOP implementation for SQL Server (that I know of, other than PDO which is experimental, same with Oracle, etc.).
The above class is decent, one suggestion I am sure many will give you is to not extend this class but to pass it into your repository classes (lessons_student_db) so you are taking on a dependency injection approach. This will also solve the fact that your $db variable is null on subsequent calls. Granted the other reason for that is you are initializing a new lessons_student_db() object for BuildContent and for Apply, so it can’t reuse what connection may have already been established.
Also by extending your data access, you make it a bit more difficult to close your connections and will end up resulting in allowing MySQL to determine when to close them.
Couple of things,
[list=1][]You have a typo in your INSERT, you have “leader:” instead of “:leader”
[]Two, refactor, refactor, refactor. Your update logic can be rewritten to
You have a typo in your INSERT, you have “leader:” instead of “:leader”
Two, refactor, refactor, refactor.
Wow. Didn’t see that refactor option at all. Makes sense. Thanks for the insight. (and for the typo – actually is was a copy-paste to this post)
not extend this class but to pass it into your repository classes (lessons_student_db) so you are taking on a dependency injection approach
Not sure I understand. Does this mean to instantiate lessons_template_db in the CRUD classes – then reference the $db with the $instantiated-object-prefix-> instead of $this-> Or is it in the form/parse classes that I instantiate the lessons_template_db and then pass the $db as an argument when instantiating the relevant CRUD db?
Would you be up to sharing an example excerpt for Class lessons_student_db and/or Class lessons_student_detail_ParseForm?
Really appreciate your reply – I suspected the code smelled, just didn’t know how big a stink.
Sure, here is the idea, so you would have the following two classes
<?php
Class lessons_template_db {
Protected $db = null;
Public Function __Construct() { // assure connection
$currentHost = pathinfo($_SERVER['REMOTE_ADDR'],PATHINFO_BASENAME);
if($currentHost == "127.0.0.1"){ // no place like home
$theUsername = "root";
$thePassword = "";
} else {
--> detail omitted <--
} // end if-else
try {
$this->db = new PDO('mysql:host=localhost;dbname=lessons;charset=UTF-8', $theUsername, $thePassword);
$this->db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$this->db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
} catch(PDOException $ex) {
echo("<p style='font-weight: bold; color: red; font-size: larger;'><br /><br /><br />A serious error occurred, contact programmer who will need the following detail<br /></p>");
echo("<br />lessons_template_db->Construct<br />");
var_dump($ex);
exit();
} // end try-catch
} // end function
public function Close()
{
// close and null out the $this->db variable
}
} // end class
(please note, I didn’t perform the updates I suggested in my prior thread; so the type and lack of refactoring still exist in this example)
<?php
Class lessons_student_db // notice I'm not extending anything
{
private $dbManager;
public fnction __construct($db)
{
$this->dbManager = $db;
}
Public Function GetStudentList() {
// dispensing with try-catch
$stmt = $this->dbManager->db->prepare("SELECT name FROM studentlist ORDER BY name");
$stmt->execute();
$nameList = array();
while($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
$nameList[] = $row['name'];
} // end while
return($nameList);
} // end function
Public Function GetStudentDayList($choice) {
// dispensing with try-catch
$stmt = $this->dbManager->db->prepare("SELECT name FROM studentlist WHERE lessonday = :lessonday ORDER BY name");
$stmt->bindValue(':lessonday', $choice, PDO::PARAM_STR);
$stmt->execute();
$nameList = array();
while($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
$nameList[] = $row['name'];
} // end while
return($nameList);
} // end function
Public Function GetStudentDetail($name) {
$namelist = array();
$stmt = $this->dbManager->db->prepare("SELECT * FROM studentlist WHERE name = :name");
$stmt->bindValue(':name', $name, PDO::PARAM_STR);
// dispensing with try-catch
$stmt->execute();
$nameList = array();
while($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
$nameList[] = $row;
} // end while
return($nameList);
}
Public Function UpdateDetail($detailVals){
$stmtArray = array();
$args = "";
if(isset($detailVals['newname']) && strlen($detailVals['newname']) > 0) {
$name = mysql_real_escape_string($detailVals['newname']);
$args .= "name=:name";
$stmtArray[':name'] = $name;
}
if(isset($detailVals['lessonday']) && strlen($detailVals['lessonday']) > 0) {
$lessonday = mysql_real_escape_string($detailVals['lessonday']);
if(strlen($args)>0) {
$args .= ", "; // add separator if args started
}
$args .= "lessonday=:lessonday";
$stmtArray[':lessonday'] = $lessonday;
}
if(isset($detailVals['independent']) && strlen($detailVals['independent']) > 0) {
$independent = mysql_real_escape_string($detailVals['independent']);
if(strlen($args)>0) {
$args .= ", "; // add separator if args started
}
$args .= "independent=:independent";
$stmtArray[':independent'] = $independent;
}
if(isset($detailVals['leader']) && strlen($detailVals['leader']) > 0) {
$leader = mysql_real_escape_string($detailVals['leader']);
if(strlen($args)>0) {
$args .= ", "; // add separator if args started
}
$args .= "leader=:leader";
$stmtArray[':leader'] = $leader;
}
if(isset($detailVals['sidewalker']) && is_numeric($detailVals['sidewalker']) > 0) {
$sidewalker = $detailVals['sidewalker'];
if(strlen($args)>0) {
$args .= ", "; // add separator if args started
}
$args .= "sidewalker=:sidewalker";
$stmtArray[':sidewalker'] = $sidewalker;
}
if(isset($detailVals['note']) && strlen($detailVals['note']) > 0) {
$note = $detailVals['note'];
if(strlen($args)>0) {
$args .= ", "; // add separator if args started
}
$args .= "note=:note";
$stmtArray[':note'] = $note;
}
$sql = "UPDATE studentlist SET " . $args . " WHERE idStudent = " . $detailVals['idStudent'];
try {
$stmt = $this->dbManager->db->prepare($sql);
$stmt->execute($stmtArray);
} catch(PDOException $ex) {
$msg = $this->DbReject($sql, $detailVals, $ex);
return $msg;
} // end try-catch
return("success");
} // end function UpdateDetail
Public Function AddDetail($detailVals){
$sql = "INSERT INTO studentlist(name,lessonday,independent,leader,sidewalker,note) ";
$sql.= "VALUES(:name,:lessonday,:independent,leader:,:sidewalker,:note)";
$stmt = $this->dbManager->db->prepare($sql);
$stmtArray = array();
$stmtArray[':newname'] = mysql_real_escape_string($detailVals['newname']);
$stmtArray[':lessonday'] = mysql_real_escape_string($detailVals['lessonday']);
$stmtArray[':independent'] = mysql_real_escape_string($detailVals['independent']);
$stmtArray[':leader'] = mysql_real_escape_string($detailVals['leader']);
$stmtArray[':sidewalker'] = $detailVals['sidewalker'];
$stmtArray[':note'] = mysql_real_escape_string($detailVals['note']);
try {
$stmt->execute($stmtArray);
$detailVals['idStudent'] = $this->db->lastInsertid();
} catch(PDOException $ex) {
$msg = $this->DbReject($sql, $detailVals, $ex);
return $msg;
} // end try-catch
return("success");
} // end function UpdateDetail
Public Function DbReject($sql, $detailVals, $ex) {
if ($ex->getCode() == '23000') {
return ("name already on file, please try again");
return false;
}
echo("<p style='font-weight: bold; color: red; font-size: larger;'><br /><br /><br />A serious error occurred, contact programmer who will need the following detail<br /></p>");
echo("<br />lessons_student_db<br />sql: {$sql}");
echo("<br />detailVals array<br />");
var_dump($detailVals);
echo("<br />ex message<br />");
var_dump($ex);
exit();
} // end function DbReject
} // end class
?>
Implementation:
// assuming the two classes are already included
$db = new lessons_template_db ();
$studentRepo = new lessons_student_db($db); // inject the database object into the repository so the connection data is provided for all CRUD operations
$studentRepo->GetStudentList();
$studentRepo->UpdateDetail($details); // assumed $details is provided somewhere
$db->Close();
I’d also rename your database class to be a generic name, DatabaseManager or something similar.
Also when it comes to extending, following the “is-a” definition that @Jeff_Mott ; talks about here is a very good practice.
Let me see if I understand.
The only change to Class lessons_template_db is
public function Close()
{
// close and null out the $this->db variable
}
So is this as simple as UNSET($this->db) or is there a PDO format?
The changes to Class lessons_student_db are
Remove the extends
AND
private $dbManager;
public function __construct($db)
{
$this->dbManager = $db;
}
I gather instantiating the dbManager in the implementer rather than in each CRUD class is to be able to pass a single connection to the different db classes (which is the issue with the EXTENDS I was doing)?
Ends up that may not be necessary, as I don’t see a close() implementation to PDO like their was for MySQL and MySQLi. So PDO must handle it on its own.
And I replaced ALL instances of $this->db with $this->dbManager->db
Yes, because by using EXTENDS you are creating a DB connection for each class that extends the lessons_template_db class, so you will ultimately end up with multiple connections and database instances. By passing in the DB connection, you can re-use the connection/instance across all of your repositories that do your CRUD work.