OOP Registration Help

[TLaude]

I'm a VB.NET programmer, but trying to get into PHP to broaden my knowledge. I'm doing a simple registration / login site just to get my hands on it.

 

The issue I am having is the registration is not completing. I have a feeling it is the way I am connecting to the database but can't seem to figure out where my error is.

 

Here is what I have.

 

register.php

						$sql = "INSERT INTO users (Username, Password, Email, FullName) VALUES ($username, $password, $email, $name)";

						$this->dbConnect('login') or die("Could not connect to server.");
						$this->dbQuery($sql) or die("Could not query the database.");
						$this->dbClose();

 

functions.php:

 

<?php

include('constants.php');

class database
{
// Each time this class is called, it will connect to the database
function __construct()
{
	$this->dbConnect();
}

// Creates a connection to the database
function dbConnect($dbname)
{
	$this->$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die("Could not connect to server.");
	$this->$dbselect = mysql_select_db($dbname, $conn) or die("Could not connect to the database.");
}

// Closes database connection if a connection is currently set
function dbClose()
{
	if(isset($this->$conn))
	{
		mysql_close($this->$conn);
	}
}

// Query's the databases
function dbQuery($sql)
{
	$result = mysql_query($sql);
	return $result;
}

function dbNumRows($result)
{
	$rows = mysql_num_rows($result);
	return $rows;
}
}


?>

 

^^ $dbhost, $dbuser, $dbpass all come out of the constants.php file

 

constants.php:

 

<?php

define($dbhost, 'localhost');
define($dbuser, 'root');
define($dbpass, '');

?>

 

Anyone able to point me in the direction of my error?

[ignace]

$this->$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die("Could not connect to server.");

 

doesn't work because it doesn't know the values $conn, $dbhost, $dbuser, $dbpass

 

$this->$dbselect = mysql_select_db($dbname, $conn) or die("Could not connect to the database.");

 

should be:

 

$this->$dbselect = mysql_select_db($dbname, $this->$conn) or die("Could not connect to the database.");

 

but again it doesn't know the value of $dbselect and $this->$conn

[TLaude]

$this->$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die("Could not connect to server.");

 

doesn't work because it doesn't know the values $conn, $dbhost, $dbuser, $dbpass

 

 

What am I doing wrong that it is not pulling the values of $dbhost, $dbuser, $dbpass from the constants.php file?

 

$this->$dbselect = mysql_select_db($dbname, $conn) or die("Could not connect to the database.");

 

should be:

 

$this->$dbselect = mysql_select_db($dbname, $this->$conn) or die("Could not connect to the database.");

 

but again it doesn't know the value of $dbselect and $this->$conn

 

Good catch on the $this->$conn. Completely missed it.

 

So.. How can I go about making sure $conn and $dbselect have a value?

 

Like this at the class level?:

var $conn;
var $dbselect;

[ignace]

$this->$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die("Could not connect to server.");

 

doesn't work because it doesn't know the values $conn, $dbhost, $dbuser, $dbpass

 

 

What am I doing wrong that it is not pulling the values of $dbhost, $dbuser, $dbpass from the constants.php file?

 

It doesn't know the variable nor its value because of a differing scope. To get these variables inside of your class you'll need to use the 'global' keyword which I highly discourage. Here's a rewrite of your database class:

 

class Database {
    protected $_connection = null;
    protected $_lastResult = null;
    
    public function __construct($pass, $name = null, $user = 'root', $host = 'localhost') {
        $this->openConnection($pass, $user, $host);
        if (null !== $name) {
            $this->select($name);
        }
    }
    
    public function __destruct() {
        $this->closeConnection();
    }
    
    public function openConnection($pass, $user = 'root', $host = 'localhost') {
        $this->_connection = mysql_connect($host, $user, $pass);
    }
    
    public function closeConnection() {
        if (is_resource($this->_connection)) {
            mysql_close($this->_connection);
            $this->_connection = null;
        }
    }
    
    public function select($name) {
        return mysql_select_db($name);
    }
    
    public function query($query) {
        return $this->_lastResult = mysql_query($query, $this->_connection);
    }
    
    public function getRowCount() {
        return mysql_num_rows($this->_lastResult) or mysql_affected_rows($this->_lastResult);//experimental
    }
}

[attock]

Here is what you can do:

 

Solution 1:

class database
{
      // Each time this class is called, it will connect to the database
      function __construct($dbhost, $dbuser,$dbpass, $dbname)
      {
          // no it knows what $dbhost, $dbuser, $dbpass and $dbname are.
$this->dbConnect($dbhost, $dbuser,$dbpass, $dbname);
      }

      // Creates a connection to the database
      function dbConnect($dbhost, $dbuser,$dbpass, $dbname)
      {
      
        ... as is ...
      }
}

// code.php
then in code create an instance like this:
$sqlConn = new database($dbhost, $dbuser,$dbpass, $dbname); // passing parms to __construct()



Solution 2: not so kool
class database
{
      // Each time this class is called, it will connect to the database
      function __construct()
      {
$this->dbConnect($dbhost, $dbuser,$dbpass, $dbname);
      }

      // Creates a connection to the database
      function dbConnect()
      {
        global $dbhost, $dbuser, $dbpass, $dbname; // now it knows what these are
        ... as is ...
      }
}

// code.php
then in code create an instance like this:
$sqlConn = new database(); 

 

Also, change:

$sql = "INSERT INTO users (Username, Password, Email, FullName) VALUES ($username, $password, $email, $name)";

 

to:

$sql = "INSERT INTO users (Username, Password, Email, FullName) VALUES ('$username', '$password', '$email', '$name')";

 

make sure that you have escaped $username, $password, $email & $name by using mysql_real_escape_string(); e.g. $username = mysql_real_escape_string($username); to prevent mysql injection.

 

[attock]

nevermind, duplicate post.. already explained by ignace:D

[TLaude]

Thanks you two. I was hoping I would be able to use a constants.php so I don't have important values like database connection info scattered. I would like to be able to just change it in 1 place (constants.php) and everything will work.

 

I will use your tips and try to mesh it with mine.

 

Thanks folks!

[ignace]

Thanks you two. I was hoping I would be able to use a constants.php so I don't have important values like database connection info scattered. I would like to be able to just change it in 1 place (constants.php) and everything will work.

 

Then use something like:

 

$config = array();

//Database
$config['database']['host_address'] = 'localhost';
$config['database']['username'] = 'root';
$config['database']['password'] = '';
$config['database']['name'] = '';

//Other Site Configuration Data

 

Then in your code:

 

$dbConfig = $config['database'];
$db = new Database($dbConfig['host_address'], $dbConfig['username'], $dbConfig['password'], $dbConfig['name']);

 

Keep doing this same method throughout your application and you can by adjusting the configuration settings alter your application's behavior.

[jkewlo]

Here you go, THis is my Database class study it. use it if you would like to it takes care of all stripslashes and md5 as well

 

<?

class Database{
  
  // private instant variables
  var $dbConnectionID;
  var $queryID;
  var $record; 
  var $host;
  var $database;
  var $user;
  var $password;
  
  /* 
    constructor
    connect to datbase server and select specified database
  */
  function Database(){
    $Config = new Configuration();

    $this->host 	= $Config->DBHost;
    $this->database 	= $Config->DBName;
    $this->user 	= $Config->DBUser;
    $this->password 	= $Config->DBPass;
    $this->connect();
  }
  
  
  
  /*
    private method
    used internally to generate dbConnectionID
  */
  function connect(){
    $this->dbConnectionID = @mysql_pconnect($this->host, $this->user, $this->password);
    if(!$this->dbConnectionID){
      echo(mysql_errno().":".mysql_error());
      exit;
    }
    else{
      $status = @mysql_select_db($this->database, $this->dbConnectionID);
      if(!$status){
        echo(mysql_errno().":".mysql_error());
        exit;
      }
    } 
  }
  
  
  //  public methods
  
  function query($sql){
    // connect to db incase connection id is not set
    if(empty($this->dbConnectionID))
      $this->connect();
    $this->queryID = @mysql_query($sql, $this->dbConnectionID);
    
    // handle error
    if(!$this->queryID){
      echo(mysql_errno().":".mysql_error());
      exit;
    }
  }  
  
  function nextRecord(){
    $this->record = @mysql_fetch_array($this->queryID);
    $status = is_array($this->record);
    return($status);
  }
  
  function numRows(){
    $rows = @mysql_num_rows($this->queryID);
    return($rows);
  }
  
  // get record field value from the current record pointed by $record
  function getField($field){
    return($this->record[$field]);
  }
}
?>
[\code]

And this is my config.php class
[code]
<?
class Configuration {

//you can add more to here
public $DBName;
public $DBPass;
public $DBUser;
public $DBHost;

function Configuration(){
	//for each variable above, assign it here
	$this->DBName = "**";
	$this->DBUser = "**";
	$this->DBPass = "**";
	$this->DBHost = "localhost";
}

}
?>
[\code]

[ignace]

if(!$this->dbConnectionID){
      echo(mysql_errno().":".mysql_error());
      exit;
    }

 

Never heard of Exception's?

 

  function Database(){
    $Config = new Configuration();

 

Don't use composition use aggregation instead (it allows for easier testing):

 

function Database(Configuration $config)

 

Plus you may want to look at the singleton pattern because if you would use Configuration in many parts of your application you would create a serious overhead.