Should I be able to access a session variable from within a class without specifically setting it as an attribute or passing it in as an argument?
yes - the $_SESSION array is a superglobal
Ya, thanks. Not sure why I questioned it but it stopped working in one situation but that’s obviously not the issue. Again, I appreciate the answer.
I disagree.
You CAN access the session, but it doesn’t mean you SHOULD. Sure you can have a class which has the task of handling session information, but the average object (in my opinion) should be given that handler, not the session directly.
The reason is that you can extend that handler to use a non-session array - i.e not actually passed as a session, but is given information by the application to make the object act differently - which does, from time to time, come in handy.
I’m going to get this moved to the PHP Application Design forum so the thread attracts some attention from the OOP experts. That might help you immensely
Thanks, I look forward to the input.
bostboy, this is the Session wrapper class I use - I pass the class around to whichever objects need it like Jake was saying above:
<?php
class Session
{
private $sessionName;
public function __construct($sessionName=null, $regenerateId=false, $sessionId=null)
{
if (!is_null($sessionId)) {
session_id($sessionId);
}
session_start();
if ($regenerateId) {
//session_regenerate_id(true);
}
if (!is_null($sessionName)) {
$this->sessionName = session_name($sessionName);
}
}
public function set($key, $val)
{
$_SESSION[$key] = $val;
}
/*
to set something like $_SESSION['key1']['key2']['key3']:
$session->setMd(array('key1', 'key2', 'key3'), 'value')
*/
public function setMd($keyArray, $val)
{
$arrStr = "['".implode("']['", $keyArray)."']";
$_SESSION{$arrStr} = $val;
}
public function get($key)
{
return (isset($_SESSION[$key])) ? $_SESSION[$key] : false;
}
/*
to get something like $_SESSION['key1']['key2']['key3']:
$session->getMd(array('key1', 'key2', 'key3'))
*/
public function getMd($keyArray)
{
$arrStr = "['".implode("']['", $keyArray)."']";
return (isset($_SESSION{$arrStr})) ? $_SESSION{$arrStr} : false;
}
public function delete($key)
{
if (isset($_SESSION[$key])) {
unset($_SESSION[$key]);
return true;
}
return false;
}
public function deleteMd($keyArray)
{
$arrStr = "['".implode("']['", $keyArray)."']";
if (isset($_SESSION{$arrStr})) {
unset($_SESSION{$arrStr});
return true;
}
return false;
}
public function regenerateId($destroyOldSession=false)
{
session_regenerate_id(false);
if ($destroyOldSession) {
// hang on to the new session id and name
$sid = session_id();
// close the old and new sessions
session_write_close();
// re-open the new session
session_id($sid);
session_start();
}
}
public function destroy()
{
return session_destroy();
}
public function getName()
{
return $this->sessionName;
}
}
Okay… I started web programming last June and have progressed pretty well. I am starting a new site and wanted to improve it from the beginning and make it as OOP friendly as I can at this point. I am toward retirement and work alone out of the house. The only reason I bring that up is it limits my interaction with people like yourself who have the best practices knowledge of when to apply what principles. This forum has provided a wealth of information and it is hard to explain how much it means. With that in mind, understand that I can read the code you submitted previously but can’t relate to why or when you would necessarily use it.
Here is a piece of a class that I put together that uses the session variables I was asking about. I would immensely appreciate any advice or criticism of the way I am approaching it. It’s how I learn. Thanks
class Users {
public $userid=0;
public $email='';
public $password='';
public $firstname='';
public $lastname='';
public $username='';
function __construct() {
if (isset($_SESSION['user']['userid'])) {
$userid = $_SESSION['user']['userid'];
$username = $_SESSION['user']['username'];
if (isset($_SESSION['user']['email'])){$email = $_SESSION['user']['email'];}
}
}
public function addUser() {
include $_SESSION['dbserver']; //carries the db connection file location
$qt_sql = "insert into users (email, password, firstname, lastname, username, dateadded, datelastchange) values (?, ?, ?, ?, ?, now(), now())";
$passwordm = md5($this->password . 'xxx');
try {
if (!$prep1 = $link->prepare($qt_sql)) {throw new Exception($link->error);}
if (!$prep1->bind_param("sssss", $this->email, $passwordm, $this->firstname, $this->lastname, $this->username)) {throw new Exception($link->error);}
if (!$prep1->execute()) {throw new Exception($link->error);}
$retval = 99;
} catch (Exception $e) {
$retval = $e->getMessage();
}
return $retval;
}
}
The first thing that jumps out is your class is called “Users”, but it represents a single user. So I would rename the class “User” for starters.
Secondly, you are pulling and storing all the user information in the session. The preferred method would be to only store the user’s uid in the session, and use that to pull the rest of the information from the database.
Third, you are storing the db connection file in the session - don’t do that, store it in a constant (which will have global scope).
Your “addUser” method in my mind should be called “save” which is really just a semantics thing - it makes more sense to call $user->save() than $user->addUser() (to me anyway). Some people like to include methods within an object’s class to retrieve or save the object to the database, others like me prefer to have a separate class to handle those responsibilities.
I don’t feel that the user class should know anything about sessions. As your code stands right now you can’t create a user unless all their information exists in a session. A user class shouldn’t be much more than a container for the various properties of a user (similar to an associative array). Asking it to know about all the logistics of the rest of the system ties it into place and makes it un-reusable.
So personally this is how I would restructure this:
// instead of storing this in the session
define('DB_CONNECTION_FILE_LOCATION', '/path/to/file.php');
// stripped down user class
class User
{
public $userid=0;
public $email='';
public $password='';
public $firstname='';
public $lastname='';
public $username='';
}
// class responsible for retrieving, saving user objects
class UserGateway
{
protected $link;
public function __construct($dbLink)
{
$this->link = $dbLink;
}
public function findById($id)
{
$qt_sql = "select * from users where userid = ?)";
try {
if (!$prep1 = $this->link->prepare($qt_sql)) {throw new Exception($this->link->error);}
if (!$prep1->bind_param("s", $id)) {throw new Exception($this->link->error);}
if (!$prep1->execute()) {throw new Exception($this->link->error);}
$user = new User;
// map retrieved db rows to user object here
$user->userid = $id;
//$user->email = etc
return $user;
} catch (Exception $e) {
$retval = $e->getMessage();
}
return false;
}
public function save(User $user)
{
$qt_sql = "insert into users (email, password, firstname, lastname, username, dateadded, datelastchange) values (?, ?, ?, ?, ?, now(), now())";
$passwordm = md5($user->password . 'xxx');
try {
if (!$prep1 = $this->link->prepare($qt_sql)) {throw new Exception($this->link->error);}
if (!$prep1->bind_param("sssss", $user->email, $passwordm, $user->firstname, $user->lastname, $user->username)) {throw new Exception($this->link->error);}
if (!$prep1->execute()) {throw new Exception($this->link->error);}
$retval = 99;
} catch (Exception $e) {
$retval = $e->getMessage();
}
return $retval;
}
}
// some sample calling code
include DB_CONNECTION_FILE_LOCATION;
$userGateway = new UserGateway($link);
if (!$user = $userGateway->findById($_SESSION['user']['userid'])) {
// couldn't find user - take action here
}
// change user password and resave
$user->password = 'newpassword';
$userGateway->save($user);
First of all, thanks so much for taking the time to spell that out.
The first thing that jumps out is your class is called “Users”, but it represents a single user. So I would rename the class “User” for starters
Understand and I should pay more attention to naming. Since it is only me I am pretty lax but I even find myself sometimes trying to use short name so I don’t have type as much. Point taken.
Secondly, you are pulling and storing all the user information in the session. The preferred method would be to only store the user’s uid in the session, and use that to pull the rest of the information from the database.
This is really interesting and one of those best practices questions that has been lingering regarding the use of session vars, db and generally how and where to store information. My thought prior to this was that this information is used pretty frequently so what is the trade-off of querying the database every time I need it vs the overhead cost of storing it in a session var and maintaining changes to that session var. I guess I have been leaning toward the side that says it is more expensive to query the db. You are saying that’s not true I assume, go ahead and lean on the db, that’s what it is supposed to do.
Third, you are storing the db connection file in the session - don’t do that, store it in a constant (which will have global scope).
Another question I have had is how constants behave exactly. I have seen people use them in places where I use session vars and I don’t use them at all because I don’t really understand them. I did some research on it but came away as confused as when I went in. I’ll define a “logic cycle” as beginning at the start of a script execution and running until PHP lets all the variables go. So when I start a logic cycle, I use a session_start() and all my session variables are recalled for me. With a constant, if I define it at the start of a logic cycle, do I need to redefine it at the start of the next logic cycle or is it now available until I reset it’s value or destroy it? And can I destroy it? And when does it get destroyed?
I don’t feel that the user class should know anything about sessions. As your code stands right now you can’t create a user unless all their information exists in a session. A user class shouldn’t be much more than a container for the various properties of a user (similar to an associative array). Asking it to know about all the logistics of the rest of the system ties it into place and makes it un-reusable.
When I first read this I didn’t understand it but the more I chomp on it, it is the fundamental supposition of OOP. True encapsulation. I think I get it and if I can turn this corner, I think it goes a long way to actually writing useful OOP code. I will paste this on the monitor so I can look at it every time I go to create a new class. Thanks.
A couple of code questions:
-
The User class is interesting to me. I would have included this in the class I was writing but it makes perfect sense to isolate it, that way it can be reused by other user based classes. Is that correct? Do you have one of these for each “subject” in the database? Does it tend to be subject oriented or database structure oriented? Which points to another question. I consider myself a pretty good database designer but was reading about OO database design and was not able to wrap my head around that concept. Is that something worth spending more time on at this point or do I have bigger fish to fry?
-
In class UserGateway, you are creating protected $link. I understand the concept of protected, but what difference does it really make at a practical level? Again, I think this is one of those sloppy habits I get into where I don’t pay attention to this kind of thing. Does this just assure that this will not be tampered with from outside because there is no reason for it to be tampered with from outside?
-
In this example with method findById, we are finding a single user so mapping to the User class attributes makes perfect sense. If I had a class that retrieved all users in a city, would I use the same User class or would there be another one that would be an array of the User class? Does that question make sense?
-
In the save method, we are using an insert which makes sense the first time around, but in your code example when you say “// change user password and resave” it looks like you are resetting the password to a new value and then executing the save method which will execute an insert command into the table where the user already exists. I assume I am missing something here? Won’t that fail? Or in your code, are you assuming that the findById issued just prior to that indicates that the user doesn’t exist?
-
In the findById method, if it goes through the try section and survives to the new User part and then maps to the User object, do you assume you exit there somehow? You have a return false sitting outside the try block, wouldn’t that get executed if the try block succeeds?
I think that’s it. What an incredible insight and a huge help. Some day I would love to talk about Exception and Error handling.
Yes - all your session variables get written to a text file on the server, and database I/O is always less expensive than disk I/O.
Constants are very simple. They are basically just a variable whose value can never been changed after it is set. Constants also have global scope. Usually any kind of configuration information is a prime candidate for a constant. The are much more useful than a global variable because they can’t accidently be overwritten. Constants are no different from variables in that they are erased from memory as soon as script execution is complete, so they do not carry over from one request to the next.
Yes that is correct.
Objects quite often do map to database tables, but not always. The best thing to do (but sometimes the hardest) is to COMPLETELY ignore the database when you are designing your objects. Worry about how to map that object to database table(s) after all your objects are designed for your system.
Yes - I prefer to make all my properties protected, and if access is needed to create getters and setters for them. That is personal preference and up to you.
Yes - I would do it in the same class - create a method called getArrayForCity($city), or getCollectionForCity($city), etc. and return an array or collection of the user objects.
Yes sorry - that was some quick psuedo code. My save() methods usually have both insert and update commands in them, and the method decides which one to do based on a set flag in the User class. The simplest way is to just see if the object has an $id, but you do have to be careful with that.
Sorry again - that was some quick code I typed up and i had some obvious errors that i may have edited after you saw the post. generally when I’m attempting to fetch an object I either return the object or false if it can’t be retrieved - so how you structure your exception handling etc. is totally up to you.
Again, thanks. Unbelievably helpful.
Any chance I can get you to expound a little on the following comment, i.e. how you build it and what you mean when you say return a collection of the user objects?
Yes - I would do it in the same class - create a method called getArrayForCity($city), or getCollectionForCity($city), etc. and return an array or collection of the user objects.
Collections are an idea from other programming languages, but basically it’s an object that contains several other objects much like an array. It is not built into php’s core, but several people have written their own classes to do this type of thing. It’s probably not important at this stage, but if you are super curious I could post a code example.
I would love to see it if you can find the time. No rush.
Alright, here is my Collection class that I wrote years ago and have been using extensively ever since:
<?php
class Collection {
protected $objects = array();
protected $deletedObjects = array();
protected $resetFlag;
protected $numObjects;
protected $iterateNum;
public function __construct()
{
$this->resetIterator();
$this->numObjects = 0;
}
public function __toString()
{
$str = '';
foreach ($this->objects as $obj) {
$str .= '--------------------------<br />'.$obj.'<br />';
}
return $str;
}
public function add($obj, $id=null)
{
$this->objects[] = $obj;
$this->numObjects++;
}
public function next()
{
$num = ($this->currentObjIsLast()) ? 0 : $this->iterateNum+1;
$this->iterateNum = $num;
}
/*
get an obj based on one of it's properties.
i.e. a User obj with the property 'username' and a value of 'someUser'
can be retrieved by Collection::getByProperty('username', 'someUser')
-- assumes that the obj has a get method with the same spelling as the property, i.e. getUsername()
*/
public function getByProperty($propertyName, $property)
{
$methodName = "get".ucwords($propertyName);
foreach ($this->objects as $key => $obj) {
if ($obj->{$methodName}() == $property) {
return $this->objects[$key];
}
}
return false;
}
/*
alias for getByProperty()
*/
public function findByProperty($propertyName, $property)
{
return $this->getByProperty($propertyName, $property);
}
/*
get an objects number based on one of it's properties.
i.e. a User obj with the property 'username' and a value of 'someUser'
can be retrieved by Collection::getByProperty('username', 'someUser')
-- assumes that the obj has a get method with the same spelling as the property, i.e. getUsername()
*/
public function getObjNumByProperty($propertyName, $property)
{
$methodName = "get".ucwords($propertyName);
foreach ($this->objects as $key => $obj) {
if ($obj->{$methodName}() == $property) {
return $key;
}
}
return false;
}
/*
get the number of objects that have a property whose value matches the given value
i.e. if there are several User objs with a property of 'verified' set to 1
the number of these objects can be retrieved by Collection::getNumObjectsWithProperty('verified', 1)
-- assumes that the obj has a get method with the same spelling as the property, i.e. getUsername()
*/
public function getNumObjectsWithProperty($propertyName, $value)
{
$methodName = "get".ucwords($propertyName);
$count = 0;
foreach ($this->objects as $key => $obj) {
if ($obj->{$methodName}() == $value) {
$count++;
}
}
return $count;;
}
/*
remove an obj based on one of it's properties.
i.e. a User obj with the property 'username' and a value of 'someUser'
can be removed by Collection::removeByProperty('username', 'someUser')
-- assumes that the obj has a get method with the same spelling as the property, i.e. getUsername()
*/
public function removeByProperty($propertyName, $property)
{
$methodName = "get".ucwords($propertyName);
foreach ($this->objects as $key => $obj) {
if ($obj->{$methodName}() == $property) {
$this->deletedObjects[] = $this->objects[$key];
unset($this->objects[$key]);
// reindex array & subtract 1 from numObjects
$this->objects = array_values($this->objects);
$this->numObjects--;
$this->iterateNum = ($this->iterateNum >= 0) ? $this->iterateNum - 1 : 0;
return true;
}
}
return false;
}
public function currentObjIsFirst()
{
return ($this->iterateNum == 0);
}
public function currentObjIsLast()
{
return (($this->numObjects-1) == $this->iterateNum) ? true : false;
}
public function getObjNum($num)
{
return (isset($this->objects[$num])) ? $this->objects[$num] : false;
}
public function getLast()
{
return $this->objects[$this->numObjects-1];
}
public function removeCurrent()
{
$this->deletedObjects[] = $this->objects[$this->iterateNum];
unset($this->objects[$this->iterateNum]);
// reindex array & subtract 1 from iterator
$this->objects = array_values($this->objects);
if ($this->iterateNum == 0) { // if deleting 1st object
$this->iterateNum = 0;
$this->resetFlag = true;
} elseif ($this->iterateNum > 0) {
$this->iterateNum--;
} else {
$this->iterateNum = 0;
}
$this->numObjects--;
}
public function removeLast()
{
$this->deletedObjects[] = $this->objects[$this->numObjects-1];
unset($this->objects[$this->numObjects-1]);
$this->objects = array_values($this->objects);
if ($this->iterateNum == $this->numObjects-1) { // if iterate num is set to last object
$this->resetIterator();
}
$this->numObjects--;
}
public function removeAll()
{
$this->deletedObjects = $this->objects;
$this->objects = array();
$this->numObjects = 0;
}
/*
sort the objects by the value of each objects property
$type:
r regular, ascending
rr regular, descending'
n numeric, ascending
nr numeric, descending
s string, ascending
sr string, descending
*/
public function sortByProperty($propName, $type='r')
{
$tempArray = array();
$newObjects = array();
while ($this->iterate()) {
$obj = $this->getCurrent();
$tempArray[] = call_user_func(array($obj, 'get'.ucWords($propName))); //$obj->get$propName();
}
switch($type)
{
case 'r':
asort($tempArray);
break;
case 'rr':
arsort($tempArray);
break;
case 'n':
asort($tempArray, SORT_NUMERIC);
break;
case 'nr':
arsort($tempArray, SORT_NUMERIC);
break;
case 's':
asort($tempArray, SORT_STRING);
break;
case 'sr':
arsort($tempArray, SORT_STRING);
break;
default:
throw new General_Exception('Collection->sortByProperty(): illegal sort type "'.$type.'"');
}
foreach ($tempArray as $key => $val) {
$newObjects[] = $this->objects[$key];
}
$this->objects = $newObjects;
}
public function isEmpty()
{
return ($this->numObjects == 0) ? true : false;
}
public function getCurrent()
{
return $this->objects[$this->iterateNum];
}
public function setCurrent($obj)
{
$this->objects[$this->iterateNum] = $obj;
}
public function getObjectByIterateNum($iterateNum)
{
return (isset($this->objects[$iterateNum])) ? $this->objects[$iterateNum] : false;
}
public function iterate()
{
if ($this->iterateNum < 0) {
$this->iterateNum = 0;
}
if ($this->resetFlag) {
$this->resetFlag = false;
} else {
$this->iterateNum++;
}
if ($this->iterateNum == $this->numObjects || !isset($this->objects[$this->iterateNum])) {
$this->resetIterator();
return false;
}
return $this->getCurrent();
}
public function resetIterator()
{
$this->iterateNum = 0;
$this->resetFlag = true;
}
#################### GETTERS
public function getDeletedObjects()
{
return $this->deletedObjects;
}
public function getIterateNum()
{
return $this->iterateNum;
}
public function getNumObjects()
{
return $this->numObjects;
}
#################### SETTERS
public function setDeletedObjects($key, $val)
{
$this->deletedObjects[$key] = $val;
}
public function resetDeletedObjects()
{
$this->deletedObjects = array();
}
}
Here are some neat things I can do with it - all based on the idea of a user collection, and appropriate methods in my gateway class:
/// get all users
$users = $userGateway->getCollectionForAll(); // returns a Collection of user objects
// add a user and save the collection - results in 'John' being added to the database
$john = new User;
$john->firstname = 'John';
$users->add($john);
$userGateway->saveCollection($users);
// draw a table of usernames
echo '<table>';
if ($users->isEmpty()) {
echo '<tr><td> No Users </td></tr>';
} else {
while($user = $users->iterate()) {
echo '<tr><td>'.htmlentities($user->username).'</td></tr>';
}
}
echo '</table>';
// get a user with an id of 1 and say hi
if ($selectedUser = $users->getByProperty('id', 1)) {
echo 'Hi, '.htmlentities($user->firstName);
}
// delete the users whose first name is bob
// deleted objects end up in the class's deletedObjects array
// and when passed to $userGateway->saveCollection() method, the method deletes all these objects for me
while($user = $users->iterate()) {
if ($user->firstname == 'Bob') {
$users->removeCurrent();
}
}
$userGateway->saveCollection($users); // now all users with first name Bob are deleted from the DB
Thanks so much. Not sure at this point how many lifetimes it will take me to understand it, but I will give it my best. I am implementing the user classes and all seems to be going well. Maybe at some point I’ll post it out here for you to shoot holes in it. That is absolutely the best way to learn this and your input has been invaluable.
Just a quick question, what is the significance of the double underscore before the toString() method?
Never mind, ignore the __toStrings question, I found it.