PDO wrapper structure

Is this PDO wrapper structure dynamic? Can using self:: rather than $this cause problems?

```
class Database
extends PDO
{
/@var mixed $stmt Used as a temporary variable to hold queries/
private $stmt = Null;

/@var $scopeSelectVar Used as a temporary variable to hold queries/
private $scopeSelectVar = Null;

/*@Constructor __construct
*
* Database __construct ( mixed $db, [ string $serverHost = Null], [ string $dbName = Null],
* [ string $dbUser = Null], [ string $password = Null], [ bool $persistent = Null])
*
* @access-public
*/
public function __construct($db,
$serverHost = Null,
$dbName = Null, $dbUser = Null,
$password = Null, $persistent = true)
{
try
{
if(is_array($db))
{
/*
* parent::_construct allows easy instantiation and multiple database connections.
* @param mixed $db When array(), var holds all parameters required to connect.
* When string, var holds the type of DB to connect to. E.g. mysql
*
* @param string $serverHost the host of the DB connection
* @param string $dbName the name of the database
* @param string $dbUser the respective name of the user
* @param string $password the password of the user connecting
* @param bool $persistent if true, allows connection to persist through the app
*
*/
parent::__construct("{$db['dbType']}:host={$db['Host']};
dbname={$db['dbName']}",
$db['dbUser'],
$db['dbPassKey'],
array(
PDO::ATTR_PERSISTENT => $persistent,
));

}//if Var $db == array() then connect using array parameters
else
{
//else connect using all the other function parameters
parent::__construct("

When trying to make things like this, it's amazing how quickly it can become complicated and how many OOP ideas and principles can be involved.

Anyway, there's a lot to cover here, so I'm just going to glaze over a few things and may come back and edit more detail in later.

For a "first heavy OOP code design" this is pretty good (probably better than the typical first real go at it), but there are a few things that stood out:

• self:: is almost always a bad sign

• It means that a variable is basically a namespaced global

• It means that a method has absolutely nothing to do with the state of an object of the class it belongs to. This can make sense in a few situations, but it is a decision that should be carefully considered unless it's one of the obvious uses.

• Your code is wrong on a technical standpoint because you cannot--well, should not, since PHP is overly lenient--call a non-static method statically. It doesn't make sense. $this and self are not interchangable at all.

• Your class is MySQL specific

• Backticks are MySQL specific

• A few other things that I'm too lazy to look back through the code and refind :)

• You're mixing concerns with query creation and database connection.

• prepare, query etc are fairly low level operations

• creating statements with methods like where is a fairly high level operation

• If you want a DBAL, create a DBAL. A DBAL should have a connection, not be a connection.

• People for some reason always want to extend PDO. There are situations where that makes sense, however, 99% of the time, it does not.

• If you want an SQL helper class, then have a class that uses a PDO instance behind the scenes. Don't couple your helper class and your connection class together.

• You're abusing exceptions

• How does a consumer of your class know that a PDOException occurred? You should almost never eat an exception. If your methods fail, the consuming code probably needs to know.

• Your logging is flawed

• If you ever find yourself used a class like C::a(); C::b(); or if you ever find yourself creating a new object in a method, it's likely a sign that the dependency should be provided by the caller.

• Provides flexibility (what if someone wants to have two instances of your class and use two different loggers?)

• Eases coupling (Database is secretly dependent on PDO -- telling consumers to uncomment the code if they have a logger named Log is not sufficient)

• Pass in a logger instead that implements a certain interface

• Allows flexibility like a null logger (a logger that just discards everything) or like having one instance log to a DB table and another to a CSV

• Depending how low level your class ends up being, a logger may not belong there.

• What I mean by this is that there's a certain level where logging just doesn't make sense. System calls don't log. Even PDO doesn't log. You should probably handle logging in application-land where the application can have a better feel for how logging should be handled.

• Your constructor kills flexibility

• SQL Server or Postgres for example, will have much different DSNs than MySQL

• in trying to be overly helpful, your constructor has killed this flexibility -- there's a reason PDO takes a DSN and not the params your constructor takes.

• You address persistence, but what about the other attributes?

• A few stylist notes (note: these are 100% opinion)

• It's fairly standard to indent to the first level inside of classes

• switch: and endswitch; are very rarely used. A lot of people will vehemently argue with me on this one, but I think that the endwhile; endif; and so on constructs are a hideous blotch a on C-esque language.

• Null is almost always written either NULL or null in PHP.

In short, this is a good start, but you're probably headed down the wrong path with this class. There's a lot of technical and design flaws here that need to be addressed. I would suggest learning the OO syntax a bit more and trying to better familiarise yourself with low level philosophies and ideas of object oriented programming (when I have more time later I'll come back and provide some links).

StackExchange Code Review Q#14589, answer score: 7