PHP Class Issue

[keyroche]

Hello, I have a bit of explaining to do so please bare with me. I am developing the backend of a site and it has a log in feature. I have separated all of the functions of the whole back end into classes: clients, users, ads, database, and security.

I am having a problem with the login function in the security class. When I have it run, it gives me the following warning: "Warning: mysql_query() expects parameter 2 to be resource, null given in database.inc.php on line 25" Then when I go to the home page again it does not redirect (as it is supposed to do in the code), so it tells me that the warning is stopping it from logging in.

The lines of code for that are: (in security.inc.php)

class security extends database{
	var $logged = false; // Boolean (ture/false) to tell if user is logged in or not.
	var $userData;

	function __construct(){
		session_start();
		header("Cache-control: private"); // Tells the user's browser to not cache their password.
		// Check to see if the session exists
		if(!isset($_SESSION['userID']) && !isset($_SESSION['username'])){
			if(strpos(curPageURL(),'index.php') != false){ // If it is the homepage, it will give you access, because the HP will just put the login form up.
			}else{
				header("Location: /administration/index.php");
				exit();
			}
		}
	}

function login($user,$pass,$successURL = '', $failureURL = ''){
		$ePass = $this->encryptPass($pass);
		$lUser = strtolower($user);
		$getUser = $this->query("SELECT * FROM users WHERE username = '$lUser' AND password = '$ePass' LIMIT 1") or die(mysql_error());

I think it is the last line in the above code that is the problem.

The relevant code for the database class is: (in database.inc.php)

class database{
	var $link;
	var $host;
	var $user;
	var $pass;
	var $db;
	
	function __construct(){
		$this->host = 'HOST';
		$this->user = 'USERNAME';
		$this->pass = 'PASSWORD';
		$this->db = 'DATABASE';
		
		$this->link = mysql_connect($this->host,$this->user,$this->pass);
		mysql_select_db($this->db);
	}
	
	## Query DB ##
	function query($query){
		return mysql_query($query,$this->link);
	}

Any help would be greatly appreciated!
Thank you in advance.

P.S. If any more information is needed, please ask.

[etftw]

I may be wrong, but I don't think you can store the link as the connection will be terminated at the end of the script. I'd imagine you'd have to reinitialise the connection every time you wish to use it.

[keyroche]

I am changing the code for the structure of the functional parts of the backend. It was all coded before and working, but then I changed it a bit to make it easier. before the code for the database was:

## Connect to DB ##
		$this->link = mysql_connect($host,$user,$pass);
		mysql_select_db($db);
		register_shutdown_function(array(&$this, 'close'));

and it worked fine. The only difference was the register_shutdown_function() if that prevents what you were talking about.

[quasipickle]

You can store the link like that & use it the entire time your script is running. You're correct though - unless you're using persistent connections, the database connection will be terminated.

What I don't understand is why PHP thinks $this->link is NULL. It should either be a valid resource or boolean FALSE (the 2 possible return values from mysql_connect().

Try calling var_dump() on $this->link immediately after your call to mysql_connect(). It's possible what's being returned isn't what you expect.

Also, since you've nicely put everything in a database abstraction class, you might want to consider upgrading to the mysqli*() functions rather than the mysql() functions. The mysqli library is much better written, so it's a little more stable & useful.

[etftw]

What I don't understand is why PHP thinks $this->link is NULL. It should either be a valid resource or boolean FALSE (the 2 possible return values from mysql_connect().

A while ago when doing some OOP in PHP I found that PHP would always call my destructor at the end of the script - do you have a destructor in these classes anywhere?

[quasipickle]

The "end of the script" is the end of PHP execution for a particular page request, not the end of a particular file. The destructor wouldn't be called until all PHP in all involved files is completed. So, I a destructor wouldn't be responsible for the link being NULL.

[etftw]

When I say the end of the script I was referring to when execution had finished as the test was all conducted in one file.

[keyroche]

I just used the following code in a test file and it worked fine, so it has to do with the way I am calling it in the class.

include('includes/globals.inc.php');
include('includes/database.inc.php');
include('includes/security.inc.php');

$db = new database();
$query = $db->query("SELECT * FROM users");
while($row = $db->fetchArray($query)){
	echo $row['username'];
}

I do not have a destructor and when I return the link in the class it is returned to NULL, but apparently it worked in the code above.

[keyroche]

I figured out why the link is being returned to NULL. It is because I am not calling the class and the link is initiated in the constructor. Since I made security an extension of database, I was just using $this-> to access database functions. I changed it to make a new database object:

$db = new database();
		$ePass = $this->encryptPass($pass);
		$lUser = strtolower($user);
		$getUser = $db->query("SELECT * FROM users WHERE username = '$lUser' AND password = '$ePass' LIMIT 1") or die(mysql_error());

So now it works. Is there a better way to do what I want? I would prefer to not have to make a new object for database in every function in security that needs it. Is there a way I can create one for the whole class? I tried putting it as an instance variable like:

class security{
	var $logged = false; // Boolean (true/false) to tell if user is logged in or not.
	var $userData;
	var $db = new database();

but I got an error: "Parse error: syntax error, unexpected T_NEW in includes/security.inc.php on line 8"

Thank you for your help. :)

@php.net

Note: Parent constructors are not called implicitly if the child class defines a constructor. In order to run a parent constructor, a call to parent::__construct() within the child constructor is required.

It's best to put parent::__construct() inside the child's constructor, but I think it's not the case here. So:

function login($user,$pass,$successURL = '', $failureURL = ''){
        $ePass = $this->encryptPass($pass);
        $lUser = strtolower($user);

        parent::__construct();

        $getUser = $this->query("SELECT * FROM users WHERE username = '$lUser' AND password = '$ePass' LIMIT 1") or die(mysql_error());

Also, append your properties and methods with public, private or protected (although this will add up to the confusion :P ).

Also, you should consider OOP design patterns (especially the "singleton" pattern for Db connections). Here's some links.

[keyroche]

Okay that worked and it makes sense. Thank you. Is there any way to do it on a class level, instead of a function by function? As for the public, private and protected- thank you for the advice. I will do that, I don't because I tried to and it did not work, but now I see I was doing it wrong. lol. I was still including the var part when putting public or private.

Is there any way to do it on a class level, instead of a function by function?

Call the parent constructor in the child's constructor. Careful with that session_start().
Read Example #1.

[keyroche]

Ah, again - that makes sense and it worked. Thank you!

[mschroeder]

-nevvermind
Singletons are almost always an indication of poor code design. They're extremely difficult to mock and test. Essentially the problem with singletons is you place the responsibility for determining the objects life cycle in the wrong place.

More beneficial to have a factory class that handles initialization and returns a single instance of a particular class if necessary.

-kroche
Looking at your code it looks like your domain objects are all extending the database class to provide database access. This will also become troublesome if you ever have to test your code independently. Something to consider would be to make your database class external and keep your domain objects isolated. If a domain object needs database object, then pass the database instance into the domain object when created. This opens the doors for utilizing the factory pattern and/or dependency injection to create domain objects and handle initializing any dependencies a domain object may have.

[keyroche]

Looking at your code it looks like your domain objects are all extending the database class to provide database access. This will also become troublesome if you ever have to test your code independently. Something to consider would be to make your database class external and keep your domain objects isolated. If a domain object needs database object, then pass the database instance into the domain object when created. This opens the doors for utilizing the factory pattern and/or dependency injection to create domain objects and handle initializing any dependencies a domain object may have.

So your saying that it would be better to make a separate object of the database class for each function that needs it, instead of extending off of the database class?

P.S. I marked the thread as solved, since my main issue has been.