# Database method to query

I have a database class that in __construct() initialize a PDO connection and insert the instance into a $db private var. Now i'm working on a method that can be used to query in this way: $db = new db;
$db->query(array( 'select' => 1, 'from' => 'table', 'where' => array('id' => 1, 'name' => 'charlie'), 'limit' => array(1, 5) ));  I did something that works pretty nicely long time ago while PDO was something unknown, but i was wondering: 1. How could i improve this code a bit 2. How can i end it? I mean how to use the PDO then to submit the query? Here's the method query(): # Defining the type if (isset($array['select'])) { $type = 'SELECT';$type_value = (is_int($array['select'])) ? '*' :$array['select']; }
if (isset($array['update'])) {$type = 'UPDATE'; $type_value =$array['update']; }
if (isset($array['delete'])) {$type = 'DELETE FROM'; $type_value =$array['delete']; }
if (isset($array['insert'])) {$type = 'INSERT INTO'; $type_value =$array['insert']; }
if (!isset($type)) { trigger_error("Database, 'type' not selected."); } // Error # From if (isset($array['from']))
{
$from = 'FROM';$from_value = mysql_real_escape_string($array['from']); // table cannot be pdoed } # Where if (isset($array['where']))
{
if (!is_array($array['where'])) { trigger_error("Database, 'where' key must be array."); }$where = 'WHERE'; $where_value =$array['where'];
# Fixing the AND problem
if (count($array['where']) > 1) {$list = $where_value; foreach ($list as $a =>$b) { $w[] = "{$a} = {$b}"; }$and = implode(' AND ', $w);$where_value = $and; } } # Limit if (isset($array['limit']))
{
if (!is_array($array['limit'])) { trigger_error("Database, 'limit' key must be array."); } if (count($array['limit']) != 2) { trigger_error("Database, 'limit' array must be two-keys-long"); }
$limit_first =$array['limit'][0];
$limit_second =$array['limit'][1];
$limit = 'LIMIT';$limit_value = "{$limit_first}, {$limit_second}";
}

# Set
if (isset($array['set'])) { if (!is_array($array['set'])) { trigger_error("Database, 'set' key must be array."); }
$edits =$array['set'];
foreach ($edits as$a => $b) {$e[] = "{$a} = {$b}"; }
$set = 'SET';$set_value = implode(',', $e); }$vals = array('from', 'from_value', 'set', 'set_value', 'where', 'where_value');
foreach ($vals as$v) { if (empty($$v)) {$$v = ''; } }

$sql = " {$type} {$type_value} {$from} {$from_value} {$set} {$set_value} {$where} {$where_value} "; # Here there would be something like mysql_query($sql), but i'd like PDO! PDO get me hornier.


And now? How to bind parameters? Is that possible to work it out?

• Code like this is why I prefer ORM offerings like Doctrine. Jan 25 '11 at 16:24
• Very usefull comment.
– Shoe
Jan 25 '11 at 16:31
• Where's little Bobby Tables when we need him?
– BenV
Jan 25 '11 at 20:51

That query method (and the db class) has a lot of responsibilities:

• Do the PDO stuff, connection handling
• Be a query builder
• be a query executor
• Handle the params and possibly execute the same statement with different params (it would to that too)
• Do all the error checking
• maybe more i don't see

Usually that functionality is handled in 2 to 3 classes, sometimes even more and not in one single function.

And you are doing some very creepy magic to achieve all the work

foreach ($vals as$v) { if (empty($$v)) {$$v = ''; } }


and all that so you can write

array("select " => "something" , ...


"select something" ...


Also you are using mysql_real_escape_string so it seems you don't want to use the pdo escaping (not binding parameters) so why try to use PDO if you limit yourself to mysql anyways ?

So far everything i spotted thinking about it for 5 minutes. Will improve upon feedback / direction from you if it helped at all :)

• I use mysql_real_escape_string only for the table since you cannot use binding parameter for tables. But, wait. You said you would use 2 or 3 classes? Why should you?
– Shoe
Jan 25 '11 at 18:38
• Because packing all that functionality into one class make one darn big and complicated (read: hart do maintain, test, debug, understand, extend) class. In short: The "once class one purpose" principle. Many "good oop" books and advocates suggest that you are able to describe the purpose of a class in one sentence without saying "and" so you get maintainable (etc. pp. can go into detail if you want) class. Jan 25 '11 at 20:31
• You shoudn't use mysql_real_escape_string for table names though, if you use another database that might be exploitable (using a database that uses different commetents than mysql bad things COULD happen. Use a whitelist for the tablename if you want so make it save. Checking that it matches (for example) /[A-Za-z0-9_]/ will be more secure and less (maybe non) exploitable whereas mysql_real_escape_string doesn't to the right thing for this context Jan 25 '11 at 20:34
• How could a different class for query building and query execution be any better? Using 3 classes will make you mad managing parent and inherit issues. Also it seems to me thousands of lines of code for nothings. My db class manage all the db class: one class one purpose. But I'm probably misunderstanding that.
– Shoe
Jan 26 '11 at 7:24
• @Charlie - a: 3 classes is far from an inheritance nightmare. b: In this instance, the classes would not be using inheritance, they simply have separate concerns, c: it's good to divide classes when they approach unmanageable levels of complexity, or have too much responsibility: misko.hevery.com/code-reviewers-guide/flaw-class-does-too-much Feb 20 '11 at 23:23