Object-oriented PDO wrapper

Question

So, I've recently started learning OOP in PHP and learned $this keyword. I know it's a reference to the current object. So I wrote a class and couple of functions in it.

<?php
Class Connection{
var $databaseHost;
var $databaseUsername;
var $databasePassword;
var $databaseName;
var $query,$result,$conn,$rows;
function setVariables($databaseHost,$databaseUsername,$databasePassword,$databaseName){
 $this->databaseHost=$databaseHost;
 $this->databaseUsername=$databaseUsername;
 $this->databasePassword=$databasePassword;
 $this->databaseName=$databaseName;
}
function playingWithDatabase(){
 $this->conn = new PDO("mysql:host=$this->databaseHost;dbname=$this->databaseName;", $this->databaseUsername, $this->databasePassword);
 $this->conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 //Connect
 try{
 $this->result = $this->conn->prepare("SELECT * FROM student_data");
 $this->result->execute();
 $this->rows = $this->result->fetch(PDO::FETCH_NUM);
 //Dirty Debug
 echo "<b> Displaying First Record </b> <br> <br>";
 echo $this->rows[1];
 echo "<br/>";
 echo $this->rows[2]; 
 echo "<br/>";
 echo $this->rows[4]; 
 echo "<br/>";
 echo $this->rows[6]; 
 echo "<br/>";
 }
 catch (Exception $e){
 echo "Failure";
 }
 return $this->rows;
}

So, as you can see I took a C# approach to get and set except I'm trying to set in another function and trying to connect to the database in that function.

I feel I'm doing a redundant job here.

Shall I reduce the usage $this and if yes, How so?
How will a professional PHP developer will do it?

Answer 1

Well, PDO wrapper is not a good object to exercise on, because

• PDO is a wrapper already, and by writing your own wrapper you'd rather spoil most of PDO's excellent features
• there is a very specific issue about database wrappers that makes them distinct from other objects - a database connection should be made only once per script execution, so you cannot instantiate it anywhere you need, but rather should pass around a single instance.

Having said that, there is a room for the improvement, either in database wrappers, OOP and general programming techniques.

Database wrapper related

Some mistakes are so common that I even wrote an article, Your first database wrapper's common mistakes. Some highlights from it:

• error reporting doesn't actually exist in your wrapper
• connection is made every time a function is called, slowing your app significantly and endangering your server to fail under too many connections
• statefulness. this one is related to your question about $this. It's ok to keep the sate related to the connection, but not for the query result. So, all your class variables beside $conn just shouldn't be

OOP related

• Database credentials as class variables. You aren't going to use $databaseHost more than once. So it makes no sense to make it a class variable. Create a PDO instance in the constructor, and then assign it to a class variable $conn. That's the only variable that would make sense to use.
• function setVariables() is rather pointless. We use constructors for setting class variables. Such a function would make sense only if your class is static and therefore never gets instantiated explicitly
• so you can tell that your class lacks a constructor
• your class is too diverse. It creates a connection, runs a specific query, and even formats the output as HTML. That's too much. Each class should be responsible for just a single job
• moreover, everything said above is stuffed in a single function

General programming issues

Error reporting

Again, I've got an article that explains the basics, PHP error reporting. Some highlights:

• NEVER EVER do anything like echo "Failure"; on error. It is both useless and harmful. If you don't know what to do with an error you get, then just let it go
• neither you should wrap a code block in a try..catch if you don't know what to do with an exception. Just let it go
• you could use a try..catch to wrap your entire code, and then write some useful handling in the catch block. the absolute minimum is to log the error message for the programmer and to show some nice excuse for a site user

Displaying results

It should be a different story, irrelevant to all database interactions. Gather all the required data first, collect it in a convenient array / structure, and then proceed to displaying your HTML page. Given there are situations when you don't have to show an HTML page but just send JSON-encoded data, or there is no data but just an HTTP header at all, it is natural that all output starts only after all database interactions are ended.

PDO wrapper

Given you called your exercise a PDO wrapper, it would be logical to show you a better code for this kind of a class. Well, you can tell I've got an article on that as well :)

Given the explanations above, your PDO wrapper should only let you to hold a database connection and to run a query. Everything else should be done by another class / procedure, even getting the query result.

PDO is made very wisely: it actually consists of two classes, PDO proper and PDOstatement, which is used to hold all the stuff specific to the particular query. Although you could wrap the latter too, let's use it as is, as PDOstatement is a very good class by itself. So, just like vanilla PDO, our wrapper would return an instance of PDOStatement that could be then used to retrieve the selected data.

class MyPDO extends PDO
{
 public function __construct($dsn, $username = NULL, $password = NULL, $options = [])
 {
 $default_options = [
 PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
 PDO::ATTR_EMULATE_PREPARES => false,
 PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
 ];
 $options = array_replace($default_options, $options);
 parent::__construct($dsn, $username, $password, $options);
 }
 public function run($sql, $args = NULL)
 {
 if (!$args)
 {
 return $this->query($sql);
 }
 $stmt = $this->prepare($sql);
 $stmt->execute($args);
 return $stmt;
 }
}

As of the running particular queries, it should be done in a completely distinct class / routine. Let's create a Student class that uses our PDO instance as a service:

class Student
{
 /* @var MyPDO */
 protected $db;
 protected $data;
 public function __construct(MyPDO $db)
 {
 $this->db = $db;
 }
 public function findAll()
 { 
 return $this->db->run("SELECT * FROM student_data")->fetchAll();
 }
}

which uses function findAll() to return all the students' data. Whereas output should be a completely different story:

$mypdo = new MyPDO('mysql:host=localhost;dbname=test;charset=utf8');
$student = new Student($mypdo);
$list = $student->findAll();
echo "<b> Displaying All Records </b> <br> <br>";
foreach ($list as $row) {
 echo $row['name'];
 echo "<br/>";
 echo $row['email']; 
 echo "<br/>";
}

• Yash: you're a genius...It'll take some time to understand all of this but it solved my queries...thank you!
• Yash: Hey, I went through your answer again and I cannot help understand Each class should be responsible for just a single job Can you please elaborate?
• Your Common Sense: That's a good question. I think this link would be a good start: en.wikipedia.org/wiki/Single_responsibility_principle