How should I do this constructor...

[Samishii23]

So I have a user login system. The login works.
What I want to do is make a variable array in the class available for general usage. Obviously in most cases set the data in the constructor.

My login / user class has the login stuff within it, as well as the user functions. In this class I have {bool LoggedIn()} that I use for login status verification. So the class could very well be constructed and the user isn't logged in.

So I make the constructor a login verification? Or keep the method? Or both? Use the method within the constructor ( if thats possible ) to login verify then set the details?

Not sure which is the best, and most efficient way.

[Dormilich]

could you elaborate on your problem, because I didn’t understand it (specifically, I didn’t get what the array has to do with the Login class).

[Samishii23]

Sorry Dorm...

I have a class "Users". With a Method "bool LoggedIn()".
The class has the login functionality in it, and I was going to add to it an array variable with the user data.

My problem is, is the way I have it setup now, is that the method LoggedIn() is the determining way of telling wether or not the user is indeed logged in.

I'm just curious if it'll be easier to rework the constructor to do the verification or use LoggedIn() in the constructor to set the user data, or have two seperate login verifications. I say two, because each user access page will see LoggedIn() to allow access or not.

Really I'm just wondering which is the more effective way. Or something else that I'm not seeing?

[Dormilich]

for your user class it is sensible, to store the user data. the obvious point to pass this data is through the constructor, although you could also use setter methods.

depending on how loggedIn() currently works, I don’t see the need of changing the method. and as such you can call this method whereever you want. if you need an opinion about the class’ code, feel free to post it (or send me a PM).

[Samishii23]

I'll post it.
Feel free to give me any points or hints about it. I'm welcome to anything.

Thanks for the replies! Really appreciate it.

[Dormilich]

Feel free to give me any points or hints about it.

it is good coding practice to make functions and methods lowercase and class names uppercase, see Userland Naming Guide

EDIT: do you have some kind of class synopsis or usage example?

[Samishii23]

I just figured seeing LoggedIn() was easier then looking at logged_in() or loggedin.
Maybe that was from my short dive into C#.

This is my main page that requires the login.

session_start();
require_once 'src/library.php';
$Conn = DBConn();
$User = new Users();
$Program = new DM();
 
$UserLoggedIn = $User->LoggedIn();
if ( $UserLoggedIn == false ) header('Location: login.php');

This is the only way I've thought of so far to do this.

[Dormilich]

DBConn is your main database handler?

[Samishii23]

Yes sir.

function DBConn() {
    // FUTURE: Implement defined ERRORs
    $Conn = @mysql_connect(CONFIG_DBHOST, CONFIG_DBUSER, CONFIG_DBPASS);
    if ( !$Conn )
        return false;
    else {
        $db = mysql_select_db(CONFIG_DBNAME, $Conn);
        if ( !$db )
            return false;
        else
            return $Conn;
        }
    }

[Dormilich]

Ok, I looked a bit more at the class and I can talk a bit about my thoughts.

Firstly, this class design is PHP 4. I would take the time to code it into PHP 5 (i.e. __construct() instead of User()).

Another point that makes the class difficult to understand is the $args parameter. though it may be convenient, I (personally) have no idea what keys are allowed and which values are allowed. I’d rather make each key an own parameter with its own default value (this also spares you the work of calling the array elements each time). You also should look into Type-Hinting.

I also see a lot of mysql_* functions and SQL Query string creation. for the former, look into PDO or MySQLi (the mysql_* functions are outdated!), for the latter, consider a separate DB handler (that may be a set of functions or a class). DB handling code does not belong in the User class, you may define a parent class that handles it. additionally, there is no prevention of SQL Injection attacks (easiest solved by Prepared Statements, for which you require PDO or MySQLi).

One important thing is error handling. currently your functions/method return different data types, which require you to always check the return value. check out Exceptions, I think your code will greatly benefit from it*.



* – PDO can use Exceptions to handle errors, this really cleans your code up!

[Samishii23]

Thanks for the response!
I'd like to point out really quick that I don't have a constructor currently. I have a __construct() in there thats commented out, which is why I had made this thread.
Also the class is named "Users" while I have the "User" method in there, its not the constructor.
Also, I use the $args array because I'm trying to make the methods a little more dynamic if possible. So some of the $args could be different, if the arg in the method I need do I do something like this:

method(null, null, null, "data");

Just quickly. Is PDO an off branch of MySQL or something like that? Cause my web host only offers MySQL(i).

Other then that thanks, i'll look into what you mentioned.

[Dormilich]

I'd like to point out really quick that I don't have a constructor currently.

you have. it is called User() (that‘s the way PHP 4 defined the class constructor, which is still working in PHP 5 (for compatibility reasons) unless there is a __construct() method defined)

Just quickly. Is PDO an off branch of MySQL or something like that? Cause my web host only offers MySQL(i).

PDO is PHP’s native Database Abstraction Layer and as such part of the PHP default configuration. it works with most databases (as long as there is a database driver installed) and it is not related to MySQLi (only by intend).

for me this would be reason enough to change the web hoster. but maybe they’ll install if if you ask nicely.

Also, I use the $args array because I'm trying to make the methods a little more dynamic if possible. So some of the $args could be different, if the arg in the method I need do I do something like this:

depending on which args you pass least often, default values make the calls easier. e.g.

function hash($data, $algo = "sha256", $salt = true, $length = 128)
{
    // function body
}
 
// a very simple hash call:
// if not explicitly overwritten, it uses the default values
hash("the string to hash");
// with a different algorithm
hash("the powers to be", "ripemd160");
// default w/o salt
// note that you can only leave out the last values
hash("what ever should be hashed", "sha256", false);

[Samishii23]

I rewrote my class hash method a little bit.

// Hashing function
// Requires $type and $str
private function Hash($type, $str, $len=128) {
    if ( $type == '' || $str == '' )
        return false;
 
    // Do Password Hash
    switch($type) {
        case 'password': // Login passwords
            // If Salt needs to be used
            if ( $this->UseSalt )
                $str = $this->HashSalt . $str;
            $str = hash($this->HashChoice, $str);
            break;
        case 'login-key': // Random hash for login keys
            $str =  hash($this->HashChoice, mt_rand(100000,1000000));
            break;
        }
 
    // If theres a different length specified
    if ( $len != 128 )
        $str = substr($str, 0, $len);
 
    // Return the hash
    return $str;
    }

A little better by your standards?
I didn't include the hash type or salt because its specified in the class "Options".

private $HashChoice = 'sha512';
private $UseSalt = false;
private $HashSalt = '(19|1|&[(8g5NcY`QvOr]<p$Or';