first time using multiple instances of an object

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

User avatar
TheMoose
Forum Contributor
Posts: 351
Joined: Tue May 23, 2006 10:42 am

Post by TheMoose »

It depends on your project and where you want to go with it.

If you don't have to come back to the users later on, then there's no reason to create a new user and store them if you only need to act on them once during the processing of the page. Because your code could easily become:

Code: Select all

foreach($user_ids as $id){
    $user[$id] = new user($id);
    $user[$id]->some_method();
    unset($user[$id]);
}
Or if you wanted to really save processing memory, do:

Code: Select all

foreach($user_ids as $id){
    userClass::some_method($id);
}
And all you would have to do is change the userClass method 'some_method' to accept the user id as an argument instead of reading it from the class's member variable. This turns it into a class function instead of an object function.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

The Ninja Space Goat wrote:is there any reason why I should or shouldn't do that?
There is no problem technially at all doing that. It is done all the time.

The reason why you perhaps shouldn't do that is because you are simply programming procedurally using classes to namespace functions. Nothing technically wrong with that, but better ways to solve these problems have been identified. While it is probably a step in you programming evolution -- don't stay there long. ;)

You may want to take a look at the Iterator and Registry patterns that do similar array-like things in PHP.
(#10850)
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

Where may i read about Iterator and Registry patterns?
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

I think you're at that stage where you should spend some time with a patterns book... You're getting the idea of OOP, but you need to see the best ways of getting some things done - and a lot of them are heavily described as "Patterns".

The Registry in its simplest form is just a method for storing Objects. Once stored, you can then fetch them from within other Objects. See: http://www.phppatterns.com/docs/design/the_registry

It seeks to solve the dependency problem. If an object depends on one or more other objects, how do you pass them into the object which needs them? You can

a) make them global (but globals are evil)
b) pass them as constructor parameters (but then you could have a very long parameter list)
c) make them singletons (can be as problematic as globals)
d) package the dependencies in a Registry (instead of many parameters, use this as one parameter - or make it a Singleton itself).

One alternative to a Registry I've been implementing in PHP is a ServiceLocator - it too stores objects, but in addition it also locates, includes and instantiates the classes automatically. Requires a bit of convention (predictable naming and such) but very useful.

The Iterator pattern is a simple way of iterating through a result set or similar. http://www.phppatterns.com/docs/design/ ... or_pattern
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

thank you maugrim... it's extremely late, but I plan on reading up on that stuff in the morning. (I really do appreciate that)

Here's what I've got so far:

Code: Select all

class user_admin{
	var $dbh;				// Database object passed by reference
	var $users;				// Array of users (objects)
	var $error;				// if there has been an error, this gets set to true
	var $messages;			// stores all messages created in this class
	var $priv_names;		// stores names of all privileges
	
	function user_admin(&$dbh){
		$this->dbh = $dbh;
		$this->error = false;
		$this->messages = array();
		$this->users = array();
	}
	function load_priv_names(){
		$sql = "SHOW COLUMNS FROM `privileges`";
		if($r = $this->dbh->select($sql)){
			while($row = $this->dbh->get_row($r)){
				if($row['Field'] != 'user_id'){
					$this->priv_names[] = $row['Field'];
				}
			}
		}
		
	}
	function populate_users(){
		$sql = "SELECT id FROM `users`";
		if($r = $this->dbh->select($sql)){
			while($row = $this->dbh->get_row($r, "MYSQL_ASSOC")){
				$id = $row['id'];
				$this->users[$id] = new user($this->dbh, $id);
				$this->users[$id]->load_values();
			}
		}
		else{
			$this->error = true;
			$this->messages[] = "Could not populate users from the database";
		}
	}
	function unpopulate_users(){
		$this->users = array();
		unset($this->users);
	}
}

class user{
	var $dbh;			// A database class reference (to work with the database class below
	
	var $user_name;		// No need for explanation
	var $pass_word;		// No need for explanation
	var $user_id;		// User's id number
	var $user_dir;		// User's directory
	
	var $privs;
	
	var $verified;		// gets set to true upon successful login
	var $error;			// if there has been an error, this gets set to true
	var $messages;		// stores all messages created in this class
	function user(&$dbh, $id=false){
		$this->dbh = $dbh;
		$this->dbh->connect();
		$this->user_id = $id;
		$this->user_dir = "files/";
		$this->privs = array();
		$this->verified = false;
		$this->error = false;
		$this->messages = array();
	}
	function login($user, $pass){
		// This method accepts a user name and password, tests them against all username/password pairs in the 'users' table, and sets the verified property to true.
		// If the username and/or password are incorrect, the verified property is not changed to true, and an error message is added to the property "messages"
		// Upon successful login, user_level, and user_dir are both set as well, which are pulled from the users table
		// Passwords are stored in sha1 format, so this method sha1's the password before testing it against the database
	
		$this->user_name = $user;
		$this->pass_word = $pass;
		$sql = "SELECT id FROM users WHERE user_name = '" . $this->user_name . "' AND pass_word = '" . $this->pass_word . "'";
		if($id = $this->dbh->select_one($sql)){
			$this->verified = true;
			$this->user_id = $id;
		}
		else{
			$this->messages[] = "Invalid login/password combination.";
		}
		
		// Add functionality to check whether user is logged in by logging user into db here
		
	}
	function logout(){
		unset($this->user_name);
		unset($this->pass_word);
		$this->verified = false;
	}
	function load_id($user){
		$this->user_name = $user;
		$sql = "SELECT id FROM users WHERE user_name = '" . $this->user_name . "'";
		if($id = $this->dbh->select_one($sql)){
			$this->user_id = $id;
		}
		else{
			$this->messages[] = "No username " . $this->user_name . " registered.";
		}
	}
	function has_priv($p){
		return $this->privs[$p];
	}
	function exists(){
		$sql = "SELECT id FROM `users` WHERE `user_name` = '" . $this->user_name . "' LIMIT 1";
		return is_int($this->dbh->select_one($sql)) ? true : false;
	}
	function valid_user_name(){
		return preg_match("/^[\s\w]{3,15}$/i", $this->user_name);
	}
	function set_privs($array){
		foreach($array as $key => $val){
			if(array_key_exists($key, $this->privs)) $this->privs[$key] = $val;
		}
	}
	function set_user_name($user_name){
		if(!$this->exists()){
			if($this->valid_user_name()){
				$this->user_name = $user_name;
			}
			else{
				$this->error = true;
				$this->messages[] = "Invalid user name - must be alpha-numeric between 3 and 15 characters. Spaces are allowed.<br>" . $this->dbh->last_error;
			}
		}
		else{
			$this->error = true;
			$this->messages[] = "User name already exists<br>" . $this->dbh->last_error;
		}
	}
	function set_pass_word($pass, $verify_pass){
		if($pass == $verify_pass){
			$this->pass_word = sha1($pass);
		}
		else{
			$this->error = true;
			$this->messages[] = "Passwords did not match";
		}
	}
	function save_changes(){
		$r1 = false;
		$r2 = false;
		$sql  = "UPDATE `privileges` SET ";
		$sql .= "`user_dir` = '" . $this->user_dir . "',";
		$sql .= "`admin` = '" . $this->privs['admin'] . "',";
		$sql .= "`mk_dir` = '" . $this->privs['mk_dir'] . "',";
		$sql .= "`del_dir` = '" . $this->privs['del_dir'] . "',";
		$sql .= "`upld_dir` = '" . $this->privs['upld_dir'] . "',";
		$sql .= "`dnld_dir` = '" . $this->privs['dnld_dir'] . "',";
		$sql .= "`ren_dir` = '" . $this->privs['ren_dir'] . "',";
		$sql .= "`mk_file` = '" . $this->privs['mk_file'] . "',";
		$sql .= "`upld_file` = '" . $this->privs['upld_file'] . "',";
		$sql .= "`del_file` = '" . $this->privs['del_file'] . "',";
		$sql .= "`dnld_file` = '" . $this->privs['dnld_file'] . "',";
		$sql .= "`view_file` = '" . $this->privs['view_file'] . "',";
		$sql .= "`ren_file` = '" . $this->privs['ren_file'] . "',";
		$sql .= "`edit_file` = '" . $this->privs['edit_file'] . "' WHERE `user_id` = '" . $this->user_id ."'";
		if(!$this->error) $r1 = $this->dbh->update_sql($sql);
		$sql  = "UPDATE `users` SET ";
		$sql .= "`user_name` = '" . $this->user_name . "',";
		$sql .= "`pass_word` = '" . $this->pass_word . "' WHERE `id` = '" . $this->user_id ."';";
		if(!$this->error) $r2 = $this->dbh->update_sql($sql);
		if($r1 AND $r2){
			return true;
		}
		else{
			$this->error = true;
			$this->messages[] = "Could not save changes!<br>" . $this->dbh->last_error;
		}
	}
	function load_values($id=false){
		if(!isset($this->user_id)) $this->user_id = $id;
		if($this->user_id){
			$sql = "SELECT * FROM `users` WHERE id = " . $this->user_id;
			$r = $this->dbh->select($sql);
			while($row = $this->dbh->get_row($r, "MYSQL_ASSOC")){
				$this->user_name = $row['user_name'];
				$this->pass_word = $row['pass_word'];
			}
			$sql = "SELECT * FROM `privileges` WHERE user_id = " . $this->user_id;
			$r = $this->dbh->select($sql);
			$row = $this->dbh->get_row($r, "MYSQL_ASSOC");
			if(!empty($row['user_dir'])) $this->user_dir .= $row['user_dir'];
			if(is_array($row)){
				$this->privs = $row;
			}
			unset($this->privs['user_id']);
			unset($this->privs['user_dir']);
		}
		else{
			$this->error = true;
			$this->messages[] = "No user id specified";
		}
	}
}

class authenticate{
	var $user_id;
	var $pass_word;
	var $dbh;
	var $verified = false;
	function authenticate(&$dbh){
		$this->dbh = $dbh;
	}
	function password_match($id, $pass){
		$this->user_id = $id;
		$this->pass_word = $pass;
		$sql = "SELECT id FROM users WHERE id = '" . $this->user_id . "' AND pass_word = '" . $this->pass_word . "'";
		if($id = $this->dbh->select_one($sql)){
			$this->verified = true;
		}
		else{
			$this->messages[] = "Invalid login/password combination.";
		}
		
		// Add functionality to check whether user is logged in by logging user into db here
		
	}
	function logout(){
		$this->user_id = false;
		$this->pass_word = false;
		$this->verified = false;
	}
}

$dbh = new dataBase;
$session = new sessionHandle(SESSION_LENGTH);

// if(empty($_SESSION['user_name'])) $session->set_var('user_name', false);
// if(empty($_SESSION['pass_word'])) $session->set_var('pass_word', false);

if(!$session->get_var('user_name')) $session->set_var('user_name', false);
if(!$session->get_var('pass_word')) $session->set_var('pass_word', false);

if(!empty($_POST['user_name'])) $session->set_var('user_name', $_POST['user_name']);
if(!empty($_POST['pass_word'])) $session->set_var('pass_word', sha1($_POST['pass_word']));

$user = new user($dbh);
$user->load_id($session->get_var('user_name'));

$auth = new authenticate($dbh);
$auth->password_match($user->user_id, $session->get_var('pass_word'));

if($auth->verified){
    // You are logged in
}
else{
    // Incorrect login
}
Am I on the right track?
Last edited by Luke on Thu Jun 08, 2006 11:12 am, edited 1 time in total.
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Another wonderful term for the reading list: Unit Testing. But Patterns first...:)

The first question is: Does it work?
The second: What is each classes responsibility?
The third: Does any class have more than 1 responsibility?

The way I think of objects are as small narrow-minded (maybe even miserly) packages of functionality. Each object does one thing, and does it well. It doesn't care what all the other objects are up to so long as they give it the input it expects, and accepts the output it provides. Otherwise the whole population should start complaining loudly.

So we could have Users (easy to spot), the DataAccess functions to operate on them, and sets of other methods for various tasks: Authentication, Authorisation, etc. Now each of these are separate tasks, which suggest we need different objects for each. (Objects should have focused areas of responsibility). Also, if we combine all of these into one big class, we run the risk of losing any future reuse. If we try to re-use a User class in another application, we'll need to edit it and start messing with all sorts of stuff if the new application already has its own Authentication module. (Objects should be reuseable). On another tack, Objects should also contain the least amount of data possible - afterall the data may change, or become deprecated. (Objects should be independent of the data they operate on.)

Aborint might pop up now, and correct me...;) But these are my personal watch-outs...

As an example. Let's pretend the code exists. We have methods, but what are they? We start the fundamental building block. We have User objects first. These carry User data, which comes from a database. So to fetch a User we could have something like:

Code: Select all

$user = new User();
$user->getByPk(1);
No source code yet - we're just getting a feel for how things slot together. At a minumum this suggest we could have a User class with properties (the data), and some Data Access functions to access the database and populate the class data. In pattern speak, we might consider using the ActiveRecord pattern, or RowDataGateway pattern, or similar. Knowing the pattern points the way to a workable implementation for our use.

Since we know users need to be authenticated, we might try an Authentication class (this is all it does - it hates doing anything else). This would be used similar to:

Code: Select all

$auth = new Authenticator();
$auth->authenticate($user);
The authenticate() method needs more info, it may need the POST data, access to SESSION, etc. These are already superglobals so the simple option is to use them directly. Further down the road we might find a need for a Request object (auto filtered version of POST), and a Session object (maybe).

Since I'm rambling on a bit here, and maybe made a point... I'll conclude with another problem. Say we might have multiple ways of authenticating a user (Password-Match, LDAP, Challenge-Response, etc.). How would you add each option to your original class? It's here where OOP starts making sense. You could have a parent Authenticate class, and various sub classes which implement each different strategy. The parent (in PHP5) could also be abstract.

Code: Select all

abstract class Authenticate {

	protected $user; // all subs a User object!

	protected function __construct() {} // might not be needed - I have a habit of sticking common
										// construction tasks into a parent constructor
	
	// abstract is an enforcer - this will create an error if a sub-class does not implement
	// an authenticate() method
	abstract function authenticate($user);

}

class Authenticate_PasswordMatch extends Authenticate {

	public function __construct() {
		// any construct tasks (if any)
	}

	public function authenticate(User $user) {
		$this->user = $user;
		if($this->passwordsMatch())
		{
			// do authentication tasks (add to a private function for readability)
		}
	}

	private function passwordsMatch() {
		if($_POST['password'] == $user->getPassword())
		{
			return true;
		}
		return false;
	}

}
Usage:

Code: Select all

$auth = new Authenticate_PasswordMatch()
$auth->authenticate($user);
Is this making a little sense? By splitting responsibility into separate classes, we can create smaller focused classes, which are easier to develop and modify. A simple PasswordMatch authentication class is a basic unit in a web app - it could be re-used in later projects with few changes (none if you remove the dependencies like $_POST, and add a function to set a Password field to match).

Such classes are also easier to test (Unit Testing or other), and more readable if you keep methods small and focused and dump complex bits of logic into nicely named private functions. It's also more complex which is the main cost of all these OOP practices.

Okay, I'm going to shut up now and stop typing away like a mad hatter on speed...
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

No! Don't stop!

Sorry to not add anything useful here, but just wanted to hop in and say these threads and posts like yours Maugrim are really helping me (and others) in getting a grip on this OOP stuff.
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Just keep asking questions and folk will keep their elbows in a state of constant stress...;)
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Ok, so if I would expand on your example:

Code: Select all

<?php
class User {

    var $userID;
    var $userName;
    var $userPassword;
    var $userEmail;

    function User($details) {
        $this->userID = $details['id']; 
        $this->userName = $details['name'];
        $this->userPassword = $details['password'];
        $this->userEmail = $details['email'];
    }

    function getByPk( $this->userID ) { /* return the userID...*/ }

    function getPassword( $this->userPassword ) { /* return the userPassword...*/ }

    function createUser() {...}
    function updateUser() {...}
    function deleteUser() {...}
}


abstract class Authenticate { }

        protected $user; // all subs a User object!

        protected function __construct() {} // might not be needed - I have a habit of sticking common
                                                                                // construction tasks into a parent constructor
       
        // abstract is an enforcer - this will create an error if a sub-class does not implement
        // an authenticate() method
        abstract function authenticate($user);

}

class Authenticate_PasswordMatch extends Authenticate {

        public function __construct() {
                // any construct tasks (if any)
        }

        public function authenticate(User $user) {
                $this->user = $user;
                if($this->passwordsMatch())
                {
                        // do authentication tasks (add to a private function for readability)
                }
        }

        private function passwordsMatch() {
                if($_POST['password'] == $user->getPassword())
                {
                        return true;
                }
                return false;
        }

}

// usage:
$user = new User();
$user->getByPk(1);

$user2 = new User();
$user2->getByPk(2);
// etc...

$auth = new Authenticate_PasswordMatch()
$auth->authenticate($user);
$auth->authenticate($user2);


?>
One thing I see is that in the class Authenticate_PasswordMatch a reference is made to the function $user->getPassword(). Wouldn't that make reusability / independency more difficult?
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Yes it would, so you can find the common denominator data - the password, user_id, and pass that into the Authenticator separately if needed.

A complete class could end up being used similar to:

Code: Select all

$user = new User();
$user->getByPk(1);

// remove User as a dependency, and use setters
$auth = new Authenticate_PasswordMatch();
$auth->setMatch($user->getPassword);
$auth->setInput($_POST['password']);
$auth->setId($user->getId());
$auth->authenticate();
Or you could just set up the constructor to accept the values as parameters:

Code: Select all

$user = new User();
$user->getByPk(1);

// remove User as a dependency, and just pass in required data as parameters
$auth = new Authenticate_PasswordMatch($user->getId(), $user->getPassword, $_POST['password']);
$auth->authenticate();
This would all remove the User dep, and leave the class even more independent. It's also a bit more complex, but that's the price you have to accept for disassociating the classes for reuse.
bdlang
Forum Contributor
Posts: 395
Joined: Tue May 16, 2006 8:46 pm
Location: Ventura, CA US

Post by bdlang »

Maugrim_The_Reaper wrote:Another wonderful term for the reading list: Unit Testing. But Patterns first...:)

The first question is: Does it work?
The second: What is each classes responsibility?
The third: Does any class have more than 1 responsibility?

The way I think of objects are as small narrow-minded (maybe even miserly) packages of functionality. Each object does one thing, and does it well. It doesn't care what all the other objects are up to so long as they give it the input it expects, and accepts the output it provides. Otherwise the whole population should start complaining loudly.
If I may say, another excellent post, thanks for your time, it definitely helps.

My own take on this, once I finally got it into my head that a class should be responsible for itself, and responsible for as little as possible, the patterns start to become clear. You stop creating 'god classes' and start focusing on _one_thing_at_a_time_, refactoring and changing the way your application works becomes alot easier.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Wow, thanks for the examples Maugrim. Indeed, those 2 solutions both remove the user dep. Very cool.

It's all still very new and difficult for me, but slowly I start to see the patterns.
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

alright, I have modified the original script (above) and added a (very simple) authentication class. Any suggestions/ideas?
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Just takes a bit of practice. Reading through some of the posts on this forum can help too. It's well worth researching patterns, you can't avoid references to them in OOP...
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

The Ninja Space Goat wrote:alright, I have modified the original script (above) and added a (very simple) authentication class. Any suggestions/ideas?
A lot of code, let's take it one class at a time starting with your user_admin class. Here are my comments:

1. This class does not do any administration, it is a Gateway to the user table. I would call it UserGateway.

2. load_priv_names() is not used and probably won't be unless you are going to do some mapping -- so get rid of it.

3. The function populate_users() is what is called a finder method in Gateway classes. In this case it is usually called findAll() because that's what it does.

4. populate_users() should do a JOIN to load all of the user data. Doing an extra query for each user is an un-necessary performance hit.


I know I mention some naming in the above list which is always a hot button issue. My point here is that if you call this a Gateway class and use those property/method naming conventions then a large group of programmers will immediately know what your code is/does.
(#10850)
Post Reply