php problem - returning a class from a function

[Whiternoise]

Currently i've got a working database class for MySql interaction, the function that i use is:

function newdb()
{
        //host, username variables etc are defined here but i removed for security

	
	$db = new db();
	$connection = $db->connect($dbhost, $dbusername, $dbpassword, $db_name);
	
	return $db;
}

That works fine, i often use it to call things like $db->query($sql); and whatnot.

What i'm currently trying to do is check whether a user is logged in, what authorisation level they are and then return a user class that i can then do things with (classes: guest, user extends guest, mod extends user, admin extends user - you get the idea)

The function i'm trying to use to create a new $user dependent on the rank that's stored in the database is:

function createuser()
{
// if the user is logged in, create the user based on rank and set all the details (from the database) as class properties
	if(isset($_SESSION['logged_in']))
	{
		if($_SESSION['userrank'] == 1)
		{
		$user = new user($_SESSION['userid']);
		$user->setuserdetails();
		}
		elseif($_SESSION['userrank'] == 2)
		{
		$user = new moderator($_SESSION['userid']);	
		$user->setuserdetails();
		}
		elseif($_SESSION['userrank'] == 3)
		{
		$user = new administrator($_SESSION['userid']);
		$user->setuserdetails();	
		}
	}
	else
	{
		$user = new guest();	
	}
	
	return $user;
}

The session variables are fine (i've checked them) and are set upon login. I've done a little debugging and i've found that without logging in, it appears to set the $user variable but it doesn't seem to like returning it properly.

Running this:

	
else
	{
		$user = new guest();	
		$user->testecho();
	}

Also works as it should (i.e. just before the function returns $user) and echoes out a test message to the browser.

Simply creating a user in the main page works, and i can call methods fine, it's just when i try to get it from the function it plays up. Am i missing something obvious?

[aidanjt]

Shouldn't you be doing session initialisation way at the start of the page loading, instead of while you're trying to create a new user?

[Whiternoise]

The session is always started right at the top of the page (i've got an include file that deals with the top part of each page and session_start() is called as the first line), so i'm not sure what you mean. The session variables listed there are all set when the user is logged in - userid is pulled from the database and used to grab everything else like username, authorisation and whatnot.

[aidanjt]

Ug, that's a butt ugly legacy practice from before PHP had actual class support. Reorganise your user class to call session_start() on object initialisation (the __constructor function).

I'm also a bit confuse with the way you're doing some things. You seem to be wrapping procedural code around your classes, which makes no sense to me. Why don't you do all that checking *inside* your classes? Make use of constructor functions for your classes if you want to execute code when a new object is created.

I'm not criticising or anything, just giving you some pointers to keep your code from becoming heavily fragmented. If everything is more cohesive you'll have an easier time of bug hunting.

Anyway, see: http://www.php.net/manual/en/language.oop5.decon.php

[sadbuttrue]

There is nothing in the code provided that shouldn't work. The only thing I can see is a missing logic step for logged in users ('else' for when/if userrank isn't set properly).

[Whiternoise]

Ok i'll give that a go

At the moment each page loads:

1. start session
2. grab any necessary includes (functions and classes)
3. create a database object (i've seen some user classes that incorporate the db connection, but i need it for other stuff so it's separate - and passed to the user class as necessary)
4. create a user object
5. do all the general content gumph

OOP confuses the bejesus out of me

I'm still not quite sure how i should do this though, would i be right in saying..

	function user($sessionuserid,$db)
	{
		$this->userid = $sessionuserid;
		$this->db = $db;
	
		$sql = "SELECT * FROM user WHERE userid = '$this->userid'";
		$result = $this->db->query($sql);
		$userarray = $this->db->FetchArray($result);
	
		foreach ($userarray as $field => $value)
		{
			$this->$field = $value;
		}
	}

So this would effectively be the construct method..

The reason i did it the way it was, was i'm sure someone told me it was better practice to have separate methods for setting properties rather than bunging it all in one (i.e. have separate SetVALUE and getVALUE methods for each thing).

EDIT: Actually ok i remember why this confused me - how would i sort it in the constructor so that it makes a new object dependent on rank? Surely i'd just end up making a constructor that returns a class and i could end up with an infinite loop? All i want is that when the user logs in, i can pull the rank from the database and then on each page i create the user and if the rank is either zero or undefined, create a guest and if it's not then create a user/mod/admin as required.

And also, if i'm creating an object based on something that's stored in the session, surely i'd have to call session_start before i create the object?

Or am i overcomplicating things and you were only referring to "setuserdetails" upon construction rather than in the other function?

[aidanjt]

No, you store the rank in the class, and the created object will retain it until you delete it or execution finishes.

[Whiternoise]

Sorry to be a pain, but could you be a bit more specific (or possibly give an example of how the code should flow?)?

Should i just have one big class rather than extending a basic "user" to classes which have methods that need more privileges? If i store the rank in the class - going with an extendable system - i'd still have to construct the object based upon what rank the user is?

oops realised i edited after you posted, but anyway

EDIT: Oh and i realised what the stupid mistake was at the start - i simply constructed the object, i forgot to assign it to a variable first - so of course whenever i tried to call a method it moaned that $user was non-object.

[aidanjt]

It's no problem.

class user {
    $userid;
    $username;
    $sessionid;
    $rank;
    $db;

__constructor($db,any,other,variables,you,want,to,pass) {
    start_session();
    $this->db = $db;
    // user initalisation block or call an init_user function
    $this->userid = $_SESSION['userid'];
    // save any other variables
    }    
}

// initialise database connection
$user = new user($db);
// do page stuff

Now if you want to access any user details you can just do if($user->rank > 1) for e.g. to determine whether to load mod/admin code.

I hope this loose example helps.

[Whiternoise]

Ah ok that makes sense.

So, are you saying that instead of creating a user from inputs, i should just go ahead and create the object and construct it based on whatever happens to be in the session?

Out of interest though, is there anything intrinsically wrong about my first approach (having an extensible class) besides the procedural bumph?

[aidanjt]

So, are you saying that instead of creating a user from inputs, i should just go ahead and create the object and construct it based on whatever happens to be in the session?

Yup, you can still make the class generic enough to create as many users as you'd like, and manage your session or theirs, or change user details. Just create an object for each user you want to manipulate. Or you could create a child class from the user parent class, and specialise it for current user sessions only, and create an object out of that instead.

Out of interest though, is there anything intrinsically wrong about my first approach (having an extensible class) besides the procedural bumph?

It's not that you couldn't do it that way, it's just an approach that's strongly discourage for reasons of maintainability.

[Whiternoise]

Righto, thanks for all this!

[scaryjim]

I'm intrigued as to what you use these user-type objects for, tbh - you seem to have all the main properties stored in session variables anyway! You don't need to create the objects to control the flow of the program - you could just check $_SESSION['userrank'] - so presumably these classes / objects do something else? How do they differ from each other?