Just looking for advice on how to improve my script

PHP_apprentice · 11-04-2003, 10:57 PM

This is a script that I use to handle some entry forms and i was wondering if it is efficient and if i could turn it into a function that I could just call when needed.

PHP Code:


			


<?php

//Begin user session

session_start();

header("Cache-control: private"); //IE 6 Fix 

//End Session information

//Include Section

require('db/db_connect.php');

require('funtions/ecape_data.php');



?>

<html>

<head>

    <title>DMP | Welcome!</title>

</head>

</html>



<?php





if (isset($_POST['submit'])) { //handle the form

    

if (empty($_POST['user_id']) && empty($_POST['pass_word'])){//check fields for data

        echo'Please Login! If you are experiencing problems contact<br>a system administrator <a href="mailto:dev@directmailpartners.com"></a> .  Thank you!<br><br>';

        echo'<a href="login.php">Return to Login!</a>';

        exit();

    }else{//assign variables

        $u = escape_data($_POST['user_id']);

        $p = escape_data($_POST['pass_word']);

        $_SESSION['user_id'] = $u;

    

        $query = "SELECT * FROM employee WHERE user_id = '$u' and pass = '$p'";

        $result = mysql_query($query, $db);

        $count_result = mysql_num_rows($result);

    

    //Verfiy login information

    if ($count_result > 0 ){//start

        $query = "SELECT fname FROM employee WHERE user_id = '$u'";

        $name = mysql_query($query, $db);

        $fname = mysql_fetch_array($name);

        $_SESSION['fname'] = $fname['fname'];

    

    

    

    //Information Displayed @ successful login

    echo'<table align="center"><tr><td><a href="edit.php">Edit Profile</a></td>';

    echo'<td><a href="inv_truck.php">Enter New Inventory</a></td>';

    echo'<td><a href="check_out.php">Check Out Inventory</a></td>';

    echo'<td><a href="check_in.php">Check In Inventory</a></td>';

    

        //Check for Admin level access

        $query = "SELECT level FROM employee WHERE user_id = '$u'";

        $result = mysql_query($query, $db);

        $level = mysql_fetch_array($result);

        $admin = $level["level"];

    

    if ( $admin >= 4 ){ //start

        echo'<td><a href="admin.php">Admin</a><br></td>';

        echo'<td><a href="job/job_new.php">Enter New Job</a><br></td>';

        echo'<td><a href="job/job_out.php">Finish Job</a><br></td>';

        echo'<td><a href="page2.php">Check Inventory<br></a></td></tr>';

        echo'<tr><td colspan = 6 align-"center"><br><br>Welcome to the members site&nbsp;', $_SESSION['fname'], '<br><br>';

        echo'You have logged in successfully<br></td></tr>';

        exit();

        }else{

            echo'</tr><tr><td colspan = 4 align="center"><br><br>Welcome to the members site&nbsp;', $_SESSION['fname'], '<br><br>';

        echo'You have logged in successfully<br></td></tr></table>';

        }//End

    

    

    }Else{

        echo'Please check to make sure you have the correct username and password!!<br>';

        echo'<a href="login.php">Return to Login</a>';

    }//end of user_id and pass check

        

    

    

    }//verify form was filled in!!!

    

    

    

    

    }//End of Form handle

            

?>

illusion · 11-06-2003, 07:24 AM

For a better practice, I'd suggest you take out all the logics from your HTML codes. E.g

function isUserExists($username, $password) {
//logics
return true/false;
}

function isAdmin($username){
//logics
return true/false;
}

if (!isUserExists($user_id, $pass_word)) {
die ("<h1>You're not a user!</h1>");
} else {
......
}

jacrewq · 11-16-2003, 11:31 AM

php.net/echo

echo <<<END
This uses the "here document" syntax to output
multiple lines with $variable interpolation. Note
that the here document terminator must appear on a
line with just a semicolon no extra whitespace!
END;

Shrike · 11-17-2003, 12:51 PM

I can't see how the if/else blocks fit together because theres no indenting, but you seem to run 3 queries when the first one gets * all fields for user_id/pass

my2c