trjhtr

I've made a simple querybuilder for my job, but I'm kind of insecure about starting to use it because I'm afraid of errors happening when I start using it. I've done some tests with where, join, etc... So far I didn't find anything, but if someone could take a look and see if there's something missing, I would appreciate it a lot.

Is this class reliable enough?

P.S.: I've only made it work for SELECT queries because it's the only thing I need.

P.P.S.: I know I could use a external lib, but it is worth nothing trying to make one myself. (And libs are kind of overwhelming for what I need)

<?php
class QueryBuilder 
 private $query;
 private $table;
 public function __constructor() 
 $this->query = '';
 $this->table = null;
 
 public function __call($method, $arguments) 
 $index = 1;
 preg_match('/where/i', $method, $isWhere);
 if(!empty($isWhere) && strlen($method) > 6) 
 $method = str_ireplace($isWhere[0], '', $method);
 
 preg_match_all('/[A-Z]/', $method, $temp, PREG_OFFSET_CAPTURE);
 $temp = $temp[0];
 if(!empty($temp)) 
 foreach($temp as $offset) 
 
 else 
 $this->query .= strtoupper($method);
 $this->query .= ' ';
 
 if(count($arguments) == 1) 
 if(!empty($arguments)) 
 foreach($arguments as $arg) 
 foreach($arg as $key => $value) 
 $this->query .= $value;
 if($index < count($arg))
 $this->query .= ', ';
 else
 $this->query .= ' ';
 
 
 
 
 else 
 if(!empty($arguments)) 
 $this->query .= $arguments[0];
 $this->query .= ' ON ';
 unset($arguments[0]);
 foreach($arguments as $arg) 
 foreach($arg as $key => $value) 
 $this->query .= $value;
 if($index < count($arg))
 $this->query .= ', ';
 else
 $this->query .= ' ';
 
 
 
 
 return $this;
 
 public function table($tables = ) 
 public function select($cols = null) 
 private function makeQuery() 
 return $this->query;
 
 public function get() 
 return $this->makeQuery();
 

$test = new QueryBuilder();
echo $test
 ->table()
 ->select(['id', 'name'])
 ->Where(['name= 'john''])
 ->whereNotIn(['id > 1'])
 ->get();

Bug with WHERE conditions

I tried running the sample code:

$test = new QueryBuilder();
echo $test
 ->table()
 ->select(['id', 'name'])
 ->Where(['name= 'john''])
 ->whereNotIn(['id > 1'])
 ->get();

It threw an exception because the argument to the table method was null (which would yield an empty array per the default argument value), so I changed it to pass an array with a single string literal ['users'] to ->table(). Then when I ran it again, I saw the string literal returned below:

SELECT id, name FROM users WHERE name= 'john' NOTIN NOT IN id > 1 

Correct me if this is wrong but most SQL engines need to have the predicates combined with the AND and OR keywords, and those where conditions NOTIN NOT IN would definitely yield an error. It is unclear how the NOT IN should be combined with the id > 1... My best guess is that a sub-query would be needed for that to work... something like

SELECT id, name FROM users WHERE name= 'john' AND id NOT IN (SELECT id FROM users WHERE id > 1)

Given that issue, I would say to your question Is this class reliable enough? No it isn't reliable, but maybe if that issue is resolved then it would be.

Constructor is useless

The only affects of the constructor are to set the two properties (instance variables) to primitive values (i.e. an empty string literal and null). Those could be initialized when declared since those values can be evaluated at compile time. Thus the constructor can be removed once those initializations are added to the declarations:

class Builder {
 private $query = '';
 private $table = null;

One advantage here would be that if this class had a parent class, then any method that overrides the same method in the parent class would need to have the same signature or at least pass sufficient parameters when calling the parent method is needed.

Variables declared even if not used

While the next section describes how to eliminate variables like $index, I do notice that variable is often declared as a local varible assigned the value 1 at the start of methods (like __call(), table(), select()). However in some cases the method may return early - for example in table() an exception is thrown if the $tables argument is null or empty. While it is only an integer, it is wise to not assign values to variables until they are needed. Imagine a large object was assigned to a variable there after calling a function (or multiple functions) - if the method returned early, then the CPU cycles used to get the variable from the function would then be wasted.

Use implode() instead of conditionally appending seperators

I see a few places like the block from select() below, where array elements are appended to the query property and then commas are added if end of the list hasn't been reached:

foreach($cols as $value) 
 $this->query .= $value;
 if($index < count($cols)) 
 $this->query .= ', ';
 
 $index++;

That can be simplified using implode() with the comma and space used as the $glue and the array $cols as the $pieces parameters.

$this->query .= implode(', ', $cols);

And this makes $index superfluous so it can be removed.