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

Static utility functions to turn arrays into SQL statements

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

Problem

I have an object that houses only static functions. The functions are stand alone and are not dependent on the state of the object.

I hear that static functions are a nightmare in terms of testing, an anti-pattern if you will. I get it, the state of the object could be impossible to predict.. But these are stateless.

My question is, Should I place these functions into a class of their own or am I ok by leaving them as static methods?

```
class Utility {

public static function arrayToInsertStatement($table, array $data) {
$sql = "INSERT INTO %s (%s) VALUES (:%s)";
return sprintf($sql, $table, implode(', ', array_keys($data)), implode(', :', array_keys($data)));
}

public static function arrayToSelectStatment($table, array $fields, $whereStatement) {
$sql = "SELECT %s FROM %s %s";
if ($whereStatement) {
$whereStatement = sprintf(" WHERE %s", $whereStatement);
}
return sprintf($sql, implode(',', $fields), $table, $whereStatement);
}

public static function arrayToUpdateStatement($table, array $data, $whereStatement = null) {
$t_sql = "UPDATE %s SET %s%s";
$sets = self::arrayToKeyedArray($data);
if ($whereStatement) {
$whereStatement = sprintf(" WHERE %s", $whereStatement);
}
return sprintf($t_sql, $table, implode(',', $sets), $whereStatement);
}

public static function arrayToWhereStatement($data, $joinType = 'AND') {
$joiner = sprintf(" %s ", $joinType);
return implode($joiner, self::arrayToKeyedArray($data));
}

public static function arrayToOrderBy(array $data) {
$o = [];
foreach ($data as $field => $direction) {
$directionBool = substr(strtolower($direction), 0, strlen($direction)) === substr('descending', 0, strlen($direction));
$o[] = sprintf($field . ' %1$s', ($directionBool ? 'DESC' : 'ASC'));
}
return implode($o, ',');
}

public static function arrayToKeyedArray($data) {
return array_map(function($value) {
return sprintf('%1$s = :%1$s', $value)

Solution

It's not so much about the state than it is about coupling. When you test a method, you want to control its dependencies; when the method under test calls a static method, you can't control that dependency and thus, the method under test is coupled with the implementation of that static method, so you might as well consider that static method inlined into the method under test, as far as testing goes.

So, you've called this class Utility. Like the Helpers and Managers of this world, a class called Utility is pretty much an incentive to become a dumping ground for just about anything that's, well, a "utility". Name things after what they're responsible for, and thank yourself later: in this case, a SqlStatementBuilderUtility would make it much clearer what the intent is, and then the class would be much less prone to grow some bubbleSort or sendMail "utility" members in the future.

Side note, the indentation is off; it's like the class-level scope doesn't exist, the class and its members are all at the same indentation level, and that doesn't look right.

I don't think static is the main concern here - indeed, these methods look like good candidates for being static. The problem is what they're being used for. It looks like it's building SQL statements out of whatever values are thrown at it, which obfuscates the actual queries your application is sending to the database server, and raises a flag about sql-injection.

Context

StackExchange Code Review Q#112644, answer score: 2

Revisions (0)

No revisions yet.