HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Database method to query

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
databasequerymethod

Problem

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:

  • How could i improve this code a bit



  • 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"); }
$lim

Solution

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" , ...


instead of

"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 :)

Code Snippets

foreach ($vals as $v) { if (empty($$v)) { $$v = ''; } }
array("select " => "something" , ...
"select something" ...

Context

StackExchange Code Review Q#200, answer score: 6

Revisions (0)

No revisions yet.