Efficiently accessing the database

I followed this tutorial to create an SQL "factory".

The link shows how to make a class who's methods will output SQL statements based on the arguments you pass to it, and I've expanded on it with the class below.

Basically, as an example of its usage: if you wanted to select from a database you can pass it an array of attributes to select, followed by the table name, and if there are conditions to the select, you pass it an array, where the array key is the column the condition is based on, and the array value is the value of that column - so a simple select would look something like this:

$theAttributes = array('name','age','height')
$theTable = 'people';
$theConditions = array('hair_colour' => 'brown');
$select = $this->executeSelect($theAttributes,$theTable,$theConditions);

and then $insert[0] would be the number of rows that were selected by the query, and insert[1] would be an array containing each row that was returned as an array.

I'm just wondering if I could get some feedback on it as it seems to work fine for me, but I'm very new to PHP, so some advice such as inefficiency, insecurity, or if it's just plain wrong.

```
require './config/config.inc.php';
require './config/n_spaces.inc.php';
require_once(__DIR__.'/SqlStatement.php');
require_once(__DIR__.'/MySqlStatement.php');

class PDOdriver {

public $conn;

public function __construct()
{
$db_host = DB_HOST;
$dbname = DB_NAME;
$this->conn = new PDO("mysql:host=$db_host;dbname=$dbname",DB_USER,DB_PASS)
or die('Cannot connect to the server, please inform your Network Administrator');
$this->st = new MySqlStatement();
}

public function executeInsert($theAttributes,$theTable,$theConditions = NULL) {

foreach($theAttributes as $index => $value) {
$bind_params[] = ":$index";
}

if(!isset($theConditions)){
$sqlstmt = trim($this->st->setTables($theTable)->setAttributes(array_keys($theAttributes))->setValues($bind_params)->makeInsert());
}

Long wall of text incoming. Hope it helps :)

First a comment on your file extensions. PHP doesn't care what a file's extension is, it could be an HTML file, text file, or anything. It's just convention that makes us use ".php". Eventually it was determined that adding the ".inc" extension would help distinguish that a file was to be included somehow. We don't have to use this convention, but usually when we do we replace ".php" with ".inc". Seeing them together is just a little odd.

Unless it can't be helped, you should avoid using the require/include_once() functions. These are a bit slower than a regular require/include(). It won't make much difference in such a small application, but if you start going crazy with includes later and are using the _once() versions heavily, you will notice a definite drop in performance.

In PHP there are special kinds of "functions" called language constructs. These functions are native C Language functions that PHP offers directly, without needing to do anything special with them. Theses constructs are generally indistinguishable from traditional functions. The only difference between them is that they are faster and some allow custom syntax that is different from normal function behavior. Your require() functions are so called constructs. So are include()s, echo()s and quite a few others. The point here is that these constructs, as I previously mentioned, have custom syntax. They can be used the same way that you would use a traditional function, or they can use this custom syntax. This custom syntax allows you to neglect adding parenthesis around the construct's parameters. This is a commonly accepted practice. The reason I bring this up is that you are switching back and forth between the two methods. Generally speaking, you should choose a "style" and be consistent with it.

require './config/config.inc.php';
require './config/n_spaces.inc.php';
require_once __DIR__.'/SqlStatement.php';
require_once __DIR__.'/MySqlStatement.php';
//OR
require('./config/config.inc.php');
require('./config/n_spaces.inc.php');
require_once(__DIR__.'/SqlStatement.php');
require_once(__DIR__.'/MySqlStatement.php');

The or die() short-circuiting syntax is usually frowned upon. The "traditional" way to do it is to wrap it in an if statement and handle the die() portion a little differently. You could have it redirect, throw an error, log an error, try another solution, connect to a default, any combination, or anything else really. But usually die() is thought of as being a very "inelegant" way to approach this. The problem is that many new developers saw this and said, "Oh boy! A quick and easy way!" and immediately adopted it. This is fine for development environments, but in a real life applications customers aren't going to want to see a white page with little or no writing on it.

$this->conn = new PDO("mysql:host=$db_host;dbname=$dbname",DB_USER,DB_PASS);
if( ! $this->conn ) {
    $this->logErr('Cannot connect to the server, please inform your Network Administrator');
}

Adding "the" to a variable seems odd and unnecessary. Those variables would mean the same thing if typed more simply as $attributes, $table, $conditions. Words like "the" are implied and usually a programmer will add them, or their own favorite placeholder, in the proper place when reading it in their head. This is the hallmark of a beginner programmer who is still trying to make sense of the language by making it read more like plain english. Many of us started off doing this :)

If you need to iterate over an array to read just the keys, then you can use array_keys() and loop over that instead. There is no need to define a variable you aren't going to use. The same goes for iterating over an array to read just the values. So you don't always have to define the keys, or indices, when iterating over an array with a foreach loop.

$attrKeys = array_keys( $theAttributes );
foreach( $attrKeys AS $index ) {

As you progress in learning the PHP language, you will come across some principles or common practices. One of these principles is called the "Don't Repeat Yourself" or DRY Principle. This states that you should try to make your code as reusable as possible without redundancies. So when you have a variable you need to define that is mostly the same, and only changes a little based on an if/else statement, or some other requirement, you can set it up to be less redundant. For instance:

$sqlstmt = $this->st->setTables($theTable)->setAttributes(array_keys($theAttributes))->setValues($bind_params);
if(isset($theConditions)){
    $sqlstmt = $this->st->setConditions($theConditions);
}
$sqlstmt = trim( $sqlstmt . $this->st->makeInsert() );

Of course, there is another common practice to do with the above code. It's probably a principle, but I don't know it by name if it is. The closest thing I can think of is the "Single Responsibility" Principle. Anyways, the specific

StackExchange Code Review Q#14213, answer score: 4