PHP: Simple SQL query builder

Question

I've been lucky enough to get my first job as a junior PHP developer. I am concerned I'm not good enough. I would like to brush up on my coding skills and get as much constructive criticism as I can.

This is a SQL query builder, built for MySQL that provides a fluent, chaining API to build a query, and allows you to output the raw SQL. How can I improve the quality of it, as well as any features you can think of that I can try to practice.

API:

$qb = new QueryBuilder('users');
$qb
 ->where('username', 'googun')
 ->orWhere('username', 'janetjackson')
 ->select(['id', 'username', 'email'])
 ->orderBy('id', 'DESC')
 ->limit(5);

QueryBuilder class:

<?php 
class QueryBuilder {
 private $tableName;
 private $whereClauses;
 private $selectColumns;
 private $maxResults;
 private $orderBy;
 public function __construct($tableName) {
 $this->tableName = $tableName;
 $this->whereClauses = [];
 $this->selectColumns = ['*'];
 $this->maxResults = -1;
 }
 public function where($column, $valueOrOperator, $value = null) : QueryBuilder {
 if ($value == null) {
 return $this->appendWhere($column, '=', $valueOrOperator);
 }
 return $this->appendWhere($column, $valueOrOperator, $value);
 }
 private function appendWhere($column, $operator, $value, $join = 'AND') : QueryBuilder {
 $this->whereClauses[] = [
 'column' => $column,
 'operator' => $operator,
 'value' => $value,
 'join' => $join
 ];
 return $this;
 }
 private function whereRaw($rawSql) : QueryBuilder {
 $this->whereClauses[] = [
 'raw_sql' => $rawSql,
 'join' => 'AND'
 ];
 return $this;
 }
 public function orWhere($column, $valueOrOperator, $value = null) : QueryBuilder {
 if ($value == null) {
 return $this->appendWhere($column, '=', $valueOrOperator, 'OR');
 }
 return $this->appendWhere($column, $valueOrOperator, $value);
 }
 public function orWhereRaw($rawSql) : QueryBuilder {
 $this->whereClauses[] = [
 'raw_sql' => $rawSql,
 'join' => 'OR'
 ];
 return $this;
 }
 public function select(array $columns) : QueryBuilder {
 $this->selectColumns = $columns;
 return $this;
 }
 public function limit(int $amount) : QueryBuilder {
 $this->maxResults = $amount;
 return $this;
 }
 public function orderBy(string $column, string $direction) {
 $this->orderBy = $column . ' ' . $direction;
 return $this;
 }
 public function buildDefaultQuery() : string {
 return 'SELECT ' . implode(',', $this->selectColumns) . ' FROM ' . $this->tableName . ' ';
 }
 public function parseClauseValue($value) : string {
 return is_numeric($value) ? $value : '\'' . $value . '\'';
 }
 public function appendWhereClausesToQuery($query) : string {
 $query .= 'WHERE ';
 foreach ($this->whereClauses as $key => $clause) {
 if ($key >= 1) {
 $query .= ' ' . $clause['join'] . ' ';
 }
 if (array_key_exists('raw_sql', $clause)) {
 $query .= $clause['raw_sql'];
 }
 else {
 $parsedValue = $this->parseClauseValue($clause['value']);
 $query .= $clause['column'] . ' ' . $clause['operator'] . ' ' . $parsedValue;
 }
 }
 return $query;
 }
 public function appendOrderByToQuery($query) : string {
 $query .= ' ORDER BY ' . $this->orderBy;
 return $query;
 }
 public function appendLimitSqlToQuery($query) : string {
 $query .= ' LIMIT ' . $this->maxResults . ';';
 return $query;
 }
 public function toSql() {
 $query = $this->buildDefaultQuery();
 if (count($this->whereClauses) > 0) {
 $query = $this->appendWhereClausesToQuery($query);
 }
 if (strlen($this->orderBy) > 0) {
 $query = $this->appendOrderByToQuery($query);
 }
 if ($this->maxResults >= 0) {
 $query = $this->appendLimitSqlToQuery($query);
 }
 return $query;
 }
}

• mickmackusa: "I am concerned I'm not good enough." ...me too and I've been doing web dev since 2007. Perhaps this is an unshakable feeling for some people.
• Rick James: Practice, practice. Experiment. Ask questions. You can get better; focus on that, not on "good enough".

Answer 1

Initial Feedback

The code looks quite modern - with return types declared, chaining supported, etc. The code is somewhat in-line with PSR-12 and thus is quite readable, though a few rules aren't followed - e.g.

• 4.4 Methods and Functions

  Method and function names MUST NOT be declared with space after the method name. The opening brace MUST go on its own line, and the closing brace MUST go on the next line following the body. There MUST NOT be a space after the opening parenthesis, and there MUST NOT be a space before the closing parenthesis.

  I don't totally agree with this one after having worked with PHP (and JavaScript) for many years involving methods and functions where the opening brace is on the same line as the method name.

• 4.5 Method and Function Arguments

  When you have a return type declaration present, there MUST be one space after the colon followed by the type declaration. The colon and declaration MUST be on the same line as the argument list closing parenthesis with no spaces between the two characters.

Suggestions

DocBlocks

PSR-5 is in draft status currently but is commonly followed amongst PHP developers. It recommends adding docblocks above structural elements like classes, methods, properties, etc. Many popular IDEs will index the docblocks and use them to suggest parameter names when using code that has been documented. At least do it for the methods like the constructor, in case you forget which order the parameters are in.

Comparison operators

In methods where() and orWhere() there is a comparison with null

if ($value == null) {

This uses loose equality checking. It is wise to get in the habit of using strict equality comparisons - i.e. === and !== unless you are sure you want to allow types to be coerced ¹ .

SQL Injection prevention: Bound parameters

SQL injection is very important to consider! One technique for mitigating this is to use bound parameters. I'd want to ensure parameters are bound to my queries whenever possible.

Default values for properties

The constructor sets values for these properties:

$this->whereClauses = [];
$this->selectColumns = ['*'];
$this->maxResults = -1;

Those lines could be eliminated by setting the values on the property declarations:

 private $whereClauses = [];
 private $selectColumns = ['*'];
 private $maxResults = -1;

Answer 2

When I started using MySQL 22 years ago, I thought about doing what you are doing. I quickly decided against it -- It is too much of a "moving target", and some constructs get quite clumsy to specify.

But the coup de grâce was when I realized that the number of keystrokes would be more than simply learning and writing SQL.

Instead, I adopted a coding style of indenting, when to split rows, etc.

Still, your project is a useful learning exercise. It will 'force' you to learn all the syntax and what it all does.

PS: stackoverflow.com has lots and lots of questions about MySQL.

• Mantykora 7: When I started using MySQL 22 years ago It's a long time ago:)
• Rick James: @Mantykora7 - I just read a Question for which one answer was "use STRAIGHT_JOIN". The response came back: "Alas, that construct is not available in the framework I am using."
• slepic: It has downsides too. A piece of typed code which calls multiple well defined methods on a builder instance is much easier to validate then a raw sql query string. But I agree that sometimes when you need something special, builders just don't have it.