8
\$\begingroup\$

I'm working on a web application that I inherited from a colleague long gone. To connect to the MySQL database I use the following classes:

statement.php

<?php
//class sql
//{
    class Statement
    {
        private $m_connection;

        function __construct($connection)
        {
            $this->m_connection = $connection;
        }

        function query($query)
        {
            $connection = $this->m_connection;
            $handle = $connection->getHandle();

            $result = mysql_query($query,$handle);

            if ($result === FALSE)
                throw new Exception(mysql_error());

            $instance = new Result($this);
            $instance->setHandle($result);

            return $instance;
        }
    }
//}
?>

result.php

<?php
//class sql
//{
    class Result
    {
        private $m_handle;
        private $m_statement;

        function __construct($statement)
        {
            $this->m_statement = $statement;
        }

        function setHandle($handle)
        {   
            $this->m_handle = $handle;
        }

        function fetch()
        {
            $handle = $this->m_handle;          
            $row = mysql_fetch_array($handle);

            return $row;
        }
    }
//}
?>

connection.php

<?php
include_once("statement.php");
include_once("result.php");

//class sql
//{
    class Connection
    {
        private $m_handle;

        function __construct($server,$username,$password)
        {
            $handle = mysql_connect($server,$username,$password);
            $this->m_handle = $handle;
        }

        function __destruct()
        {
            $handle = $this->m_handle;
            @mysql_close($handle);
        }

        function createStatement()
        {
            return new Statement($this);
        }

        function getHandle()
        {
            return $this->m_handle;
        }
    }
//}
?>

named.php

<?php
include_once("connection.php");

function createNamedConnection($name)
{
    if ($name == "dbalias1")
    {
        $connection = new Connection("first.host.com","user","uspw");

        $statement = $connection->createStatement();
        $statement->query("USE db1");

        return $connection;
    }   
    if ($name == "dbalias2")
    {
        $connection = new Connection("second.host.com","user2","nouse");

        $statement = $connection->createStatement();
        $statement->query("USE db2");

        return $connection;
    }

    return null;
}
?>

To have a connection I can use the following script:

$connection = createNamedConnection("dbalias1");
$statement = $connection->createStatement();
$query = "SELECT   *
          FROM     tContent c
          WHERE    c.cID > 100";
$result = $statement->query($query);
while(($row = $result->fetch()) !== FALSE){
    //do something
}

The problem is that those classes caused me a lot of trouble, and I'm sure there is work to be done on them. I don't know where to begin to make those classes more secure and easy to use.

\$\endgroup\$
8
  • 4
    \$\begingroup\$ What kind of trouble are they causing? \$\endgroup\$
    – Michael K
    Feb 4, 2011 at 13:54
  • \$\begingroup\$ @Michael: Well mainly I got this kind of message: Warning: mysql_query(): 7 is not a valid MySQL-Link resource in E:\webroot\dbtest\lib\statement.php on line 18 and it is difficult each time to find out the reason why. But it is only one example of the problem I had. To summarize debugging is quite difficult. \$\endgroup\$
    – Eldros
    Feb 4, 2011 at 14:12
  • \$\begingroup\$ What's the point of separating connection, statement, result? \$\endgroup\$
    – Shoe
    Feb 4, 2011 at 14:50
  • 1
    \$\begingroup\$ @Charlie: You tell me, I can't ask my (ex-)colleague anymore. \$\endgroup\$
    – Eldros
    Feb 4, 2011 at 15:11
  • \$\begingroup\$ Pdo already do it. Use it or rewrite this code by your own. Also delete those includes and use and __autoload() function. :) \$\endgroup\$
    – Shoe
    Feb 4, 2011 at 15:23

2 Answers 2

4
\$\begingroup\$

Consider using mysqli instead of mysql:

mysqli

Mysqli (the "i" stands for "improved") is more OO-friendly and has better security. It also seems like the standard mysql functions are being deprecated (some of them anyway).

\$\endgroup\$
1
  • \$\begingroup\$ I've finally taken the time to do the conversion to mysqli, and it seemed to need much more resources. Is there any performance concern about mysqli compared to mysql? \$\endgroup\$
    – Eldros
    Sep 30, 2011 at 8:43
1
\$\begingroup\$

I believe this code is unnecessary:

class Connection
{
  //...

    function __destruct()
    {
        $handle = $this->m_handle;
        @mysql_close($handle);
    }
   //...
}

As I understand it, MySQL connections should be cleaned up when your script finishes executing anyway. It looks like this is the cause of the bug, since the handle is being closed and later a query is attempted on the handle, causing the error message you're getting.

What's not clear to me on first glance is why the Connection object is being destructed (and taking the MySQL database handle with it) when there's still a statement holding a reference to it. It's possible that you're cloning the connection somewhere, so that there are two connections holding the same MySQL handle. When the first one gets garbage collected the handle is closed, even though the second one is still using it. If my guess here is correct, then removing the destructor should fix the problem.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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