PHP MySQLi Wrapper Class For API

Question

I've never had my code reviewed before, so I am taking this opportunity to see what can be improved with my coding style. I am still somewhat "new" to PHP, in the sense that I'm an old dog finally learning new tricks.

Here is a MySQLi wrapper I've coded in PHP. Overall, this code is part of an API project. I'm making an API in PHP which I will use for my WPF Desktop Application for the Database Models.

Here is Class.Config.php:

<?php

namespace Database;

use Exceptions\DatabaseException;
use Exceptions\ConnectionException;

class Config
{
    /**
    * @var string $host The connection host.
    * @var string $username The connection username.
    * @var string $password The connection password.
    * @var string $dbname The connection database name.
    */
    private string $host, $username, $password, $dbname;

    /**
    * @var \mysqli $conn The mysqli connection object.
    */
    private \mysqli $conn;

    /**
    * Configures the Config object with the default connection string.
    *
    * @param array $config The config array, expects indexes of: host, username, password, and dbname.
    * @param mysqli $connection The optional mysqli connection object.
    */
    public function __construct(?array $config = null, \mysqli $connection = null)
    {
        if ($connection !== null && !($connection instanceof \mysqli))
        {
          throw new \InvalidArgumentException(
            "Invalid mysqli connection object passed. Instance of 'mysqli' expected, '" . gettype($connection) . "' given."
          );
        }

        if ($config === null)
        {
            // Retrieve an array of the connection data from the database.ini file.
            $config = parse_ini_file("database.ini");
        }

        // Retrieve the database credentials from the parsed file
        foreach($config as $key => $value)
        {
          $this->$key = $value ?? null;
        }

        // Verify that the config file properly sets up the variables.
        try
        {
            if (!isset($this->host, $this->username, $this->password, $this->dbname)
                || empty($this->host)
                || empty($this->username)
                || empty($this->password)
                || empty($this->dbname))
            {
                throw new \InvalidArgumentException("Connection string expects values for ['host', 'username', 'password', 'dbname']");
            }
        }
        catch (\InvalidArgumentException $e)
        {
            error_log("Missing Configuration Index: {$e}");
            throw $e;
        }

        // Create a new MySQLi connection
        $this->conn =
            $connection ??
            new \mysqli(
                $this->host,
                $this->username,
                $this->password,
                $this->dbname
            );
    }

    /**
    * Validates the connection.
    *
    * @return mysqli The mysqli connection object.
    */
    public function connect()
    {
        try
        {
            /**
            * Checks to see if there are any errors with the validated connection string.
            *
            * @var \mysqli $this->conn The connection object configured in the constructor.
            */
            if ($this->conn->connect_error)
            {
                throw new ConnectionException(
                  $this->conn->connect_error
                );
            }
            else
            {
                return $this->conn;
            }
        }
        catch (ConnectionException $e)
        {
            error_log("Connection Error: " . $e->getMessage());
            throw $e;
        }
        catch (DatabaseException $e)
        {
            error_log("Database Error: " . $e->getMessage());
            throw $e;
        }
        catch (Exception $e)
        {
            error_log("Error: " . $e->getMessage());
            throw $e;
        }
    }
}
?>

Here is class.MySQLi.php:

<?php
namespace Database;

use Database\Config;

class MySQLi
{

  /**
  * @var \mysqli $conn The mysqli connection object.
  */
  private \mysqli $conn;

  /**
  * Initialize the connection between the database config.
  */
  public function __construct()
  {
      $conn = new Config;
      $this->conn = $conn->connect();
  }

  /**
  * Method for getting the connection variable.
  *
  * @return \mysqli | bool $this->conn The mysqli object configured in the constructor.
  */
  public function getConnection() : \mysqli | bool
  {
    return $this->conn;
  }

  /**
  * Close the connection once the script ends.
  */
  public function __destruct()
  {
      $this->conn->close();
  }

  /**
  * Build a parameterized query to protect against SQL injections by default.
  *
  * @param string $query The SQL query string with placeholders for parameters.
  * @param mixed ...$params The array? of parameters to be bound to the query.
  * @return \mysqli_result|bool Returns the result of the query or false if an error is encountered.
  */
  public function Query(string $query, ...$params) : \mysqli_result | bool
  {
      // Prepare the SQL statement or trigger an error if preparation fails.
      $statement = $this->getConnection()->prepare($query) or trigger_error($this->getConnection()->error, E_USER_ERROR);

      if (!$statement) return false;

      // Build the parameters if provided.
      if(!empty($params))
      {
          $types = [];
          $bindParams = [];

          foreach ($params as $param)
          {
              // Get the type of each parameter: i, d, s, or b.
              $types[] = $this->getParamType($param);
              $bindParams[] = $param;
          }

          try
          {
              $types = implode("", $types);
              $statement->bind_param($types, ...$bindParams);
          }
          catch (\ArgumentCountError $e)
          {
              return false;
          }
      }

      // Execute the statement and return the result if successful
      if ($statement->execute())
      {
          $result = $statement->get_result();
          $statement->close();
          return $result;
      }

      return false;
  }

  /**
  * Insert rows of data into a database table.
  *
  * @param string $query The SQL query string with placeholders for parameters.
  * @param mixed ...$params The array? of parameters to be bound to the query.
  */
  public function Insert(string $query, ...$params)
  {
  // Still debating usage.
  }

  /**
  * Fetch rows of data from a database table.
  *
  * @param string $query The SQL query string with placeholders for parameters.
  * @param mixed ...$params The array? of parameters to be bound to the query.
  * @return array|false Returns an array of rows on success, or false on failure.
  */
  public function Rows(string $query, ...$params) : array | bool
  {
    if($result = $this->Query($query, ...$params))
    {
        $rows = [];

        while ($row = $result->fetch_assoc())
        {
            $rows[] = $row;
        }

        return $rows;
    }

    return false;
  }

  /**
 * Get the parameter type for binding in a prepared statement.
 *
 * @param mixed $param The parameter value.
 * @return string Returns the parameter type character.
 */
  private function getParamType($param) : string
  {
      if (is_int($param))
      {
          return "i";
      }
      elseif (is_float($param))
      {
          return "d";
      }
      elseif (is_string($param))
      {
          return "s";
      }
      else
      {
          return "b";
      }
  }
}
?>

Usage

Simplify Fetching Rows From Database

With my wrapper class, I can simply do this:

// Get the database connection
$conn = new MySQLi;

// SQL query to select all data from the table
$query = "SELECT * FROM structure_ranks";

// Fetch all rows from the result set and store them in the array
$rows = $conn->Rows($query);

// Output the array as JSON
echo json_encode($rows, JSON_PRETTY_PRINT);

As opposed to doing this:

// Create an instance of the Config class
$config = new Config();

// Get the database connection
$conn = $config->connect();

// Check the connection
if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

// SQL query to select all data from the table
$sql = "SELECT * FROM structure_ranks";

// Execute the query
$result = $conn->query($sql);

// Fetch all rows from the result set and store them in the array
$array = [];
if ($result->num_rows > 0) {
    while ($row = $result->fetch_assoc()) {
        $array[$row['id']] = $row;
    }
}

// Close the connection
$conn->close();

// Output the array as JSON
echo json_encode($array, JSON_PRETTY_PRINT);

Answer 1

There is quite a lot of issues and possible improvements. For example, the entire Config class being one big pile of useless cargo cult code that you wrote not because it is really needed but just to try your hand at those "new tricks". Just one quick example. The following condition will never be evaluated to true, being essentially useless:

if ($connection !== null && !($connection instanceof \mysqli))

simply because one line above you already asked PHP to check the $mysqli variable type and throw exception in case it's incorrect:

public function __construct(?array $config = null, \mysqli $connection = null)

Here you can see a live demonstration. You are expecting this code to throw \InvalidArgumentException with custom error message in case $mysqli is not null and not mysqli. But, as you can see, it's another error, which is thrown even before the execution reached that condition.

And it's the case with almost every other exception thrown in your code.

Yet I am not sure whether you are inclined to listen (or even still have any interest in a review at all). So I would rather concentrate on the false assumptions that led you to the creation of these two rather monstrous classes.

The "As opposed to doing this", being rewritten using the correct approach and the most recent PHP version, should be read as

// get the configuration options
$config = require 'config.php';

// Get the database connection
$conn = new mysqli(...$config['db']);

// SQL query to select all data from the table
$sql = "SELECT * FROM structure_ranks";

// Fetch all rows from the result set and store them in the array
$result = $conn->query($sql)->fetch_all(MYSQLI_ASSOC);

// Output the array as JSON
echo json_encode($array, JSON_PRETTY_PRINT);

If you have any questions regarding the code present or not present in this example, feel free to ask in the comments or you may help yourself to the short article on mysqli I wrote.

As you can see, when used properly, mysqli is already as concise as your custom solution. And even with prepared statements:

// SQL query using a variable
$sql = "SELECT * FROM structure_ranks WHERE rank > ?";

// Fetch all rows from the result set and store them in the array
$result = $conn->execute_query($sql, [$rank])->fetch_all(MYSQLI_ASSOC);

// SQL query using a variable
$sql = "INSERT INTO structure_ranks (rank, whatever, foo, bar) VALUES (?,?,?,?)";

// A separate Insert method is indeed of no use
$conn->execute_query($sql, [$rank, $whatever, $foo, $bar]);

The only place for improvement I can see here is the syntax for getting all rows which is indeed rather verbose. So in your place I would focus this custom class on several fetch methods that return different types of array, such as

• fetchRows() just an alias for fetch_all(MYSQLI_ASSOC)
• fetchList() to get values of single columns
• fetchIndexed($index = 'id') that implements the fetching loop from "as opposed" section
• fetchDict() which combines the above two, taking the first selected column for index and the second for value
• fetchCell() just an alias for native mysqli's fetch_column()
• fetch() just an alias for fetch_assoc()

Given you showed some interest in the review, I will add some. A quite good example is

try
    {
        if (!isset($this->host, $this->username, $this->password, $this->dbname)
            || empty($this->host)
            || empty($this->username)
            || empty($this->password)
            || empty($this->dbname))
        {
            throw new \InvalidArgumentException("Connection string expects values for ['host', 'username', 'password', 'dbname']");
        }
    }
    catch (\InvalidArgumentException $e)
    {
        error_log("Missing Configuration Index: {$e}");
        throw $e;
    }
}

which, taken by itself, without any context, can be rewritten to

if (!$this->host || !$this->username || !$this->password || !$this->dbname) {
    $e = new \InvalidArgumentException("Connection string expects values for ['host', 'username', 'password', 'dbname']");
    error_log("Missing Configuration Index: {$e}");
    throw $e;
}

because

• isset() being useless for checking properties that deliberately exist, being defined in the class definition. Here is another live demonstration: even without isset() this code behaves exactly the same.
• empty() being twice useless as it is checking the existence of the existing variable that was already checked for existence by isset
• try being useless because it is used here not on purpose, but for the control flow, which is already done by if statement.

Yet, taking into account the actual context, this code block should be just removed altogether. Because, again, in case your credentials are incorrect, mysql will readily provide the error message telling you that. Besides, this pesky verification just gets in the way. Why can't I provide an empty parameter - or null, to use the default value?

Another vivid example is

    catch (ConnectionException $e)
    {
        error_log("Connection Error: " . $e->getMessage());
        throw $e;
    }
    catch (DatabaseException $e)
    {
        error_log("Database Error: " . $e->getMessage());
        throw $e;
    }
    catch (Exception $e)
    {
        error_log("Error: " . $e->getMessage());
        throw $e;
    }

As we can see, the code is basically duplicated here three times, only to add textual remarks that are rather superfluous. Why not to leave just the Exception case (or Throwable for that matter)?

Yet again, catching exception only to log it makes little sense. PHP already can log errors. Why not to just leave this exception alone and let PHP log it just like any other error?

I honestly, genuinely don't understand, why database errors should receive some special treatment or a supplementary textual remark. Like, never in my life I've seen anyone wrapping a require call in a try-catch that simply adds "Filesystem error:" and throws the exception again. But for some reason many people consider essential to add a try catch for the database call.

A rather small issue: here, the else clause is superfuluous

        {
        else
        {
            return $this->conn;
        }

and usually written as

        }
        return $this->conn;

Finally, like I said in the comments, the if statement here is superfluous too.

        if ($this->conn->connect_error)
        {
            throw new ConnectionException(
              $this->conn->connect_error
            );
        }

Like it is said in the article I linked above, you can always configure mysqli to throw exceptions by itself, which renders all such conditions also useless.