Page 1 of 2

My first OOP script -- Login

Posted: Tue Dec 26, 2006 11:13 pm
by wildwobby
Well, I made my first OOP thing. It's not 100% finished but I want to make sure I'm even doing it right. All I've done so far is make the classes.

Heres what I've got.

Code: Select all

<?php
session_start();
Class Mysql {
	var link;
	var database;

	function Mysql($db,$dbuser,$dbpass) {
		$this->link = mysql_connect('localhost', $dbuser, $dbpass) or die(mysql_error());
		$this->database = mysql_select_db($db, $link);
		if (!$this->database) {
			die('COULD NOT CONNECT: ' . mysql_error());
		}
	}
}
		 
class Auth {
	var $quesr;
	var $qpass;
	var $query;
	var $result;
	var $location;

	function login($user,$pass) {
		$this->quser = addslashes($user);
		$this->qpass = addslashes($pass);
		$this->query = sprintf("SELECT COUNT(*) FROM users WHERE username='%s' AND password='%s'", mysql_real_escape_string($this->quser), mysql_real_escape_string($this->qpass));
		$this->result = mysql_query($this->result);
		if (!$this->result) {
			die('QUERY FAILED: ' . mysql_error());
		}
		if ($this->result != 1) {
			$errmsg = "No match found";
// Set location to login script because failed login.
			header('Location: http://rwphoto.thepeopleshost.com/login.php/');
			exit;
		} else {
			$_SESSION['auth'] = 1;
			$_SESSION['authx'] = (time() + 900);
// Set Fowarded location if login passes
			header('Location: http://rwphoto.thepeopleshost.com/members.php/');
			exit;
		}
	}

	function checkLogin() {
		if ($_SESSION['auth'] != 1) {
			$errmsg = "Authentication required to view the requested page.";
// Set location to login script if authentication to login-required page fails
			header('Location: http://www.rwphoto.thepeopleshost.com/login.php/');
			exit;
		}
		if ($time() > $_SESSION['authx']) {
			$errmsg = "You're login has timed-out, please login again.";
//Login timeout, must re-login. Set redirect location
			header('Location: http://www.rwphoto.thepeopleshost.com/login.php/');
			exit;
		}
		$_SESSION['authx'] = (time() + 900);
	}

	function logout() {
		session_destroy();
	}
}
?>

Posted: Tue Dec 26, 2006 11:30 pm
by neel_basu
Its php Not JavaScript Put a $ Sign At Begining Of Every Variable Name e.g. $link,
$database
var Is Not Required For Variable Declearation Putting A $ sign Would Make it a Variable

Posted: Tue Dec 26, 2006 11:36 pm
by John Cartwright
neel_basu wrote:Its php Not JavaScript Put a $ Sign At Begining Of Every Variable Name e.g. $link,
$database
var Is Not Required For Variable Declearation Putting A $ sign Would Make it a Variable
You are mistaken. In classes, it is common practice to declare your variables. These variables are used within the class scope, and are not the same as regular variables. I would suggest you read http://php.net/oop.

As for the OP, how exactly are you initializing your classes? Before giving you my full blown critique, I would like to see this.

Posted: Tue Dec 26, 2006 11:48 pm
by neel_basu
Jcart wrote:
neel_basu wrote:Its php Not JavaScript Put a $ Sign At Begining Of Every Variable Name e.g. $link,
$database
var Is Not Required For Variable Declearation Putting A $ sign Would Make it a Variable
You are mistaken. In classes, it is common practice to declare your variables. These variables are used within the class scope, and are not the same as regular variables. I would suggest you read http://php.net/oop.

As for the OP, how exactly are you initializing your classes? Before giving you my full blown critique, I would like to see this.
One Must Put A $ Sign Before Your Variable Name
=====================================
This Would Not Work
=======================================

Code: Select all

<?php
class tst
{
   var vr = "hi";
}
$gtc = new tst();
echo $gtc->vr;
?>
Ya Var Is required in class i didn't looked at his code closely i thought classes has ended

Posted: Wed Dec 27, 2006 12:18 am
by John Cartwright
After reading your comment I looked back to the auth class and noticed there were $ signs, didn't bother to look at the other one. I stand corrected.

Posted: Wed Dec 27, 2006 12:23 am
by neel_basu
wildwobby wrote:Well, I made my first OOP thing. It's not 100% finished but I want to make sure I'm even doing it right. All I've done so far is make the classes.

Heres what I've got.

Code: Select all

<?php
session_start();
Class Mysql {
	var link;
	var database;
................................................................
...................................................
Look At The link And The database

Posted: Wed Dec 27, 2006 12:37 am
by Christopher
Besides the syntax problems I think the real issue is that this is not OO design. You have simply taken four functions and wrapped them into class constructs. The Mysql class should probably be a real DB class that can be used for queries and result sets. The Auth classes should probably be split up as well.

Posted: Wed Dec 27, 2006 4:09 am
by Rovas
Besides the syntax problems I think the real issue is that this is not OO design.
I agree you should create 3 classes one with the connection to the DB (because when you change your db or change to different engine of db example from MySQL to SQL SERVER), the query class and the query result class in keeping OOP principle that says : "Each class has to have one and only one objective to fulfill." .
In the real world this can be achieved all the time but you should 'overload' your classes with different methods (functions) that don' t reffer directly to the functions of the object.

Re: My first OOP script -- Login

Posted: Wed Dec 27, 2006 4:21 am
by timvw
wildwobby wrote:

Code: Select all

var link;
This is invalid syntax.
wildwobby wrote:

Code: Select all

function Mysql($db,$dbuser,$dbpass) {
 $this->link = mysql_connect('localhost', $dbuser, $dbpass) or die(mysql_error());
 $this->database = mysql_select_db($db, $link);
You are using $link, and undefined variable.
wildwobby wrote:

Code: Select all

function login($user,$pass) {
		$this->quser = addslashes($user);
		$this->qpass = addslashes($pass);

		$this->query = sprintf("SELECT COUNT(*) FROM users WHERE username='%s' AND password='%s'", mysql_real_escape_string($this->quser), mysql_real_escape_string($this->qpass));
Why are you adding slashes to $user and $pass (mysql_real_escape_string is what you need for a mysql query).
wildwobby wrote:

Code: Select all

$this->result = mysql_query($this->result);
if ($this->result != 1) {
$errmsg = "No match found";
What are you trying to do here? I think you want to use a mysql_fetch_* function to access the resultset data...
wildwobby wrote:

Code: Select all

$_SESSION['auth'] = 1;
$_SESSION['authx'] = (time() + 900);
// Set Fowarded location if login passes
header('Location: http://rwphoto.thepeopleshost.com/members.php/');
exit;
You probably want to call session_write_close before the header('Location' call to ensure the session data is persisted...
wildwobby wrote:

Code: Select all

if ($time() > $_SESSION['authx'])
Where does $time come from?


Anyway, if you just ran your code with ini_set('error_reporting', E_ALL); ini_set('display_errors', TRUE); you would already get warnings and notices about syntax issues...

Posted: Wed Dec 27, 2006 5:08 am
by panic!
Just came back to this forum for a browse and remember I don't come here because it's full of bickering children.

Posted: Wed Dec 27, 2006 10:30 am
by Luke
panic! wrote:Just came back to this forum for a browse and remember I don't come here because it's full of bickering children.
please, if you don't have anything of value to add to the thread, please don't post in it. If you have a problem with the forum start a new thread about it or contact a mod. :roll:

Posted: Wed Dec 27, 2006 11:48 am
by RobertGonzalez
panic! wrote:Just came back to this forum for a browse and remember I don't come here because it's full of bickering children.
And this post of yours has done what to add value to the original posters thread? Yeah, thanks for your input.

Problems that you may have with our community should be addressed to a Moderator or Administrator through PM. I'll thank you for complying with that protocol.

Posted: Wed Dec 27, 2006 12:48 pm
by wildwobby
Ok, thanks for replies!

I've tried to change some things, but I don't understand what you guys mean by splitting the classes up to make it true oop.

Anyways heres what I've got now...

loginclass.php

Code: Select all

<?php
session_start();
Class Mysql {
	var link;
	var database;

	function Mysql($db,$dbuser,$dbpass) {
		$this->link = mysql_connect('localhost', $dbuser, $dbpass) or die(mysql_error());
		$this->database = mysql_select_db($db, $link);
		if (!$this->database) {
			die('COULD NOT CONNECT: ' . mysql_error());
		}
	}
}
		 
class Auth {
	var $quesr;
	var $qpass;
	var $query;
	var $result;
	var $location;

	function login($user,$pass) {
		$this->quser = addslashes($user);
		$this->qpass = addslashes($pass);
		$this->query = sprintf("SELECT COUNT(*) FROM users WHERE username='%s' AND password='%s'", mysql_real_escape_string($this->quser), mysql_real_escape_string($this->qpass));
		$this->result = mysql_query($this->result);
		if (!$this->result) {
			die('QUERY FAILED: ' . mysql_error());
		}
		if ($this->result != 1) {
			$errmsg = "No match found";
// Set location to login script because failed login.
			header('Location: http://rwphoto.thepeopleshost.com/login.php/');
			exit;
		} else {
			$_SESSION['auth'] = 1;
			$_SESSION['authx'] = (time() + 900);
// Set Fowarded location if login passes
			header('Location: http://rwphoto.thepeopleshost.com/members.php/');
			exit;
		}
	}

	function checkLogin() {
		if ($_SESSION['auth'] != 1) {
			$errmsg = "Authentication required to view the requested page.";
// Set location to login script if authentication to login-required page fails
			header('Location: http://www.rwphoto.thepeopleshost.com/login.php/');
			exit;
		}
		if ($time() > $_SESSION['authx']) {
			$errmsg = "You're login has timed-out, please login again.";
//Login timeout, must re-login. Set redirect location
			header('Location: http://www.rwphoto.thepeopleshost.com/login.php/');
			exit;
		}
		$_SESSION['authx'] = (time() + 900);
	}

	function logout() {
		session_destroy();
	}
}
?>
login.php

Code: Select all

<?php
session_start();
?>
// form with action to self...
if (isset($_POST['userid']) && isset($_POST['passwrd'])) {
include '../loginclass.php/';
$db = new Mysql('users', '*****', '*****');
$auth = new Auth;
$user = $_POST['userid'];
$pass = $_POST['passwrd'];
$auth->login($user, $pass);
}
?>
members.php

Code: Select all

<?php
session_start();
include '../loginclass.php/';
$auth = new Auth;
$auth->checkLogin();
//page content...
?>

Posted: Wed Dec 27, 2006 1:04 pm
by timvw
I think you've posted the old code again.. Since there are still lots of syntactical errors.

Posted: Wed Dec 27, 2006 4:57 pm
by wildwobby
oops sorry...

here it is:

Code: Select all

<?php
session_start();
Class Mysql {
	var $link;
	var $database;

	function Mysql($db,$dbuser,$dbpass) {
		$this->link = mysql_connect('localhost', $dbuser, $dbpass) or die(mysql_error());
		$this->database = mysql_select_db($db, $link);
		if (!$this->database) {
			die('COULD NOT CONNECT: ' . mysql_error());
		}
	}
}
		 
class Auth {
	var $quesr;
	var $qpass;
	var $query;
	var $result;
	var $location;

	function login($user,$pass) {
		$this->quser = $user;
		$this->qpass = $pass;
		$this->query = sprintf("SELECT COUNT(*) FROM users WHERE username='%s' AND 

password='%s'", mysql_real_escape_string($this->quser), mysql_real_escape_string($this->qpass));
		$this->result = mysql_query($this->query);
		if (!$this->result) {
			die('QUERY FAILED: ' . mysql_error());
		}
		if ($this->result != 1) {
			$errmsg = "No match found";
// Set location to login script because failed login.
			header('Location: http://rwphoto.thepeopleshost.com/login.php/');
			exit;
		} else {
			$_SESSION['auth'] = 1;
			$_SESSION['authx'] = (time() + 900);
// Set Fowarded location if login passes
			session_write_close()
			header('Location: http://rwphoto.thepeopleshost.com/members.php/');
			exit;
		}
	}

	function checkLogin() {
		if ($_SESSION['auth'] != 1) {
			$errmsg = "Authentication required to view the requested page.";
// Set location to login script if authentication to login-required page fails
			header('Location: http://www.rwphoto.thepeopleshost.com/login.php/');
			exit;
		}
		if (time() > $_SESSION['authx']) {
			$errmsg = "You're login has timed-out, please login again.";
//Login timeout, must re-login. Set redirect location
			header('Location: http://www.rwphoto.thepeopleshost.com/login.php/');
			exit;
		}
		$_SESSION['authx'] = (time() + 900);
	}

	function logout() {
		session_destroy();
	}
}
?>