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.