2

I have two class files: one is called class.database.php, and it is used solely for any functions that have to be done on the database (connect, disconnect, query, etc.)

This is class.database.php:

<?php
  class DATABASE
  {
    public function __construct() {
        $this->getConnected();
    }

    public function getConnected() {
        $dbHost = "localhost";
        $dbUser = "tysonmoyes";
        $dbPassword = "F!lmtrepid";
        $db = "tysonmoyes";

        $dbConn = new mysqli($dbHost, $dbUser, $dbPassword, $db);
        $this->dbConn = $dbConn;
    }

    function queryDB($queryString) {
        return mysqli_query($this->getConnected(), $queryString);
    }

    public function close() {
        mysqli_close($this->connection);
    }
  }
?>

My second class file is called class.users.php, and it handles all the information on a user account. It looks like this:

<?php
  include_once('config.php');
  class USER
  {
    private $conn;

    // All the variables needed for the user profile.
    public $username;
    public $userID;
    public $password;
    public $firstName;
    public $lastName;
    public $emailAddress;
    public $address;
    public $city;
    public $province;
    public $country;
    var $myConn;

    function __construct($conn){
        $this->myConn = $conn;
    }

    function createNewUser($username, $password) {
        // Clean inputs
        $username = trim($username);
        $password =  trim($password);

        // Encrypt password
        $password = md5($password);

        // Check if username already exists
        $checkSQL = "SELECT * FROM users WHERE username = '$username'";
        $checkResult = $this->myConn->queryDB($checkSQL);
        if($checkResult->num_rows > 0) {
            $error = "true";
            $errorMessage = "This username has already been taken. Please try again";
        }

        else {
            $insertSQL = "INSERT INTO users(username, password) VALUES('$username', '$password')";
            //$insertResult = $this->callDB()->query($insertSQL);

            // Get the user ID
            $userID = $this->insert_id;

            // Set the SESSION globals
            $_SESSION['username'] = $username;
            $_SESSION['userID'] = $userID;
        }

    }

    function login($username, $password) {
        $sql = "SELECT * FROM users WHERE username = '$username' && password = '$password'";
        $result = $this->conn->query($sql);
        $row = $this->conn->fetch_array($result, MYSQL_ASSOC);
        $count = $this->conn->num_rows($result);
        if ($count == 1) {
            // Set Session Variables
            $_SESSION['username'] = $username;
            $_SESSION['userID'] = $row['userID'];

            return true;
        }
    }

    function isLoggedIn() {
        if(isset($_SESSION['username'])) {
            return true;
        }

        else {
            return false;
        }
    }

    function redirect($url) {
        header("Location: $url");
    }

    function logout() {
        session_destroy();
        unset($_SESSION['username']);
    }
  }
?>

As you can see, the class.user.php calls a "config.php" file, which simply creates a new DATABASE and a new USER, using a link created from making a new DATABASE:

<?php
  // Turn on all error reporting
  ERROR_REPORTING(E_ALL);
  ini_set('display_errors', 1);

  // Start Session
  session_start();

  // Set error to false, and blank error message
  $error = "false";
  $errorMessage = "";

  // Include Database info
  require_once('class.database.php');
  $link = new DATABASE();


  // Include User info
  require_once('class.user.php');

  // Create instance for user class
  $activeUser = new USER($link);
?>

Now, I'd like to focus on my queries, because none of them are working, and I understand why. The query function is in the DATABASE class, but $this is pointing to the USER class.

My question is: How should I write my query so that it properly calls the DATABASE class.

Also, before anyone mentions it, yes, I know md5 is a no-no, but this is for a class project that will be using mock user data, and our professor said that md5 was sufficient encryption for this project

EDIT: For the sake of this, could we focus on the createNewUser function in class.user.php? That's the part I'm currently playing with.

17
  • Why don't you just inherit (extend) your Database class in your Users class? Commented Dec 2, 2016 at 6:06
  • Rohit, the correct formatting for that would be: class USER extend DATABASE, correct? Commented Dec 2, 2016 at 6:07
  • class USER extends DATABASE in that case you don't need this // Include Database info require_once('class.database.php'); $link = new DATABASE(); Commented Dec 2, 2016 at 6:09
  • Your login is not using the right connection, you should be using $this->myConn not $this->conn from what I can see. Commented Dec 2, 2016 at 6:10
  • 1
    Lastly your $this->dbConn needs to be returned in your getConnected() method. This is likely the main culprit as to why your connection doesn't work. Commented Dec 2, 2016 at 6:17

3 Answers 3

3

Why, not to make a database connection link oncе. Without using method getConnected everytime to make new connection to db?

And what is $this->connection in method close of class DATABASE, perhaps, it must be a connection link.

  class DATABASE
  {
    protected $dbConn; //connection link

    protected static $dbHost = "localhost";
    protected static $dbUser = "tysonmoyes";
    protected static $dbPassword = "F!lmtrepid";
    protected static $db = "tysonmoyes";

    public function __construct() {
        $this->getConnected();
    }

    public function getConnected() {

        //if connection link allready exists return it;
        if(isset($this->dbConn)) {
            return $this->dbConn;
        }

        $this->dbConn = new mysqli(self::$dbHost, self::$dbUser, self::$dbPassword, self::$db);
        return $this->dbConn;
    }

    function queryDB($queryString) {
        return mysqli_query($this->getConnected(), $queryString);
    }

    public function close() {
        mysqli_close($this->dbConn);
    }
  }
Sign up to request clarification or add additional context in comments.

2 Comments

The return in __construct - what is the point? I don't think it should be there
Ya, realy. @return statement must be in method getConnected (as it is called from __construct).
2

I think you forgot to return you db connection link.

Database Class:

<?php
  class DATABASE
  {
    public function __construct() {
        $this->getConnected();
    }

    public function getConnected() {
        $dbHost = "localhost";
        $dbUser = "tysonmoyes";
        $dbPassword = "F!lmtrepid";
        $db = "tysonmoyes";

        $dbConn = new mysqli($dbHost, $dbUser, $dbPassword, $db);
        $this->dbConn = $dbConn;
        return $dbConn;
    }

    function queryDB($queryString) {
        return mysqli_query($this->getConnected(), $queryString);
    }

    public function close() {
        mysqli_close($this->connection);
    }
  }
?>

Comments

2

As mentioned in my comment and as others mention, return the connection foremost.

I personally think PDO would be a better option because parameterizing is really easy, but you are probably supposed to use mysqli_ so I would rework your set up a bit. This is not tested, just take notes of differences:

/classes/Database.php

<?php
class Database
    {
        private static  $singleton,
                        $con;
        # I LIKE TO RETURN THE SAME INSTANCE OF A CLASS HERE, OPTIONAL
        public function __construct()
            {
                if(!(self::$singleton instanceof Database))
                    self::$singleton = $this;

                return self::$singleton;
            }
        # I LIKE TO STORE THE CONNECTIONS AND RETURN IT INSTEAD OF POSSIBLY
        # CREATING A NEW INSTANCE
        public function getConnected()
            {
                # IF THIS STATIC IS NOT A CONNECTION, MAKE ONE
                if(!(self::$con instanceof MySQLi))
                    self::$con = new mysqli(DB_HOST, DB_USER, DB_PASS, DB_NAME);
                # RETURN THE CONNECTION
                return self::$con;
            }

        public function query($sql)
            {
                return mysqli_query(self::$con, $sql);
            }

        public function close()
            {
                mysqli_close(self::$con);
            }
    }

/config.php

<?php
# I PERSONALLY LIKE TO USE A CONFIG (PREFS FILE) TO STORE MY DB INFO INCASE IT CHANGES
define('DB_HOST',"localhost");
define('DB_USER',"tysonmoyes");
define('DB_PASS',"F!lmtrepid");
define('DB_NAME',"tysonmoyes");
define('DS',DIRECTORY_SEPARATOR);
# I ALSO LIKE TO STORE PATH CONSTANTS SO IT'S AN EASY AND CONSISTANT WAY TO
# LOCATE/INCLUDE FILES
define('ROOT_DIR',__DIR__);
define('CLASS_DIR',ROOT_DIR.DS.'classes');
# START SESSION
session_start();
# USING AN AUTOLOADER IS A MUST ON CLASSES
spl_autoload_register(function($class) {
    if(class_exists($class))
        return;
    # SHOULD RETURN A PATH LIKE:
    # /var/www/domain/httpdocs/myproject/classes/Database.php
    # WHEN CALLING $Database = new Database();
    $path = str_replace(DS.DS,DS,CLASS_DIR.DS.str_replace("\\",DS,$class)).'.php';
    # Presuming the file is named properly (and I have done the path right),
    # it would add the class file for you without using include anywhere.
    if(is_file($path))
        include_once($path);
});

/classes/User.php

<?php
class User
    {
        private $conn;
        # I WOULD SET ALL USER INFO TO AN ARRAY INSTEAD OF IN SEPARATE VARIABLES
        private $userData = array();

    # I MIGHT HINT AT TYPE HERE
    public function __construct(\Database $conn)
        {
            $this->conn = $conn;
        }

    public function createNewUser($username, $password)
        {
            $username = trim($username);
            $password =  trim($password);

            // Encrypt password
            $password = password_hash($password);

            // Check if username already exists
            # SQL INJECTION ISSUE HERE, YOU NEED TO BIND PARAMS HERE
            $checkSQL = "SELECT * FROM users WHERE username = '$username'";
            $checkResult = $this->conn->query($checkSQL);

            if($checkResult->num_rows > 0) {
                $error = "true";
                $errorMessage = "This username has already been taken. Please try again";
            }
            else {
                # INJECTION ISSUE HERE
                $insertSQL = "INSERT INTO users(username, password) VALUES('$username', '$password')";
                //$insertResult = $this->conn->query($insertSQL);

                // Get the user ID
                $userID = $this->conn->getConnected()->insert_id;

                // Set the SESSION globals
                $_SESSION['username'] = $username;
                $_SESSION['userID'] = $userID;
            }
        }

        public function login($username, $password)
            {
                # YOU SHOULD NOT BE INJECTING HERE. I USE PDO, SO I WON'T
                # ATTEMPT A GOOD FIX HERE...BUT A FIX IS REQUIRED
                # YOU SHOULD ALSO NOT MATCH PASSWORD HERE, JUST USERNAME
                # USE password_verify() TO MATCH HASH
                $sql = "SELECT * FROM users WHERE username = '$username'";
                $result = $this->conn->query($sql);
                $row = $this->conn->getConnected()->fetch_array($result, MYSQL_ASSOC);

                # DO A CHECK FIRST THAT THERE IS A ROW RETURNED FOR USERNAME (NOT SHOWN IN MY EXAMPLE...)
                # DO THE MATCH HERE
                $valid = password_verify($_POST['password'],$row['password']);

                if($valid) {
                    // Set Session Variables
                    $_SESSION['username'] = $username;
                    $_SESSION['userID'] = $row['userID'];

                     return true;
                }
            }

        public function isLoggedIn()
            {
                if(isset($_SESSION['username'])) {
                    return true;
                }
                else {
                    return false;
                }
            }

        public function redirect($url)
            {
                header("Location: $url");
                # YOU SHOULD EXIT HERE
                exit;
            }

        public function logout()
            {
                session_destroy();
                unset($_SESSION['username']);
                # YOU SHOULD PROBABLY REDIRECT HERE TO REFRESH THE SESSION
            }
    }

/index.php

# INCLUDE THE CONFIG ON ALL PAGES
include(__DIR__.DIRECTORY_SEPARATOR.'config.php');

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

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.