barney0o0 Posted March 4, 2008 Share Posted March 4, 2008 Hi Folks...i hope you can hold my hand with this one. Ive taken bits and bobs from tuts and i cant get my registration form to function correctly. So far i can enter information successfully to the database, however if i enter a duplicate 'username', i add another entry to the database rather than creating an error message.... Can anyone spot the obvious blunder?... many thanks in advance <?php if(isset($_POST['formSent'])) { $db = '*'; $dbhost = '*'; $dbuser = '*'; $dbpass = '*'; mysql_connect($dbhost, $dbuser, $dbpass)or die("could not connect to database"); mysql_select_db($db); $name = mysql_real_escape_string($_POST['name']); $email = mysql_real_escape_string($_POST['email']); $age = (int)$_POST['age']; $username = mysql_real_escape_string($_POST['username']); $password = md5($_POST ['password']); $checkuser = mysql_query("SELECT username FROM regusers WHERE username=$username"); $username_exist = mysql_num_rows($checkuser); if($username_exist > 0){ echo "Name Taken."; unset($username); include 'form.php'; exit(); } else { $query = "INSERT INTO regusers (userid, email, age, username, password) VALUES('', '$email', '$age', '$username', '$password')"; if (@mysql_query($query)) { echo '<p>Account Created</p>'; } else { echo '<p>error creating user account: ' . mysql_error() . '</p>'; } } } ?> and the form,, <form name="reg" method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>"> Quote Link to comment Share on other sites More sharing options...
Agricola Posted March 4, 2008 Share Posted March 4, 2008 In the following line $checkuser = mysql_query("SELECT username FROM regusers WHERE username=$username"); you need to have $username in single quotation marks like this, it also will not hurt to limit the query to 1 especailly if you have a huge database. try this. $checkuser = mysql_query("SELECT username FROM regusers WHERE username='$username' LIMIT 1 "); Quote Link to comment Share on other sites More sharing options...
barney0o0 Posted March 5, 2008 Author Share Posted March 5, 2008 Thanks for your response... Ive changed the form quite a bit (added new fields, checked the fields, ensured that passwords match and that the username isnt taken).When i complete the form in full, it works fine, HOWEVER if i dont fill in a form, or passwords dont match etc the webpage goes crazy!, i.e. it loops through the errors messages (and the actual webpage) until my browser crashes! So obvioulsy, at the end of a check, im calling the wrong function. Ive had a look around, but im after something that would display the error messages above the form. I hope you can help, so near yet so far for a newb <form name="reg" method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>"> <fieldset> <legend>Create Your Personal Account</legend> <p>Field marked * are required.</p> <?php include("connection.php"); if(isset($_POST['submit'])) { if(empty($_POST['username']) || empty($_POST['password']) || empty($_POST['password2']) ||empty($_POST['emailad']) || empty($_POST['firstname']) || empty($_POST['surname'])) { // Display the error message. echo "Please completed all required form fields"; // Redirect to the form. include 'mailinglist.php'; // Stop the code to prevent the code running after redirecting. exit(); } $username = $_POST['username']; $password = $_POST['password']; $password2 = $_POST['password2']; $email = $_POST['email']; $firstname = $_POST['firstname']; $surname = $_POST['surname']; $emailed = $_POST['emailed']; $nostreet = $_POST['nostreet']; $town = $_POST['town']; $city = $_POST['city']; $postcode = $_POST['postcode']; $updates = $_POST['updates']; if($password != $password2) { echo "Sorry, your passwords are not identical."; include 'mailinglist.php'; exit(); } $password = md5($password); $db = '*'; $dbhost = '*'; $dbuser = '*'; $dbpass = '*'; mysql_connect($dbhost, $dbuser, $dbpass)or die("could not connect to database"); mysql_select_db($db); $checkuser = mysql_query("SELECT username FROM regusers WHERE username='$username' LIMIT 1 "); $username_exist = mysql_num_rows($checkuser); if($username_exist > 0){ echo "Sorry that username you shave been taken. Please try another."; unset($username); exit; } else { $query = "INSERT INTO regusers (id, username, password, emailed, firstname, surname, nostreet, town, city, postcode, updates ) VALUES('', '$username', '$password','$emailed','$firstname','$surname','$nostreet','$town','$city', '$postcode', '$updates')"; if (@mysql_query($query)) { echo '<p>your user account has been created, and you may login.</p>'; } else { echo '<p>error creating user account: ' . mysql_error() . '</p>'; } } } ?> Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted March 5, 2008 Share Posted March 5, 2008 I don't know if it's the forums doing it but your code indentation is all over the place and it really makes it difficult to follow your program's flow of execution. You're new so you don't have any yet, but you really must give yourself a set of guidelines when you program to make your code more readable. Here are a few I set for myself: 1) No logical block of code should be larger than a single screen height if you can help it. This implies that you should use functions where they make sense. 2) No line of code should be wider than 80 characters. 3) Every time I open a code block, with a curly brace, I indent two spaces. I keep my indentation of two spaces until I close the code block with the matching curly brace. I've developed quite a few more over the years which I won't take the time to type here, but having a systematic approach to programming will greatly reduce the amount of errors you introduce into your code. Quote Link to comment Share on other sites More sharing options...
Agricola Posted March 5, 2008 Share Posted March 5, 2008 One other tip to make life easier, label your close brackets if you can not identify quickly to which opening bracket they belong. If you nest loops you definitly need to label close brackets. just look at bottom of your code, 3 close curly braces and no idea what they close. }//end if $_POST[username] check }//end if $_POST[submit] etc Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted March 5, 2008 Share Posted March 5, 2008 @agricola I've seen quite a few pieces of source code that do the same thing. In the end, and this is my opinion, they appear cluttered and no easier to read or maintain. My recommendation is that if your code blocks are getting so big you find that necessary, then it is time to take the body and put it in a function. The following example is self-documenting and clean. <?php // Validate all of our items while($item = getSomeItemFromAList()){ if(isValidItem($item)){ processItem($item); }else{ promptUserToFixItem($item); } } ?> The following is not: <?php // Validate all of our items while($item = getSomeItemFromAList()){ if(isValidItem($item)){ // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! }else{ // ITEM IS INVALID // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! // here is some php code yippeeeeeeeeeeeee!!! } // END ITEM VALID CHECK } // end item loop ?> Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.