0

I'm a .Net developer that have taken over a PHP project. This project has a database layer that looks like this:

<?php

class DatabaseManager {

    private static $connection;

    const host = "projectname.mysql.hostname.se";
    const database = "databaseName";
    const username = "userName";
    const password = "password";

    public function __construct()
    {

    }

    public function instance($_host = null, $_username = null, $_password = null)
    {
        if(!self::$connection)
        {
            if(!$_host || !$_username || !$_password)
            {
                $host = self::host;
                $username = self::username;
                $password = self::password;         
            }else{
                $host = $_host;
                $username = $_username;
                $password = $_password;
            }
            self::$connection = mysql_connect($host, $username, $password);
            $this->setDatabase();
        }
        return self::$connection;
    }

    public function setDatabase($_database = null)
    {
        if(!$_database)
        {
            $database = self::database;
        }else{
            $database = $_database;
        }
        $connection = $this->instance();
        mysql_select_db($database, $connection) or die(mysql_error());  
    }
} ?>

I have written a php file that uses this layer but after a while i got these mysql errors implying i didn't close my connections which i hadn't. I try to close them but know i get other weird errors like system error: 111. Very simplyfied my php file looks like this:

<?php
$return = new stdClass();

$uid = '9999999999999';
$return->{"myUid"} = $uid;

$dm = new DatabaseManager();
$dmInstance = $dm->instance();

/* MY CLICKS */
$sql = sprintf("SELECT count(*) as myClicks FROM clicks2011, users2011 WHERE clicks2011.uid = users2011.uid AND users2011.uid = %s AND DATEDIFF(DATE(at), '%s') = 0 AND exclude = 0", mysql_real_escape_string($uid), mysql_real_escape_string($selectedDay));
$result = mysql_query($sql, $dmInstance) or die (mysql_error());
$dbResult = mysql_fetch_row($result);
$return->{"myClicks"} = $dbResult[0];

mysql_close($dmInstance);

echo json_encode($return); ?>
Johan B
  • 1,966
  • 1
  • 21
  • 39
  • What does `$return->{"myUid"} = $uid` means? never seen such a thing in PHP. Also you should go with PDO since what you are trying to do with `sprintf()` is just a walkaround of what PDO does better. – Shoe Mar 11 '11 at 21:11
  • 2
    It seems like there is a problem with your setup, because database connections are automatically closed after the script finishes executing (unless they are persistent, of course). ie: you don't explicitly call `mysql_close()` – NullUserException Mar 11 '11 at 21:12
  • @Charlie that line is casting the string to an object. It seems like an egregious waste of processing power to me, but I'm sure there are some valid uses for it. See [this question](http://stackoverflow.com/questions/931407/what-is-stdclass-in-php) for an explanation of stdClass (which is instantiated on the first line of @Johan s script). As for the actual problem, it is not with this code, as @NullUserException said: mysql connections are closed automatically. For anyone who is interested, [My PHP PDO DAO](https://github.com/rockerest/myframework/blob/master/backbone/Database.php) – rockerest Mar 11 '11 at 21:21

3 Answers3

2

Okay, I'm going to post this as an answer because I think one (possibly both) of these things will help you.

First: You don't need to manually close your MySQL connections. Unless you have set them up so that they persist, they will close automatically. I would avoid doing that unless you determine that every other problem is NOT the solution.

In addition, I would switch to using prepared statements. It's more secure, and pretty future-proof. I prefer PHP's PDO over mysqli, but that's up to you.

If you'd like to look over an example of a simple PDO object to take the many lines out of creating prepared statements and connections and getting results, you can look at my homebrew solution.

Second: "System Error 111" is a MySQL error. From what I've read, it appears that this error typically occurs when you are using PHP and MySQL on the same server, but telling PHP to connect to MySQL via an IP address. Switch your $host variable to 'localhost'. It is likely that this will solve that error.

rockerest
  • 9,847
  • 3
  • 33
  • 67
  • Thanks man. If i had the time, I'd definitely read up on the best practices and rewrite the code. I'll have to taint my soul and stick with this code abomination for the moment though :/. – Johan B Mar 14 '11 at 08:43
1

The problem here is you're calling mysql_close and not specifying a valid mysql connection resource object. You're, instead, trying to close an instance of the DatabaseManager object.

You'll probably want to run mysql_close(DatabaseManager::connection); which is where the DatabaseManager is storing the resource object.

Additionally, I'd personally recommend you learn PDO or use the mysqli drivers. In future releases of PHP the built in mysql functions will be moved into E_DEPRECATED

vicTROLLA
  • 1,554
  • 12
  • 15
  • Oh... I didn't realize $dmInstance was a return of self::$connection. Keep in mind that the $connection variable is being scoped as private. You may want to open that up with 'public'. – vicTROLLA Mar 11 '11 at 21:14
1

Try implement __destrcut

public function __destruct()
{
    mysql_close(self::$connection)
}

Then simply use unset($dm);

slier
  • 5,855
  • 5
  • 31
  • 51