Page 1 of 1

Help optimising this MySQL wrapper

Posted: Mon Jul 23, 2007 10:10 am
by spamyboy
I found this MySql in one tutorial, but it doesnt realy look `smoth` to me for now.
What could I add, update, fix to make it faster / more optimised.

Code: Select all

<?php
class sql
{
    var $dbhost;
    var $dbuser;
    var $dbpass;
    var $dbase;
    var $sql_query;
    var $mysql_link;
    var $sql_result;
    var $query_count;

    function sql()
    {
	
        $this->dbhost = '***';
        $this->dbuser = '***';
        $this->dbpass = '***';
        $this->dbase = '***';
	
        $this->mysql_link = '0';
        $this->query_count = '0';
        $this->sql_result = '';
        $this->connection();
    }

    function connection()
    {
        $this->mysql_link = @mysql_connect( $this->dbhost, $this->dbuser, $this->dbpass ) or $this->error( mysql_error( $this->mysql_link ), $this->sql_query, mysql_errno( $this->mysql_link ) );
        @mysql_select_db( $this->dbase ) or $this->error( mysql_error( $this->mysql_link ), $this->sql_query, mysql_errno( $this->mysql_link ) );
    }

    function error( $error_msg, $sql_query, $error_no )
    {
	    $error_date = date("F j, Y, g:i a");
//Heredoc Strings must be on the far left lines        
$error_page = <<<EOD
<html><head><title>mySQL database Error</title>
<style>P,BODY{ font-family:arial,sans-serif; font-size:11px; }</style></head><body>
&nbsp;<br><br><blockquote><b>There appears to be an error while trying to complete your request.</b><br>
You can try to go back and try again by clicking <a href="javascript:history.go(-1);">here</a>, if you
still get the error you can contact the site administrator by clicking <a href='mailto:spamyboy@gmail.com?subject=SQL+Error+lyricing'>here</a>
<br><br><b>Error Returned</b><br>
<form name='mysql'><textarea rows="15" cols="60">mySQL query error: {$sql_query}
mySQL error: {$error_msg}
mySQL error code: {$error_no}
Date: {$error_date}</textarea></form><br>We apologise for any inconvenience</blockquote></body></html>
EOD;
//echo $error_page;
    }

    function close()
    {
        mysql_close( $this->mysql_link );
    }

    function sql_query( $sql_query )
    {
        $this->sql_query = $sql_query;
        $this->sql_result = @mysql_query( $sql_query, $this->mysql_link );
        if (!$this->sql_result)
        {
            $this->error( mysql_error( $this->mysql_link ), $this->sql_query, mysql_errno( $this->mysql_link ) );
        }
            $count = $this->query_count;
            $count = ($count + 1);
            $this->query_count = $count;
    }

    function num_rows()
    {
        $mysql_rows = mysql_num_rows( $this->sql_result );
        if (!$mysql_rows)
        {
            $this->error( mysql_error( $this->mysql_link ), $this->sql_query, mysql_errno( $this->mysql_link ) );
        }
        return $mysql_rows;
    }

    function affected_rows()
    {
        $mysql_affected_rows = mysql_affected_rows( $this->mysql_link );
        return $mysql_affected_rows;
    }        

    function fetch_array()
    {
        if ( $this->num_rows() > 0 )
        {
            $mysql_array = mysql_fetch_assoc( $this->sql_result );
            if (!is_array( $mysql_array ))
            {
                return FALSE;
            }
            return $mysql_array;
        } 
    }

    function fetch_rows()
    {
        if ( $this->num_rows() > 0 )
        {
            $mysql_array = mysql_fetch_row( $this->sql_result );
            if (!is_array( $mysql_array ))
            {
                return FALSE;
            }
            return $mysql_array;
        }

    function query_count()
    {
        return $this->query_count;
    }
}
}
?>

Posted: Mon Jul 23, 2007 11:32 am
by TheMoose
Faster? Not much.

Optimized? A lot.

To critique it:
- it does not allow for multiple queries to be worked on at the same time
- it incorporates the query, result, connection, and output into a single class
- it's not a singleton (ok that's a biased critique ;))

Separate it to at least 2 classes, one to handle the connection to the database, another to handle querying from the database. This will at least allow you to have multiple queries running at the same time, or at least be able to refer to previous queries should the need arise (which it usually will).

Posted: Mon Jul 23, 2007 1:57 pm
by superdezign
TheMoose wrote:Separate it to at least 2 classes, one to handle the connection to the database, another to handle querying from the database. This will at least allow you to have multiple queries running at the same time, or at least be able to refer to previous queries should the need arise (which it usually will).
I find that the querying should be handled by the database class as that is what a database class should do. Objects all have a purpose and the database object's purpose is to communicate with the database.

However, you should still break this up into two classes, the first handling the database communication, and the second being results. The result objects would only be created for queries that return result sets (SELECT, SHOW, etc.) which can be determined with is_resource(). Each result object will contain the entire results set for you to traverse through the same way you would normally, but they should all be separate so that you can access any given result set at any time. You'll find it necessary because oftentimes one result set will rely on another and you'll need to do queries while handling other result sets.

FYI, the best way to discover what is missing in an application is to use it practically in a project and evolve it to meet your requirements and needs.

Posted: Mon Jul 23, 2007 2:11 pm
by TheMoose
superdezign wrote:I find that the querying should be handled by the database class as that is what a database class should do. Objects all have a purpose and the database object's purpose is to communicate with the database.
Yeah that's what I meant by it, I just chose the wrong words to say it :P

Posted: Mon Jul 23, 2007 2:23 pm
by superdezign
TheMoose wrote:Yeah that's what I meant by it, I just chose the wrong words to say it :P
I figured as much. It'd be strange to have the result holding the database object rather than the database object holding the result.