Is this the right format for a User class?

[EatRamen]

Hiya! Basically I'm wondering if this is the right way to have a user class.

 

<?php
class User
{
var $id;
var $username;
var $password;
var $email;
var $avatar;
var $userinfo;
var $logged_in;

function __construct()
{
	global $database;
	$this->userinfo = $database->fetcharray("SELECT * FROM users WHERE username ='$_COOKIE['username']' AND password ='$_COOKIE['password']'");
	$this->checkLogin();
	if ($this->logged_in)
	{
		$this->id = $this->userinfo[id];
		$this->username = $this->userinfo[username];
		$this->password = $this->userinfo[password];
		$this->email = $this->userinfo[email];
		$this-avatar = $this->userinfo[avatar];
	}
}
function getUserInfo($username)
{
	global $database;
	return $database->fetcharray("SELECT * FROM users WHERE username ='$username'");
}
function checkLogin()
{
	if ($this->userinfo[username] == '')
	{
		return FALSE;
	}
	else
	{
		return TRUE;
	}
}
}
?>

 

I haven't tested it.. And it's not fancy, but it's a simple example that I just coded right now to show you how I usually do mine. Do you think you could show me your ways? And anything else I should include it it.

 

Also-- I'm wondering how my database class and global file should look. Also how it should be set up, etc. I just really want to make sure that I'm doing what I'm supposed to do! =D

 

Thanks,

-Ramen

[Isityou]

Heres one of my old user class from a previous Mode-View-Controller framework. Its outdated and unsafe. I also hope you sanitize all you MySQL input or for that matter all places you expect user input, as you will be vulnerable to SQL injection and other security issues.

 

<?php
// Make sure everything is executed from within the framework
if(!defined('IN_FRAMEWORK')) die ("No direct access!");

class user extends Module
{
public $moduleName = "user";
  
public $templateFile = NULL;

public $pageTemplateFile = NULL;

public function __construct()
{
	global $vars;
	global $DIRECTORY;

	$this->set('DIRECTORY', $DIRECTORY);

	// Must be setS
	$this->set('moduleName', $this->moduleName);

	$this->pageTemplateFile = NULL;

	if($vars[1] == "")
	{
		$this->templateFile = FRAME_MODULE_PATH."/user/template/".'login'.".tpl";
	}
	else
	{
		$this->templateFile = FRAME_MODULE_PATH."/user/template/".$vars[1].".tpl";
	}

	// If a method exists, execute it
	if(method_exists('user',$vars[1]))
	{
		// Execute the function
		$this->$vars[1]();
	}
}

public function login()
{	
	if(isset($_POST['submitted']))
	{
		$error = array();

		$username  = mysql_real_escape_string($_POST['username']);
		$password  = mysql_real_escape_string($_POST['password']);
		$password2 = mysql_real_escape_string($_POST['password2']);

		if($username == "")
		{
			$error[] = "You must enter a username!";
		}

		if($password == "")
		{
			$error[] = "You must enter a password!";
		}

		if($error)
		{

		}
		else
		{
			global $DB;
			$sql = "SELECT * 
					FROM users
					WHERE users_username = '$username '
					AND users_password = '$password'";
			$result = $DB->query($sql);

			if($row = mysql_fetch_array($result))
			{
				global $Session;
				$Session->register('USER_ID', $username);

				// Store innformation into the database
				$IP   = $_SERVER['REMOTE_ADDR'];
				$Host = gethostbyaddr($IP);
				$screen_res = $HTTP_COOKIE_VARS["users_resolution"];

				header('Location: /fsi/content/admin');
			}
			else
			{
				$error[] = "Invalid username/password!";
			}
		}

		$errors = implode("<br />", $error);
		$this->set('error', $errors);

		$this->templateFile = FRAME_MODULE_PATH . "/$this->moduleName/template/login.tpl";
	}
	else
	{
		$this->templateFile = FRAME_MODULE_PATH . "/$this->moduleName/template/login.tpl";
	}
}

public function logout()
{	
	unset($_SESSION['USER_ID']);
	session_destroy();
	$error[] = "You have been logged out!";
}

public function _default()
{	
	$this->templateFile = FRAME_MODULE_PATH . "/$this->moduleName/template/user.tpl";
}
}
?>

[aschk]

I would like to denounce the use of "global" as it's particularly unsafe and subject to being overwritten, and should be replaced with the use of Singletons or a registry object (that contains your database object).

 

e.g.

<?php
class database {

static private $instance = null;
private $conn;
private $res;

private function __construct(){

}

static public function getInstance(){
	if(is_null(self::$instance)){
		self::$instance = new self();
	}
	return self::$instance;
}

public function connect($hostname,$username,$password,$database = null){
	$this->conn = mysql_connect($hostname,$username,$password);
	if($database){
		mysql_select_db($database,$this->conn);
	}
}

public function query($sql){
	$this->res = mysql_query($sql,$this->conn);
}

public function fetchAssoc(){
	if($this->res){
		return mysql_fetch_assoc($this->res);
	} else {
		throw new Exception("MySQL Result is not valid.");
	}
}

}


$db = database::getInstance();
$db->connect("localhost","james","password","test");
$db->query("SELECT * FROM domains LIMIT 0,10");
while($row = $db->fetchAssoc()){
echo $row['name'];
}
?>

 

I would like it noted that this is something i have drawn up in 5 minutes only and therefore do not condone it's usage, but hopefully you see how it works as a singleton. i.e. you can't create a "new database()" and will always re-use the same object (so hopefully it'll maintain connection), but keeping it encapsulated as an object stops the naughty use of "global" which can be overwritten by anything else.

[aschk]

I think perhaps I should provide an example of this in use of the context of your class. Thus:

 

<?php
class User
{
private $id;
private $username;
private $password;
private $email;
private $avatar;

public function __construct($username,$password)
{
	$this->username = $username;
	$this->password = $password;
}

public function auth(){
	// Get the database connection.
	$db = database::getInstance();

	$sql = "SELECT id,email,avatar FROM users WHERE username ='{$this->username}' AND password ='{$this->password}'";
	$db->query($sql);
	if($db->numRows === 1){
		// Get the database association info.
		$info = $db->fetchAssoc();

		// Set the internal variables.
		$this->id = $info['id'];
		$this->email = $info['email'];
		$this->avatar = $info['avatar'];
		return true;
	}
	return false;
}
}
?>

 

Curiously why are you using COOKIES? Use session variables if anything.

Also, you might want to consider "creating" the user before you actually authenticate them, just in case you want to register them as a guest or insert them into the database later.

 

example usage:

<?php
$user = new User($_COOKIE['username'],$_COOKIE['password']);
if($user->auth()){
  echo "Hurray, our user authenticated and now has populated their own data";
}
?>

[EatRamen]

Thanks! You have no idea how much you have just helped me! =D

 

So should I put all my other functions that pertain to the User in my User class as well? Such as logging him/her in, logging out, displaying avatar? And even functions that just help me with the User (Like displaying avatar could display someone elses avatar that is not my User).

 

Sorry for asking so many questions! This is my first /real/ OOP website.

[EatRamen]

Oh and also, how should I set up the files? Should I have all of my classes in one global file, or should I have them in a seperate file from each other and include them when I need to? But I would need to use the User class on every page so how would that work?

 

I think I've heard of auto-includer thingys but I wouldn't know where to start with that!

[trq]

So should I put all my other functions that pertain to the User in my User class as well? Such as logging him/her in, logging out, displaying avatar?

 

Logging in and out sounds more like it should be in some sort of authentication class rather than any user class. try and keep your classes as specific as possible.

 

I think I've heard of auto-includer thingys but I wouldn't know where to start with that!

 

The manual is as good a place as any.

[aschk]

I've found the PEAR naming convention to be VERY useful.

e.g.

 

for a class called Registry_Session, __autoload would translate this to Registry/Session.php

<?php
function __autoload($class)
{

$fileWords = explode('_', $class);
foreach ($fileWords as $key => $word) {
	$fileWords[$key] = ucfirst($word);

}
$file = implode('/', $fileWords) . '.php';

if(!require_once($file)){
	// We failed to include this file.
}
}
?>

 

So to reiterate: the class is called Registry_Session

<?php
class Registry_Session {
  public $var = "blah";
}
?>

but the file is in the directory Registry and is called Session.php

 

If you use this directory/file structure through your application it allows you to drop in classes into the folder structure (as long as they're named correctly etc) and NEVER have to include them, as they'll be automagically included.

 

There is only 1 downside to this. You might end up with really long class names.

e.g. Registry_Session_Specific_Another_Blah, which means your code will look like:

<?php
$test = new Registry_Session_Specific_Another_Blah();
?>

 

Still unless you're extending your classes unnecessarily you shouldn't end up with this happening that often.

[EatRamen]

Okay, great! Thanks!

 

So I tried a user class again and how does this look:

 

<?php
class User
{
var $id;
var $username;
var $password;

function __construct($username, $password)
{
	$this->username = $username;
	$this->password = $password;
}

function authUser()
{
	global $database;

	$sql = "SELECT * FROM users WHERE username ='$this->username' AND password ='$this->password'";
	$info = $database->fetchArray($sql);

	if ($database->numRows($sql) == 1)
	{
		$this->id = $info['id'];

		//Set the other stuff here as well
	}
	else
	{
		return false;
	}
}
}

$user = new User($_SESSION['username'], $_SESSION['password']);

if ($user->authUser())
{
echo "You are logged in!";
}
else
{
echo "You are NOT logged in!";
}
?>

[EatRamen]

And after you tell me how the User class looks, let me know how the Database is too! (Please)

 

<?php
class Database
{
var $db_host;
var $db_user;
var $db_pass;
var $db_name;

function __construct()
{
	$this->connect("******", "******", "******");
	$this->select_db("******");
}

function connect($db_host, $db_user, $db_pass)
{
	$this->db_host = $db_host;
	$this->db_user = $db_user;
	$this->db_pass = $db_pass;

	@mysql_connect($this->db_host, $this->db_user, $this->db_pass)
	or die("Failed to connect to MySQL!");
}

function select_db($db_name)
{
	$this->db_name = $db_name;

	@mysql_select_db($this->db_name);
}

function query($sql)
{
	return @mysql_query($sql) or die(mysql_error());
}

function fetchArray($sql)
{
	return mysql_fetch_array(@mysql_query($sql)) or die(mysql_error());
}

function numRows($sql)
{
	return mysql_num_rows(@mysql_query($sql)) or die(mysql_error());
}
}

$database = new Database;
?>

 

Another question: When I use the autoload function, do I also put the "$database = new Database;" in the Database.php file? Or what? And where /do/ I put the __autoload function anyways? Like, in a global file?

[aschk]

Usually I keep the classes separate from any implementation code (i.e. controllers).

So DON'T do any intialisation of classes inside the class. (i.e. $database = new Database() );

e.g. User.php is

<?php
class User { // stuff here }
?>

ONLY

 

Feel free to write a page (mypage.php) that uses this class however. I won't got into front controllers at this point, but my preferred technique is to use index.php to control ALL the flow of the application, and thus it is the ONLY page that does the include for autoload.php or even have the __autoload function in it.

 

In other words: put the autoload.php at the base of your application (htdocs), and include it in ALL your working files.

[EatRamen]

Okay.. Does anyone have a good set of a User System (with login, logout, register and then maybe something extra like a forum) in OOP laying around that I could possibly see? I really want to see how everything goes together, but it's hard to do that when I don't really have a clue. It would just be for learning purposes.

[deadimp]

Find a CMS or a framework and see how they do it.

 

I have one in Thacmus: /db/user.php

However, I haven't released a new version in a few months (but I plan to in a week or so). The user system I have is kind of weak, not exactly in security, but in structure. Also, the stylesheets are broken (will be fixed within the new release).

In the newer version, there's a little more cohesion to manage permissions and all that better, especially with the basic implementation of user groups and such.

[aschk]

Interstingly you've done something I used to do (deadimp).

You have made the user class EXTEND the dbi class. This is my opinion (and it is just my opinion) is not quite correct. Especially semantically...

If you think about it logically a user is NOT a sub-class of a database, which is what you have implied with your design. As a suggestion I would say that a user interacts with a database (i.e. uses a database object), but is not a child of a database.

 

I'll have a browse round some more classes as I like what you're doing

[deadimp]

[shames himself for going off topic]

Yeah, that whole static/instance hybrid design isn't the best implementation known to man, but it is the best implementation I've been able to think of.

However, I figured that since certain classes have raw data that is retrieved from the database and put right into the class, I decided to skip a few steps and had the database directly interface to the class.

My [kinda messed up] thinking on that: DBI -> DataBase Instance -> instance that interacts with the database.

 

I may be wrong, but I was looking through RoR and I think I saw kind of the same thing, except that the class declared it's database variables using field information rather than having it defined statically as I do.... I don't know because I haven't played around with RoR - which I need to. Of course, RoR is MVC, and mine is... well, nothing really standard (as I know).

 

I'll have a browse round some more classes as I like what you're doing

I feel the next version is a hell-lot better organized than the current one (though it's still kind of a mess) so it might be a little easier to browse, too. Glad to see someone interested in it!

[blackcell]

Please tell me that this stuff isn't an easy thing to grasp (mentally).

I have been glancing at it off and on for the last month (6 months of php programming for me) and it still dumbfounds me.  I haven't had a need to implement a class/framework/OOP is any of my applications but I want to know how to do this crap for the sake of satisfying curiosity and I can't find any material that will break through to me.

[aschk]

You're going to love this then.

http://www.onlamp.com/pub/a/php/2005/09/15/mvc_intro.html - published back in 2005 so it's not that old.

Hope it gets you started. It certainly got me on the road. Make sure to read the comments about it at the end, and also check out

http://www.patternsforphp.com/

for information on implementing patterns in PHP.

 

I found that after looking heavily at that first article, implementing it, and then reading stuff on patterns (over and over) I eventually started to get to grips with OOP.

 

note: with the first article make sure to read the follow-up ones too

[blackcell]

Hey thanks man.

[aschk]

No problemo

 

Going back to the original post, I would perhaps agree with thorpe in that the user class might not necessarily implement the auth() function and instead there could be an Auth class that has a validateUser(User $user) function.

e.g.

 

<?php
class Auth {
  public function validateUser(User $user){
    // Get the username
    $username = $user->getUsername();
    $password = $user->getPassword();

    // Perform lookup in DB (missing code here)...

    if(<lookup was successful>){
      return true;
    }
    return false; // Occurs if lookup failed.
  }
}

 

note very carefully that i'm using typecasting in the function, meaning that it requires a User object type.