My first OOP script -- Login

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

wildwobby
Forum Commoner
Posts: 66
Joined: Sat Jul 01, 2006 8:35 pm

My first OOP script -- Login

Post 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();
	}
}
?>
User avatar
neel_basu
Forum Contributor
Posts: 454
Joined: Wed Dec 06, 2006 9:33 am
Location: Picnic Garden, Kolkata, India

Post 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
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post 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.
User avatar
neel_basu
Forum Contributor
Posts: 454
Joined: Wed Dec 06, 2006 9:33 am
Location: Picnic Garden, Kolkata, India

Post 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
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post 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.
User avatar
neel_basu
Forum Contributor
Posts: 454
Joined: Wed Dec 06, 2006 9:33 am
Location: Picnic Garden, Kolkata, India

Post 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
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post 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.
(#10850)
Rovas
Forum Contributor
Posts: 272
Joined: Mon Aug 21, 2006 7:09 am
Location: Romania

Post 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.
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Re: My first OOP script -- Login

Post 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...
User avatar
panic!
Forum Regular
Posts: 516
Joined: Mon Jul 31, 2006 7:59 am
Location: Brighton, UK

Post 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.
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post 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:
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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.
wildwobby
Forum Commoner
Posts: 66
Joined: Sat Jul 01, 2006 8:35 pm

Post 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...
?>
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

I think you've posted the old code again.. Since there are still lots of syntactical errors.
wildwobby
Forum Commoner
Posts: 66
Joined: Sat Jul 01, 2006 8:35 pm

Post 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();
	}
}
?>
Post Reply