Someone find the error in my code?

P

paradoxblue

Gawd
Joined
Jan 4, 2006
Messages
591
this is my php viewdata page. I am getting an error and I can not figure out why.
Mind you it was once used for 3 rows and I added a 4th.
the submit functions work fine as the data shows in my phpmyadmin page.

heres the error
Warning: mysql_fetch_object(): supplied argument is not a valid MySQL result resource in /home/.findus/takcollect/collection/Games/viewdata.php on line 38
Click to expand...

heres the code for the page.. im at a loss,, been working for 2 days trying to figure out wth is up.
<?php
class database
{
private $db_handle;
private $user_name;
private $password;
private $data_base;
private $host_name;
private $sql;
private $results;

function __construct($host="mysql.collection.takagari.com",$user,$passwd)
{
$this->db_handle = mysql_connect($host,$user,$passwd);
}

function dbSelect($db)
{
$this->data_base = $db;
if(!mysql_select_db($this->data_base, $this->db_handle))
{
error_log(mysql_error(), 3, "/phplog.err");
die("Error connecting to Database");
}
}

function executeSql($sql_stmt)
{
$this->sql = $sql_stmt;
$this->result = mysql_query($this->sql);
}
function outputGenerate()
{
echo "<table cellspacing='0' cellpadding='0' width='0' border='1'>\n<tr>";
echo "<tr><td width=\"400\">Number</td><td width=\"100\">Game</td><td width=\"400\">Link</td><td width=\"100\">Console</td></tr><tr>";
while($record = mysql_fetch_object($this->result))
{
echo "<td width='400'>".$record->number."</td>";
echo "<td width='100'>".$record->game."</td>";
echo "<td width='400'>".$record->link."</td>";
echo "<td width='100'>".$record->console."</td>";
echo "</tr><tr></tr><tr>";
}
echo "</tr></table>";
}
}
$host='mysql.collection.takagari.com';
$user = "takcollect";
$passwd = "*****";
$db = "games";
$sql = "SELECT * FROM games ORDER BY games";
$dbObject = new database($host,$user,$passwd);
$dbObject->dbSelect($db);
$dbObject->executeSql($sql);
$dbObject->outputGenerate();
?>
Click to expand...
 
Is that code a direct copy and paste from your code? The reason I ask is that two things I see just visually is the '$pas swd' argument to the __construct function might be something from retyping in this editor.

Also, the argument to mysql_fetch_object is $this->result. Should this be $this->results?

Just shooting for something. I haven't done enough PHP other than code tweaks.
 
That'd be the key. You declare the variable $results, but from thenon refer to $this->result, which just creates a locally scoped variable that other functions can't access. Make those names consistent and you'll be good.

A few other remarks:
* for the love of all that is holy, indent your code
* your constructor could use some error catching.
* I can't think of any good reason to store the password, etc. as variables. If they're class members, they're in RAM. RAM can be written to disk as cache or can be read by malicious processes. Have them passed as parameters to a connect() method or something, and then discarded.
 
Classes are used to encapsulate parts of your code. Try to keep everything a class needs inside that class... i.e. Don't pass connection variables to a class that could be kept inside the class. The other option is to set the classes values when you declare the variables.


Code is untested...
Code: 
<?php
class database
{
	private $db_handle;
	private $user_name;
	private $passwd;
	private $data_base;
	private $host_name;
	private $sql;
	private $results;

	function __construct()
	{
		$this->host_name='mysql.collection.takagari.com';
		$this->user = "takcollect";
		$this->passwd = "*****";
		$this->data_base = "games";
			$this->db_handle = mysql_connect($host,$user,$passwd) or die("error connecting to database");
	}

	function dbSelect($db)
	{
		$this->data_base = $db;
		if(!mysql_select_db($this->data_base, $this->db_handle))
		{
			error_log(mysql_error(), 3, "/phplog.err");
			die("Error selecting database");
		}
	}

	function executeSql($sql_stmt)
	{
		$this->sql = $sql_stmt;
		$this->result = mysql_query($this->sql);
	}
	function outputGenerate()
	{
		echo "<table cellspacing='0' cellpadding='0' width='0' border='1'>\n<tr>";
		echo "<tr><td width=\"400\">Number</td><td width=\"100\">Game</td><td width=\"400\">Link</td><td width=\"100\">Console</td></tr><tr>";
		while($record = mysql_fetch_object($this->result))
		{
			echo "<td width='400'>".$record->number."</td>";
			echo "<td width='100'>".$record->game."</td>";
			echo "<td width='400'>".$record->link."</td>";
			echo "<td width='100'>".$record->console."</td>";
			echo "</tr><tr></tr><tr>";
		}
		echo "</tr></table>";
	}
}

$sql = "SELECT * FROM games ORDER BY games";
$dbObject = new database();
$dbObject->dbSelect($db);
$dbObject->executeSql($sql);
$dbObject->outputGenerate();
?>
 
Dyslexic_Dog said:
Classes are used to encapsulate parts of your code. Try to keep everything a class needs inside that class... i.e. Don't pass connection variables to a class that could be kept inside the class. The other option is to set the classes values when you declare the variables.
Click to expand...
Except that defeats the other point of OO programming....reusability. Now he has to edit the class code any time he moves it to a new project, which is the exact opposite of what is wanted.
 
Editing one class doesn't mean you can't reuse it, I'm not saying my example is perfect but you can create set methods to change those variables. My database class is much more robust than this one here, but for the sake of time I just changed the example around a bit to reflect better design.
 
Changing those variables doesn't change the connection setting. Also, there's still no reason to STORE those values in variables as they're only used once. Here's an example class I wrote recently (Python, not PHP) that shows more of what I'm describing (it's incomplete; there aren't all the error checks there need to be, but it's getting there):
Code: 
import MySQLdb

# Error Syntax:
# To see if an error occured, call good()
# To get an error and clear the error flag, call getError()
# Errors are reported as tuples of the form
# (errorNumber, errorMessage)

class database:
    dbhandle = ""
    result = ""
    error = (0,"")

    def __init__(self,h,u,p,dbn):
        try:
            self.dbhandle = MySQLdb.connection(h,u,p,dbn)
        except MySQLdb.Error, e:
            self.error = e

    def __init__(self,h,u,p):
        try:
            self.dbhandle = MySQLdb.connection(h,u,p)
        except MySQLdb.Error, e:
            self.error = e

    def __del__(self):
        try:
            self.dbhandle.close()
        except MySQLdb.Error, e:
            return

    def reconnect(self,h,u,p,dbn):
        self.dbhandle.close()
        self.error = (0,"")
        try:
            self.dbhandle = MySQLdb.connection(h,u,p,dbn)
        except MySQLdb.Error, e:
            self.error = e

    def query(self,querystring):
        if self.good():
            try:
                self.dbhandle.query(querystring)
                self.result = self.dbhandle.store_result()
            except MySQLdb.Error, e:
                self.error = e
        
    def getresult(self):
        if self.good() and (self.result != ""):
            try:
                return self.result.fetch_row(0,1)
            except MySQLdb.Error, e:
                self.error = e
        else:
            return []

    def getrow(self, numrows=1):
        if self.good() and (self.result != ""):
            try:
                return self.result.fetch_row(numrows,1)
            except MySQLdb.Error, e:
                self.error = e
        else:
            return []

    def good(self):
        return self.error == (0,"")

    def getError(self):
        temp = self.error
        self.error = (0,"")
        return temp
Connect once with the DB parameters in the client code where they should be, and life is fine.
 
You must log in or register to reply here.
Back
Top