1
\$\begingroup\$

I'm new to web programming, and writing some web apis to enable users to sync their data between devices.

Is it okay to use singleton pattern to prevent reconnecting the MySQL database on every api call?

<?php

final class MySQLiConnection
{
    private static $connection = null;

    private function __construct() {}

    public static function getInstance() : MySQLiConnection
    {
        if ($connection == null || !$connection->ping())
        {
            $connection = new mysqli("localhost", "id", "password", "database");
        }

        return $connection;
    }

    public function execute(string $sql, iterable $params) : void
    {
        if ($statement = $connection->prepare($sql))
        {
            foreach ($params in $param)
            {
                if (is_int($param))
                {
                    $statement->bind_param("i", $param);
                }
                else if (is_double($param))
                {
                    $statement->bind_param("d", $param);
                }
                else if (is_string($param))
                {
                    $statement->bind_param("s", $param);
                }
            }

            $statement->execute();
            $statement->close();
        }
    }

    public function getResult(string $sql, iterable $params) : mysqli_result
    {
        if ($statement = $connection->prepare($sql))
        {
            foreach ($params in $param)
            {
                if (is_int($param))
                {
                    $statement->bind_param("i", $param);
                }
                else if (is_double($param))
                {
                    $statement->bind_param("d", $param);
                }
                else if (is_string($param))
                {
                    $statement->bind_param("s", $param);
                }
            }

            $statement->execute();

            $result = $statement->get_result();
            $statement->close();

            return $result;
        }
    }
}

// example
MySQLiConnection::getInstance()->execute("SELECT * FROM users WHERE forename = ? AND surname = ?", "ben", "dover");

?>
\$\endgroup\$
  • \$\begingroup\$ There are some rather important issues in your second code, I encourage you to post another question with the renewed code. It is completely allowed by the site rules and will let me to review the new version \$\endgroup\$ – Your Common Sense Jan 25 at 5:12
  • \$\begingroup\$ @YourCommonSense Thanks =) codereview.stackexchange.com/questions/212212/… \$\endgroup\$ – Taekmin Lee Jan 25 at 14:10
1
\$\begingroup\$

First, on your premises.

  • First of all, no pattern would prevent PHP from making a connection to MySQL database on every web api call. Because PHP will die between different API calls, along with all its singletons, connections and any other stuff. So all you can prevent is reconnecting the MySQL database on every call to a database connection class.
  • No, it is frowned upon using singleton pattern to prevent reconnecting the MySQL database on every call to a database connection class. A more accepted approach is Dependency Injection.
  • Funny enough, due to a typo, this class would not prevent reconnecting the mysql. Had you error_reporting set to E_ALL, PHP would have signaled that you are trying to use a non-existent variable $connection every time getInstance() is called. Surely you wanted to call it as $this->connection instead.

Now to the code.

The intention is very good, especially I like getResult() and execute() methods that allow you to avoid that bind param hassle. However, there is evidently a duplicated code. Why not to make execute to return $statement? It will let you make getResult() as simple as

public function getResult(string $sql, iterable $params) : mysqli_result
{
    return $this->execute($sql, $params)->get_result();
}

And finally. I just noticed, halfway the review process, that your code is off topic, as it simply doesn't work. Mysqli is not PDO, you cannot bind your parameters in a loop. So you need to rework it. In order to help, here are two my articles:

\$\endgroup\$

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.