Implementation of database access class [PHP]

Question

I programmed a class for database access for my REST service. According to my understanding it should be implemented as singleton, because there can be a lot of simultaneous requests. Is there everything okay with this approach?

Is there a way to handle the getData... functions better? They basically do all the same thing, expect for the SQL query and the different parameters for the query.

Is it good to store the connection data (username, password..) in a JSON file?

Note that I've modified some lines from the actual code (SQL-queries and function names), because I think they are not relevant for my questions.

class DBManager {
protected static $con;
protected static $_instance = null;
protected function __construct() {
    $data = self::getConnectionData ();
    $host = $data ["host"];
    $name = $data ["name"];
    self::$con = new PDO ( "pgsql:host=$host;dbname=$name", $data ["user"], $data ["password"] );
}
protected function __clone() {}

public static function getInstance() {
    if (null === self::$_instance) {
        self::$_instance = new self ();
    }
    return self::$_instance;
}
protected function getConnectionData() {
    $raw = file_get_contents ( "conf/database.json" );
    $json = json_decode ( $raw, true );
    return $json;
}
public function getData1($year) {
    $query = "...";
    $stmt = self::$con->prepare ( $query );
    $stmt->bindParam ( 1, $year );
    $stmt->execute ();
    $result = $stmt->fetchAll ( PDO::FETCH_ASSOC );
    return $result;
}
public function getData2($year, $month) {
    $query = "...";
    $stmt = self::$con->prepare ( $query );
    $stmt->bindParam ( 1, $year );
    $stmt->bindParam ( 2, $month);
    $stmt->execute ();
    $result = $stmt->fetchAll ( PDO::FETCH_ASSOC );
    return $result;
}

// more getData functions...
}

Answer 1

The reason for singletons

First, you've got some misunderstandings about what singletons accomplish. PHP automatically keeps all variables separate from request to request, so the fact that you have simultaneous requests makes no difference for the singleton pattern. Rather, the singleton pattern makes sure that you only have one instance of a particular class inside a single request. Your constructor is protected, and so the only way to get a DB connection object is through the static getInstance method, which makes sure there is only ever one instance. The relevancy of this to a DB connection class is that if the system can create more than one instance of your DB connection class per request, and if your DB connection class opens a new connection to the database everytime it is created, then you can potentially get many open connections to MySQL from a single HTTP request. That's a quick way to overload your database's open connection pool and end up with failed requests. It doesn't have anything to do with having multiple requests at the same time.

The reason to not use singletons

That being said, while this is a good stepping stone to a better application, it shouldn't be your final stepping point. Singletons are, in essence, a design pattern that should be considered deprecated. The reason is because they break dependency injection. If you're new to this concept (we all were at some point) then it is absolutely worth a lot of googling and reading. Dependency injection, other than making your code more flexible and easier to maintain, also makes it possible for you to mock and test your code thoroughly (via unit and integration tests). It is a completely different way of building an application, so I wouldn't expect you to just throw everything away and start over. However, it is never to soon to start learning, so you can do yourself a favor and start reading about these things now.

Oftentimes with dependency injection you will have a dependency injection container that handles the details for you, and these containers usually have a way of flagging classes as "singleton" classes, which means that the dependency injection container will never initialize more than one. At that point in time though, the classes don't implement the singleton pattern: they act like normal classes with a normal constructor, and rely on the DI container to make sure there is only one.

Passwords in config files

This is fairly common, and is normally fine. Just make sure that the file with your DB credentials isn't committed to your code repository.

Public functions

The other problem you have is that you are trying to stick your data access method directly into your DB connection class. You don't want to actually do that. For an application of almost any size, you are going to quickly end up with far too many methods, and this class will become impossible to read/update/maintain. It's all about the separation of concerns. Ideally, your DB connection class would just be a simple wrapper around the PDO object, and your models classes (depending on how you organize your application), would then be responsible for using the DB connection class to execute their own queries, as is relevant to them.