Oop Login And Database

[guestabc1]

Hi, I am trying to produce a OO login system (I am new to OO so this is just a simple test project that hopefully I can reuse!).

 

However I am unsure how I should get the database connection into my SystemUser class, at the moment I am extending the database class, but the database connection is not required for every instance of the class. For example the database connection is required when logging in, as I need to check the e-mail and password are correct, but then once logged in on other pages I don't require the database as I'm simply checking the set session variables e.g. the getIsAdminUser()

 

Here's a cut down version of my code, any help/advice would be greatly appreciated, thanks in advance!

 

class Database
{
protected $_db_connection;
function __construct($db_host, $db_user, $db_password, $db_name)
{
$this->_db_connection = new mysqli($db_host, $db_user, $db_password, $db_name);
}

function __destruct()
{
if($this->_db_connection)
$this->_db_connection->close();
}
}

 

 

class SystemUser extends Database
{
public function loginSystemUser($e_mail, $password)
{
// Checks the user login credentials and sets up a login session if they are valid
}

private function logSystemUserLogin()
{
// Logs a system user login attempt in the database
}

public function logoutSystemUser
{
// Destroys all set sessions
}

public function getIsAdminUser()
{
// Checks if the user is a admin user
}
}

 

 

$system_user = new SystemUser($db_host, $db_user, $db_password, $db_name);

[JonnoTheDev]

You can pass a database object into the SystemUser login method as a parameter if you like

 

i.e.

 

[code]public function loginSystemUser($db, $e_mail, $password)
{
$db->query("");
// Checks the user login credentials and sets up a login session if they are valid
}

// calling code
$db = new Database();
$user->loginSystemUser($db, $email, $pass);
[/code]

 

There is also no issue in creating objects inside class methods. You do not need to extend the database class every time. i.e

 

public function loginSystemUser($e_mail, $password)
{
$db = new Database();
// Checks the user login credentials and sets up a login session if they are valid
}

Edited December 20, 2012 by neil.johnson

[cpd]

There is also no issue in creating objects inside class methods. You do not need to extend the database class every time. i.e

 

public function loginSystemUser($e_mail, $password)
{
$db = new Database();
// Checks the user login credentials and sets up a login session if they are valid
}

 

There is an issue when you have namespaces because your creating a dependency. In fact, you've got a dependency here where the class creating the Database object is dependent on it being in the same namespace.

Edited December 20, 2012 by CPD

[JonnoTheDev]

I agree, however, will namespaces be used in this persons project? Large scale projects, yes. guestabc1, if you are using PHP 5.3 then you can find more info here:

 

http://www.sitepoint.com/php-53-namespaces-basics/

[guestabc1]

Thank you for the advice!

 

I'm not sure where I've picked this up from, but I thought it was bad practice to create an instance of an object inside a class?

 

I did think about passing in the database connection to the relevant methods but I wasn't sure if that is the correct thing to do because i've got loginSystemUser($db_connection, $e_mail, $password) which then calls the logSystemUserLogin() both of which require the database connection?

 

I have thought about setting $this->_db_connection from the loginSystemUser method and then logSystemUserLogin will have access to it but this doesn't seem right?

 

Thanks for your help again!

Edited December 20, 2012 by guestabc1

[cpd]

In the given scenario I would argue the database object should be passed to the user object upon instantiation. This will prevent you having to create multiple instances of the same database class. Alternatively you can use the Singleton pattern to ensure a single instance of the database class is created.

 

My previous comments speculate slightly and its worth noting every system will have some dependencies. You can limit the dependency on the database object by creating a factory that instantiates database dependent classes and passes the database object to them. This then allows the classes that need a database to be instantiated without ever knowing where the database object actually is.

[guestabc1]

In the given scenario I would argue the database object should be passed to the user object upon instantiation. This will prevent you having to create multiple instances of the same database class. Alternatively you can use the Singleton pattern to ensure a single instance of the database class is created.

 

This is kind of how I have it at the moment, although the user class extends the database class so my understanding is the database object is getting set upon construct. This approach is fine if I need to access the database all the time in the user object, but it is only upon login I need to do this, as once logged in, the system will be checking against sessions rather than accessing the database.

 

The singleton pattern seems interesting but I'm still not sure where to call the instance of the database class from?

 

My previous comments speculate slightly and its worth noting every system will have some dependencies. You can limit the dependency on the database object by creating a factory that instantiates database dependent classes and passes the database object to them. This then allows the classes that need a database to be instantiated without ever knowing where the database object actually is.

 

I don't quite understand this... sorry?! thank you for helping though!

[cpd]

You shouldn't extend to the database class as it effectively means your saying "A User is a Database" which is complete wish-wash. Extending "because I want to use what's available in the super object" isn't a reason to create a inheritance hierarchy.

 

A User may USE a database object but it doesn't represent a database itself nor should it contain the logic to directly interact with the database. Passing the database object via the method isn't wrong or right, its merely another method. I feel the database object should be passed to the User object for use.

 

E.g.

 

<?php

class Database {
// Code emitted
}

class User {

  private $database = null;

  public function __construct(Database $database) {
     $this->setDatabase($database);
  }

  private function _setDatabase(Database $database) {
     // Carry out validation on the $database object
     $this->database = $database;

     return true/false;
  }

  public function login() {
     $this->database->query("Some query");
     // Some logic
     return true/false;
  }
}

$database = new Database();
$user = new User($database);

?>

Edited December 20, 2012 by CPD

[guestabc1]

You shouldn't extend to the database class as it effectively means your saying "A User is a Database" which is complete wish-wash. Extending "because I want to use what's available in the super object" isn't a reason to create a inheritance hierarchy.

 

This makes sense to me now, thank you!

 

 

<?php

class Database {
// Code emitted
}

class User {

private $database = null;

public function __construct(Database $database) {
$this->setDatabase($database);
}

private function _setDatabase(Database $database) {
// Carry out validation on the $database object
$this->database = $database;

return true/false;
}

public function login() {
$this->database->query("Some query");
// Some logic
return true/false;
}
}

$database = new Database();
$user = new User($database);

?>

 

This makes sense for the login, but for where the database is not required this seems like a waste of resource. For example I have a method called getIsAdminUser() this basically checks the session variable to see what rights are set, every page in the system uses this method but does not require the database, is there a way of optionally setting the database in the construct or is this bad practice or should I extend the user class in someway to define the sessions? This is the part that is most confusing me! Thanks again for all your help!

[TOA]

If I may offer another opinion..

 

I would actually say your user class should have no knowledge of where the data comes from at all. What happens if the business rules change to include csv's? I think you should just pass in an array of data so the user doesn't care where it comes from. Handle the logging in the client code or a mediary class.

 

Just my two cents.

[cpd]

I agree with TOA. The actual validation of login credentials probably shouldn't be in the User class but instead in some sort of Authentication class. You can still have a login method that sets the necessary session data but doesn't actually carry out any queries.

[ignace]

The actual validation of login credentials probably shouldn't be in the User class but instead in some sort of Authentication class.

 

Or in the Controller, since you may want at some point support OAuth (Facebook Connect, Twitter, ..).

 

class AuthController {
  public function loginAction() {
    // normal form login
    // forwards to authenticateAction() for authentication
  }
  public function authenticateAction() {
    // authenticate callback of OAuth
  }
}

 

Which means your User object has to be completly authentication/database agnostic and be nothing more then a simple DTO.

 

class User {
  private $id;
  private $username;
  private $password;
  
  // getters/setters
}

Edited December 21, 2012 by ignace

[cpd]

I would never put the authentication logic into a controller. Instead it should be placed in an abstraction layer that implements an Authentication interface. The AuthController can then execute an "authenticate()" method defined by the Authentication interface allowing any authentication object (OAuth, database authentication etc) that implements the Authentication interface to be passed to the "AuthController"; this is polymorphism. The AuthController doesn't care for how authentication is carried out, it only expects a true or false response so it can then set the relevant session data.

 

That said, I don't think the OP states an MVC architecture so this is all going a little off piece.

Edited December 21, 2012 by CPD

[guestabc1]

Or in the Controller, since you may want at some point support OAuth (Facebook Connect, Twitter, ..).

 

class AuthController {
 public function loginAction() {
   // normal form login
   // forwards to authenticateAction() for authentication
 }
 public function authenticateAction() {
   // authenticate callback of OAuth
 }
}

 

Which means your User object has to be completly authentication/database agnostic and be nothing more then a simple DTO.

 

class User {
 private $id;
 private $username;
 private $password;

 // getters/setters
}

 

Thank you, this was the way I suspected.... Where would the instance of the AuthController be called from? as calling this from the User class is then the same as having the authentication embedded in this class? But the user class needs to know if the user is authenticated because of setting the session variables? Sorry if it should be obvious to me, but like I said in my opening post this is a learning curve for me