PHP MySQL connection class

I'm trying to build a class that connects to the database. It does the job but it is not very elegant. I also have no understanding of singleton class or dependency injection.

Is this safe? Can you explain how I can make this more elegant in simple language?

Also, I use the $logs array instead of commenting. I got some remarks on it, while it makes debugging easy for me (I call print_r() on the logs of certain classes during development). What do you guys think?

logs[] = "Checking if the database is already open.";
            if($this->db_connection != false){
                $this->logs[] = "The database connection is already open.";
            }
            else{
                $this->logs[] = "The database connection is not already open.";
                try{
                    $this->db_connection = new PDO('mysql:host='. DB_HOST .';dbname=' . DB_NAME .';charset=utf8', DB_USER, DB_PASS);
                    $this->logs[] = "A new database connection has been established.";
                }
                catch (PDOException $e){
                    $this->logs[] = "A new database connection could not be established.";
                }
            }
    }
  }
?>

I use the class like this:

if($db_connection){
 $query_search = $db_connection->prepare('SELECT * FROM ...');
 $query_search->bindValue(':sqlvar', $phpvar, PDO::PARAM_STR);
 $query_search->execute();
 $query_result = $query_search->fetch();
}
else{
 // Failed connection
}

From the things I see in your code it looks to me like you are quite new to Object Oriented Programming (OOP), so I will try to keep that in mind during this review.

Basic OOP

The __construct() method of a class is called only once per object of that class, when it is instantiated like so: $db = new Database(). Your class contains a field $db_connection, which default to false. Since the __construct() method is called only when the object is being instantiated, there is no reason to check if ($this->db_connection != false), since $db_connection will always be false for that object.

Static vs non-static

It seems to me like you are misunderstanding how regular class fields and static class fields work. Lets do a quick example based on your code:

$database1 = new Database();
$database2 = new Database();

I now have two instantiated Database objects. Lets assume that when I created $database1, the connection succeeded, so a PDO object will be assigned to $db_connection:

// Will print some stuff about the PDO object
print_r($database1->db_connection);

However, when creating $database2, the $db_connection field will be set to false for $database2, because it is in no way related to the $db_connection field of any other instantiated Database object.

If you were to make $db_connection a static field, then they would be shared between all class instances, because it would be a field on the actual class itself, and not on the instantiated objects.

Using fields

What else is weird is that you assign the global variables DB_HOST, DB_NAME, DB_PASS and DB_USER to some private fields of the class, and then in the constructor you do not use the private fields, but you use the global variables again. This is very unusual and should definitely be removed like so:

$this->db_connection = new PDO('mysql:host='. $this->host .';dbname=' . $this->user .';charset=utf8', $this->name, $this->pass);

Now that we have that out of the way, you should not be setting private fields of a class based on some global variable that might be there. You should change it so they have to be passed to the constructor like so:

public function __construct($host, $user, $password, $name) {
    // Use the variables provided as arguments to the constructor here
}

This makes your class less dependent on your specific setup and global variables and allows for easier reuse and testing of your class. If you for whatever reason need to setup a connection to another database, you can reuse the class with different parameters. You can now use it like this:

$database = new Database(DB_HOST, DB_USER, DB_PASS, DB_NAME);

Furthermore, now that these values are passed as arguments to the constructor, you no longer need the fields to store them in, because you only use them in the constructor!

At this stage into the code review, your class would look something like this:

logs[] = "Attempting to connect to the database.";

        try{
            $this->db_connection = new PDO('mysql:host='. $host .';dbname=' . $name .';charset=utf8', $user, $pass);
            $this->logs[] = "A new database connection has been established.";
        }
        catch (PDOException $e) {
                $this->logs[] = "A new database connection could not be established.";
        }
    }
}

One small nitpick, you should get into the habit of not ending PHP files with the ?> closing tag. See this question for an explanation.

Logging with interfaces

One final thing I will just briefly mention: You shouldn't be logging like this. You should be logging to an interface. Consider the interface:

interface Logger {
    public function log($data)
}

Now your Database class could take an optional Logger object in the constructor like so:

public function __construct($host, $user, $pass, $name, Logger $logger = NULL) {
    $this->logger = logger;
}

Now you can replace $this->logs[] = "..."; with $this->logger->log("...");. The beauty of this is that your Database class no longer cares how the data is being logged, only that it is being logged. You could create a ConsoleLogger that logs to the console or a FileLogger, that logs the data to a file. Any of these could be passed to the constructor of a new Database object, as long as they implement the Logger interface.

Edit: The Singleton

Since you verified that you attempted to make a singleton, I will talk about singletons for a bit. By definition, a singleton is a class that can only be instantiated once. What does this mean? This means that you should not be able to do this:

$db1 = new DatabaseSingleton();
$db2 = new DatabaseSingleton();

You have to ensure that only one instance of this class is made. You might get sneaky and attempt to do something like this:

```
// Don't do this!
class DatabaseSingleton() {
private static $instance_count = 0;

public function __construct() {

StackExchange Code Review Q#132842, answer score: 8