Just looking for advice on how to improve my script

PHP_apprentice

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
//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

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

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

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 🙂