1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

PHP Critiuqe

Discussion in 'HTML, Graphics & Programming' started by unknowndomain, 8 Jul 2006.

  1. unknowndomain

    Mobster

    Joined: 18 Apr 2004

    Posts: 2,613

    Location: London

    Could you let me know if there are any improvments that can be made on this code that ive written...

    PHP:
    <?php

    /*

            +-------------------------------------------------+
            | USER MANAGER CLASS                              |
            |  by Tom Lynch                                   |
            +-------------------------------------------------+
            | This class will allow you to easily manage a    |
            | user base.                                      |
            +-------------------------------------------------+
            | MYSQL                                           |
            | =====                                           |
            | CREATE TABLE users (                            |
            | user_id INT NOT NULL AUTO_INCREMENT PRIMARY KEY,|
            | username VARCHAR(255) NOT NULL,                 |
            | password VARCHAR(255) NOT NULL,                 |
            | email VARCHAR(255) NOT NULL                     |
            | ) ENGINE = MYISAM;                              |
            |                                                 |
            | FUNCTIONS                                       |
            | =========                                       |
            | set_db_details - sets database details          |
            | db_connect - used internally to connect to db   |
            | login_user - logs in the user                   |
            | check_user - verifies username                  |
            | logout_user - logs out the user                 |
            | user_id - sets the user id to be used           |
            | user_name - sets the username to be used        |
            | user_details - sets the user details to be used |
            | add_user - adds user                            |
            | update_user - updates user details              |
            | remove_user - removes user                      |
            | get_user - gets user details                    |
            | list_users - gets a list of users               |
            | last_user_if - gets last user id                |
            | get_user_id_from_username - gets user id        |
            | get_user_id_from_email - gets user id           |
            |                                                 |
            | HOW TO: INITIALIZE CLASS                        |
            | =======================                         |
            | include_once("user.php");                       |
            | $manager = new usermanager;                     |
            | $manager->set_db_details($host, $username,      |
            | $password, $database, $table);                  |
            |                                                 |
            | HOW TO: CALL A FUNCTION                         |
            | =======================                         |
            | $output = $manager->function(var1, var2);       |
            |                                                 |
            | HOW TO: CHECK FOR ERRORS                        |
            | ========================                        |
            | if ($manager->error) {                          |
            |    echo "Error: " . $manager->error;             |
            | };                                              |
            +-------------------------------------------------+

    */

    class usermanager {

        
    // Specify variables
        
    var $db_host;                // Holds database hostname
        
    var $db_username;        // Holds database username
        
    var $db_password;        // Holds database password
        
    var $db_database;        // Holds database name
        
    var $db_user_table// Holds user table name
        
    var $user_id;                // Holds user id
        
    var $user_name;            // Holds username
        
    var $user_password;    // Holds password
        
    var $user_email;        // Holds email address
        
    var $error;                    // Holds error messages

        // Set database details
        
    function set_db_details($host$username$password$database$table) {

            
    $this->db_host $host;
            
    $this->db_username $username;
            
    $this->db_password $password;
            
    $this->db_database $database;
            
    $this->db_user_table $table;

        }

        
    // Connect to database
        
    function db_connect() {

            
    mysql_connect($this->db_host$this->db_username$this->db_password)
            or 
    $this->error "The connection to the host failed, please try again";

            
    mysql_select_db($this->db_database)
            or 
    $this->error "Unable to select database, please try again";

            return 
    $output;

        }

        
    // Login User
        
    function login_user() {

            
    $this->db_connect();
            
    $_SESSION['username'] = mysql_real_escape_string($this->user_name);
            
    $_SESSION['password'] = mysql_real_escape_string($this->user_password);

            
    session_regenerate_id()
            or 
    $this->error "There was a script error at login, you may no longer be logged in, please try again";

        }

        
    // Check to ensure user is valid
        
    function check_user() {

            
    $this->db_connect();

            
    $output 0;

            
    $query mysql_query("SELECT * FROM " $this->db_user_table " WHERE username = '" $_SESSION['username'] . "' && password = '" $_SESSION['password'] . "';")
            or 
    $this->error "There was an error searching the database, please try again";

            if (
    mysql_num_rows($query) > 0$output 1;

            return 
    $output;

        }

        
    // Logout User
        
    function logout_user() {

            
    session_unset();

        }

        
    // Get user details
        
    function user_id($user_id) {
            
    $this->db_connect();
            
    $this->user_id mysql_real_escape_string($user_id);
        }

        function 
    user_name($user_name) {
            
    $this->db_connect();
            
    $this->user_name mysql_real_escape_string($user_name);
        }

        function 
    user_details($user_name$user_password$user_email=0) {
            
    $this->db_connect();
            
    $this->user_name mysql_real_escape_string($user_name);
            
    $this->user_password md5(mysql_real_escape_string($user_password));
            
    $this->user_email mysql_real_escape_string($user_email);
        }

        
    // Perform administrative functions
        
    function add_user() {

            
    $this->db_connect();

            
    $query_user mysql_query("SELECT * FROM " $this->db_user_table " WHERE username = '" $this->user_name "';");
            
    $query_email mysql_query("SELECT * FROM " $this->db_user_table " WHERE email = '" $this->user_email "';");

            if (
    mysql_num_rows($query_user) > and mysql_num_rows($query_email) > 0) {
                
    $this->error "You must enter a unique username and email address";
            } elseif(
    mysql_num_rows($query_user) > 0) {
                
    $this->error "You must enter a unique username";
            } elseif(
    mysql_num_rows($query_email) > 0) {
                
    $this->error "You must enter a unique email address";
            } else {
                
    mysql_query("INSERT INTO " $this->db_user_table " (username, password, email) VALUES ('" $this->user_name "', '" $this->user_password ."', '" $this->user_email "');")
                or 
    $this->error "There was an error inserting the user into the table, please try again";
            };

            return 
    $output;

        }

        function 
    update_user() {

            
    $this->db_connect();

            
    $query_user mysql_query("SELECT * FROM " $this->db_user_table " WHERE username = '" $this->user_name "';");
            
    $query_email mysql_query("SELECT * FROM " $this->db_user_table " WHERE email = '" $this->user_email "';");

            if (
    mysql_num_rows($query_user) > and mysql_num_rows($query_email) > 0) {
                
    $this->error "You must enter a unique username and email address";
            } elseif(
    mysql_num_rows($query_user) > 0) {
                
    $this->error "You must enter a unique username";
            } elseif(
    mysql_num_rows($query_email) > 0) {
                
    $this->error "You must enter a unique email address";
            } else {
                
    mysql_query("UPDATE " $this->db_user_table " SET username = '" $this->user_name "', password = '" $this->user_password ."', email = '" $this->user_email "' WHERE user_id = '" $this->user_id "';")
                or 
    $this->error "There was an error updating the user, please try again";
            };

            return 
    $output;

        }

        function 
    remove_user() {

            
    $this->db_connect();

            
    mysql_query("DELETE FROM " $this->db_user_table " WHERE user_id = '" $this->user_id "';")
            or 
    $this->error "There was an error removing the user from the database, please try again";

            return 
    $output;

        }

        function 
    get_user() {

            
    $this->db_connect();

            
    $query mysql_query("SELECT * FROM " $this->db_user_table " WHERE user_id = '" $this->user_id "';")
            or 
    $this->error "There was an error searching the database, please try again";

            
    $data mysql_fetch_array($query) or
            
    $output $this->error "There was an error fetching the data from the database, please try again";

            
    $output['id'] = $data['user_id'];
            
    $output['username'] = $data['username'];
            
    $output['email'] = $data['email'];

            return 
    $output;

        }

        function 
    list_users() {

            
    $this->db_connect();

            
    $query mysql_query("SELECT user_id, username, email FROM " $this->db_user_table ";")
            or 
    $this->error "There was an error searching the database, please try again";

            
    $count 0;

            while (
    $count != mysql_num_rows($query)) {
                
    $output[$count] = mysql_fetch_array($query);
                
    $count += 1;
            }

            return 
    $output;

        }

        function 
    last_user_id() {

            
    $this->db_connect();

            
    $output mysql_insert_id()
            or 
    $this->error "There was an error retriving the previous user id, please try again";

            return 
    $output;

        }

        
    // Database searching
        
    function get_user_id_from_username() {

            
    $this->db_connect();

            
    $query mysql_query("SELECT user_id FROM " $this->db_user_table " WHERE username = '" $this->user_name "';")
            or 
    $this->error "There was an error searching the database, please try again";


            
    $data mysql_fetch_row($query);

            
    $output $data[0];

            return 
    $output;

        }

        function 
    get_user_id_from_email() {

            
    $this->db_connect();

            
    $query mysql_query("SELECT user_id FROM " $this->db_user_table " WHERE email = '" $this->user_email "';")
            or 
    $this->error "There was an error searching the database, please try again";

            
    $data mysql_fetch_row($query);

            
    $output $data[0];

            return 
    $output;

        }

    }

    ?>
     
  2. robc123

    Gangster

    Joined: 24 Jun 2005

    Posts: 249

    Break the code out into different Classes. I dont understand why you have all the database code in with the functionality?

    Perhaps:

    - Functionality
    - Security
    - Database
     
  3. Dj_Jestar

    Caporegime

    Joined: 18 Oct 2002

    Posts: 29,080

    Location: Back in East London

    As above, seperate the logic from the data layer.. though I'm not quite sure where 'security' comes into it as I don't see any there..

    Code:
    <?php
    
    class UserManager
    {
        //vars'
        var $db;
    
        function UserManager ()
        {
            $this->db = new DataBaseWrapper();
        }
    }
    
    ?>
    The use the db object internally to the UserManager object.
     
    Last edited: 8 Jul 2006
  4. robc123

    Gangster

    Joined: 24 Jun 2005

    Posts: 249

    This is an interesting post really and something which even though I provided suggestions that I dont fully understand.

    I havent yet read something about object orientation to this effect but what is the best way of breaking a system down into classes in regards to a full database system. For example if you take an address book; do you just have 3 classes?

    - User Interface
    - Address
    - Database

    Does anyone know any good articles on this?
     
  5. akakjs

    Wise Guy

    Joined: 18 Oct 2002

    Posts: 1,044

    Just a quick note, I can't see any mention of hashing user passwords (my php sucks though); you should never store passwords a plain text in any data system.

    akakjs
     
  6. robc123

    Gangster

    Joined: 24 Jun 2005

    Posts: 249

    PHP:
    function user_details($user_name$user_password$user_email=0) { 
            
    $this->db_connect(); 
            
    $this->user_name mysql_real_escape_string($user_name); 
            
    $this->user_password md5(mysql_real_escape_string($user_password)); 
            
    $this->user_email mysql_real_escape_string($user_email); 
        } 

    Hashing like md5?
     
  7. Dj_Jestar

    Caporegime

    Joined: 18 Oct 2002

    Posts: 29,080

    Location: Back in East London

    Need to look up on Patterns, don't expect to get it instantly, it's something that is forever evolving and has been since the 1950's (The "Patterns" concept was originally designed by Architects, then later adopted for use in prgramming)

    Wiki
     
  8. robmiller

    Capodecina

    Joined: 26 Dec 2003

    Posts: 16,522

    Location: London


    Most languages have frameworks that help you break things down. MVC is the most popular, which breaks things into the Model, View and Controller layers for easy separation of business, presentation and event logic. See here for more details.

    A good example of this separation would be using something like Smarty and then writing an API for your business logic. So you'd have a page that ended up looking like this in its most simplest form:

    Code:
    $id = intval($_GET['id']);
    
    $post = $post::from_id($id);
    
    $smarty->assign('post', $post);
    $smarty->display('post.tpl');
    
    The business logic is separate, in the $post class, and the view logic is separate still, in the Smarty template which we're parsing.
     
  9. akakjs

    Wise Guy

    Joined: 18 Oct 2002

    Posts: 1,044

    fair enough, missed that :D

    I hate this blue on blue code box thing :p I have enough difficulty reading php as it is...

    akakjs
     
  10. robc123

    Gangster

    Joined: 24 Jun 2005

    Posts: 249

    I know what you mean though about the hashing, I expected there to be more as well. I imagine most of it must be done before passing it to the class
     
  11. unknowndomain

    Mobster

    Joined: 18 Apr 2004

    Posts: 2,613

    Location: London

    How do you mean? if used correctly the only time u would pass a password into the flass is through this function so it will always be md5'd how much more do you need?