php one-time prepared statement execution function

Question

I use prepared statements often even when I only need to execute the statement once (for security), so I implemented a function to abstract away all the function calls on the mysqli_stmt object, as well as bind_param()'s first argument since as far as my tests show it works identically even when int parameters are marked as strings.

<?php
$conn = new mysqli('localhost', 'name', 'password', 'db');

if ($conn->connect_error)
    die('Connection to database failed: ' . $conn->connect_error);

function stmt($query, $params){

   array_unshift($params, str_repeat('s', sizeof($params)));
   for($i = 1; $i < sizeof($params); $i++){
      $params[$i] = &$params[$i];
   }

   $stmt = $GLOBALS['conn']->stmt_init();
   $stmt->prepare($query);
   $method = new ReflectionMethod('mysqli_stmt', 'bind_param');
   $method->invokeArgs($stmt, $params);
   $stmt->execute();

   if($stmt->error){
      $result = ['error' => $stmt->error];
   } else {
      $result = $stmt->get_result();
   }

   $stmt->close();
   return $result;
}
?>

Usage example:

<?php
   $result = stmt('SELECT * FROM table_name WHERE id IN(?,?,?)', [1,2,3]);
?>

Answer 1

I tested your code and it does work, so far, so good. This type of code has been written many times, so by searching online you can find a lot of good examples.

The main problem I have with your code is that it is rather difficult to understand. I can work it out, but it is not obvious. Starting with prepending the $params with something, then the very weird: $params[$i] = &$params[$i] loop, followed by the usage of ReflectionMethod normally used for reverse-engineering code.

I prefer more down to earth code for a simple function like this. Something like:

function executeQuery($mysqli, $query, $parameters)
{
    $stmt = $mysqli->stmt_init();
    if ($stmt->prepare($query)) {
        $types = str_repeat("s", count($parameters));
        if ($stmt->bind_param($types, ...$parameters)) {
            if ($stmt->execute()) {
                return $stmt->get_result();
            }
        }
    }
    return ['error' => $stmt->error];
}

Short and sweet. Some notes:

• I try to use a function name that actually reflects what the function does.
• I supply the database connection as an argument, for more flexibility. You can use multiple database connections and they don't need to be in the global scope.
• I check whether the query could be properly prepared.
• My code differs quite a bit from your code when it comes to binding the parameters. As you can see this is quite straightforward. Using ... to access variable arguments has been available since PHP 5.6 which came out in 2014.
• By directly returning the results when the execution was successful I know that an error must have occurred when the last line of the function is executed. This therefore also catches other problems.

Personally I would not have expected to get a MySQLi result object out of this function. Because it will always have to be processed. Why not do this processing inside this function? Like this:

function executeQuery($database, $query, $parameters)
{
    $stmt = $database->stmt_init();
    if ($stmt->prepare($query)) {
        $types = str_repeat("s", count($parameters));
        if ($stmt->bind_param($types, ...$parameters)) {
            if ($stmt->execute()) {
                if ($result = $stmt->get_result()) {
                    $rows = [];
                    while ($row = $result->fetch_assoc()) {
                        $rows[] = $row;
                    }
                    return $rows;
                }
            }
        }
    }
    return ['error' => $stmt->error];
}

Now you simply get an array back. I agree that is not much different from returning a MySQLi result, but I am thinking ahead. Suppose you decide to change over from MySQLi to PDO in the future. You can easily recode the function above to work with PDO, but recoding the handling of MySQLi results everywhere in your code will be a lot harder. So I am using the function to abstract away from a particular database interface.

Some people don't like the deep nesting of if () {} blocks. To prevent this you could instead write something, like the code below, for all these blocks:

 if (!$stmt->prepare($query)) {
     return ['error' => $stmt->error];
 }

I have to repeat that there are lots of ways of doing this. The answer I gave is based on the code you presented. It is, for instance, not hard to find out the type of the parameters, and adjust the $types string accordingly.

Answer 2

First of all, that's a very good idea to create such a function. It says you are a programmer in your heart. Sadly, but most PHP users never come to the idea of such an automation, writing thousands of repeated lines of code over and over again.

What could be criticized about your code is already pretty much covered in the other answer. However, the solution offered there is still far from being optimal.

First of all, the error reporting is absolutely flawed in both cases. Returning an array with error information instead of the actual query result is absolutely unacceptable. It will lead to numerous errors and confusions in your code. Errors must be thrown, not returned. For mysqli it's especially simple because it can throw exceptions by itself. Check out my article on PHP error reporting principles

Next, returning the mysqli result may cause an error if you are running a DML query. So the only proper return value would be a mysqli statement.

Using "s" for all data types is a very smart move, it will serve you 999 times out of 1000. However, adding a possibility to set the types explicitly is a good idea anyway.

Taking all the above into consideration, I wrote such a function myself, a Mysqli helper function:

function prepared_query($mysqli, $sql, $params, $types = "")
{
    $types = $types ?: str_repeat("s", count($params));
    $stmt = $mysqli->prepare($sql);
    $stmt->bind_param($types, ...$params);
    $stmt->execute();
    return $stmt;
}

As you can see, it is not only much simpler but also much more flexible. Note the examples section in the article linked above. As you can see, I tested this function with many query types and return values.

On a side note, the proper mysqli connection is a bit more complex than just a single line of code.

Answer 3

My code after improvement based on the reviews

And another improvement: if no query parameters are passed, run a simple query instead of a prepared statement.

Note: one thing I didn't implement from the reviews is the idea of returning an array instead of the mysqli_result object - because of performance and memory considerations.

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
try {
   $conn = new mysqli('localhost', 'username', 'password', 'db');
   $conn->set_charset('utf8mb4');
} catch (Exception $e) { // catch and re-throw to avoid exposing database credentials
     throw new Exception($e->getMessage(), $e->getCode());
}

function query($mysqli, $sql, $params = [], $types = '')
{
   if(!$params){
      return $mysqli->query($sql);
   }
   $types = $types ?: str_repeat('s', count($params));
   $stmt = $mysqli->prepare($sql);
   $stmt->bind_param($types, ...$params);
   $stmt->execute();
   try{
      $result = $stmt->get_result();
   } catch (Exception $e) {
      $result = false;
   }
   $stmt->close();
   return $result;
}