Jump to content


Photo

Improving this script...


  • Please log in to reply
3 replies to this topic

#1 Drezard

Drezard
  • Members
  • PipPipPip
  • Advanced Member
  • 244 posts

Posted 12 September 2006 - 11:51 AM

Hello, I want to know some ways i could improve this script (make it faster, take up less bandwidth, more secure, things like that). This is my first good script. I want to make it 99% perfect though so, I need the pro's perspective. Thanks so much.

<html>
<head>
<?php
include('header.php');
?>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>

<body>
<?php
if (!isset($_POST['submit'])) { 	// if the form has not been submitted before
				?>

	<form action="<?=$_SERVER['PHP_SELF']?>" method="post">
    Name(s): <input type="text" name="name"> <br>
    Email: <input type="text" name="email1"> 
    @<input type="text" name="email2">
   	Daytime Phone Number:<input type="text" name="daytime"> <br> 
    Nighttime Phone Number:<input type="text" name="nighttime"> <br>
	Mobile Number:<input type="text" name="mobile"> <hr>
    Street Number:<input type="text" name="streetnum"> 
    Street Name: <input type="text" name="streetname"> <br>
    Suburb: <input type="text" name="suburb">
    State: <input type="text" name="state"> 
    Postcode: <input type="text" name="postcode"> <br>
    Asking Price: <input type="text" name="askprice"> 
    Bedrooms: <input type="text" name="bedrooms">
    Bathrooms: <input type="text" name="bathrooms"> <hr>  
	Desired Username: <input type ="text" name="user"> <br>
	Password: <input type="password" name="pass1">
	Retype Password: <input type="password" name="pass2">                                   
    <input type="submit" name="submit">
	</form>

	          <?php
	}
else {
	include('connect.php');			
	$pass1 = empty($_POST['pass1']) ? die ("Please Enter A Password") : mysql_escape_string($_POST['pass1']);
	$pass2 = empty($_POST['pass2']) ? die ("Please Retype Your Password") : mysql_escape_string($_POST['pass2']);
	$user = empty($_POST['user']) ? die ("Please Enter A Username") : mysql_escape_string($_POST['user']);
	$sql = "SELECT * FROM users WHERE user='$user'";
    $result = mysql_query($sql);

  	// Mysql_num_row is counting table ro
	$count=mysql_num_rows($result);

   	// If result matched $user and $pass, table row must be 1 row

		if($count==1){

		echo "Username already exists";	

		}
		if ($count == 0) {
				
		if ( $pass1 == $pass2 ) {
			
			$expire = strtotime('next month',$date);										// Start a connection to the MySQL Database 									
  			$name = empty($_POST['name']) ? die ("Please Enter Your Name") : mysql_escape_string($_POST['name']);
 			$email1 = empty($_POST['email1']) ? die ("Please Enter Your Email Address") : mysql_escape_string($_POST['email1']);
    		$email2 = empty($_POST['email2']) ? die ("Please Enter Your Email Address") : mysql_escape_string($_POST['email2']);
    		$daytime = empty($_POST['daytime']) ? die ("Please Enter Your Daytime Phone Number") : mysql_escape_string($_POST['daytime']);
    		$nighttime = empty($_POST['nighttime']) ? die ("Please Enter Your Nighttime Phone Number") : mysql_escape_string($_POST['nighttime']);
    		$mobile = empty($_POST['mobile']) ? die ("Please Enter Your Mobile Number") : mysql_escape_string($_POST['mobile']);
			$streetnum = empty($_POST['streetnum']) ? die ("Please Enter Your Streetnumber") : mysql_escape_string($_POST['streetnum']);
			$streetname = empty($_POST['streetname']) ? die ("Please Enter Your Streetname") : mysql_escape_string($_POST['streetname']); 
			$suburb = empty($_POST['suburb']) ? die ("Please Enter Your Suburb") : mysql_escape_string($_POST['suburb']);
			$state = empty($_POST['state']) ? die ("Please Enter Your State") : mysql_escape_string($_POST['state']); 
			$postcode = empty($_POST['postcode']) ? die ("Please Enter Your Postcode") : mysql_escape_string($_POST['postcode']); 
			$askprice = empty($_POST['askprice']) ? die ("Please Enter Your Asking Price") : mysql_escape_string($_POST['askprice']);
			$bedrooms = empty($_POST['bedrooms']) ? die ("Please Enter How Many Bedrooms Your House Has") : mysql_escape_string($_POST['bedrooms']); 		 				       
			$bathrooms = empty($_POST['bathrooms']) ? die ("Please Enter How Many Bathrooms Your House Has") : mysql_escape_string($_POST['bathrooms']); 
			$joindate = date(dmY);
			$activationID = rand(10000, 99999);
			$activated = 0;
			$to = "$email1@$email2";
			$subject = "Sellyourownhome.com.au activate your account";
			$message = "Welcome to Sell your own home The easiest and cheapest way to sell your own home <br> <br> <a href=\"localhost/activate.php\"> <br> <br> Thanks";
			$headers = "Help@sellyourownhome.com.au";
	 
// mail () function will go here...

			/* The above code is to check whether the user has or has not filled in all of the form. This means the user is forced to fill out the whole
			form instead of just half of it. This also puts all of the information from the form into Variables so we can use them later. This code also
			emptys all of the posts, so that it frees up memory so we can do other things faster. */	
			if(!empty($_POST)){
						     										// This code checks to see if the form has been emptied.
       			$query = "INSERT INTO  users (name, email1, email2, daytime, nighttime, mobile, streetnum, streetname, suburb, state, postcode, askprice, bedrooms, bathrooms, joindate, expiredate, activationID, activated) VALUES ('$name', '$email1', '$email2', '$daytime', '$nighttime', '$mobile', '$streetnum', '$streetname', '$suburb', '$state', '$postcode', '$askprice', '$bedrooms', '$bathrooms', '$joindate', '$expire', '$activationID', '$activated')";
/* This is the query, this is what we are asking the MySQL database to do. This specific query asks the database to add the Variables we took
out of that form into the database. So we can later use those variables in different scripts. We can also use them to store user information such
as: how much money a user has. */  
 				$result = mysql_query($query) or die ("User already exists");     
/* This code above executes the query. */
    			echo "Account Created";  // This is the message the user gets if the process completed and their account was sucess fully added.
				mysql_close($connection);
/* Finally we close the MySQL connection that we opened using Connect.php. This is the end of the hard bit of the script. Below is the tying up
of loss ends. The loss ends of else { and if { statements. */
			}
	} else { 
	echo "Your passwords dont match";
}
}
}



?>
</body>
</html>

- Cheers, Daniel


#2 ober

ober
  • Staff Alumni
  • Advanced Member
  • 5,337 posts
  • LocationEast Coast, USA

Posted 12 September 2006 - 02:27 PM

Ok, a few things:

1) I don't trust the empty() function.  Some people do, it's a personal preference.  There are other, better ways to compare strings to null or nothing that will ALWAYS give you a valid response.  empty() will not.

2) I try to avoid using die() and exit() as much as possible.  You should be able to trap errors and provide a much cleaner way of handling errors than simply dumping out of your script.  It takes some extra work, but it can be done.

3) if ($count == 0) {

if ( $pass1 == $pass2 ) {

This is unnecessary.  These can be combined into one if statement:
if ($count == 0 && $pass1 == $pass2 ) {

Info: PHP Manual


#3 obsidian

obsidian
  • Staff Alumni
  • Advanced Member
  • 3,202 posts
  • LocationSeattle, WA

Posted 12 September 2006 - 02:29 PM

i'll expand a tad on ober's empty() comment: i would restrict use of empty() to simply see whether or not a String contains any value. for instance, empty() will return false if the string contains NULL, "", or "    ".
You can't win, you can't lose, you can't break even... you can't even get out of the game.

<?php
while (count($life->getQuestions()) > 0)
{   $life->study(); } ?>
  LINKS: PHP: Manual MySQL: Manual PostgreSQL: Manual (X)HTML: Validate It! CSS: A List Apart | IE bug fixes | Zen Garden | Validate It! JavaScript: Reference Cards RegEx: Everything RegEx

#4 Jenk

Jenk
  • Members
  • PipPipPip
  • Advanced Member
  • 778 posts

Posted 12 September 2006 - 02:32 PM

empty(0) === true, too.




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users