Blackicer Posted September 7, 2006 Share Posted September 7, 2006 hi, i was wondering if there is anything i can do to clean up/tidy up this code...[quote]<?PHPrequire_once 'includes/config.php';$reg_error = '';//If user has submitted form...if($_POST[submit]=="Register") { //Define lnonly() - change. function lnonly($string) { $eregi = eregi_replace("([A-Z0-9]+)","",$string); if(empty($eregi)) { return true; } return false; } //error checking - Outsource to OOP // check that each required field has information in it if((!$_POST['username']) || (!$_POST['password1']) || (!$_POST['password2']) || (!$_POST['email']) || (!$_POST['hometown'])){ $reg_error = 'You did not submit the following required information:<br />'; if(!$_POST['username']){ $reg_error .= 'Username.<br />'; } if((!$_POST['password1']) || (!$_POST['password2'])){ $reg_error .= 'Password (both fields).<br />'; } if(!$_POST['email']){ $reg_error .= 'Email Address.<br />'; } if(!$_POST['hometown']){ $reg_error .= 'Hometown.<br />'; } $reg_error .= '<br />'; } // check acceptance of TOS if(!$_POST["agreed"]=="true") { $reg_error .= 'You must agree to the TOS!<br /><br />'; } //check against existing users. $email_address = no_inject($_POST['email']); $username = no_inject($_POST['username']); $sql_email_check = mysql_query("SELECT email FROM users WHERE email='$email_address'"); $sql_username_check = mysql_query("SELECT username FROM users WHERE username='$username'"); $email_check = mysql_num_rows($sql_email_check); $username_check = mysql_num_rows($sql_username_check); if(($email_check > 0) || ($username_check > 0)){ $reg_error .= 'Please fix the following errors:<br />'; if($email_check > 0){ $reg_error .= 'Your email address has already been used by another member in our database.<br /> If you have already registered and wish to recover your password, please go to reset your pass <a href=\"#\">here</a>.<br /> Otherwise, please try a different Email address.<br />'; unset($_POST['email']); } if($username_check > 0){ $reg_error .= 'The username you have selected has already been used by another member in our database.<br /> Please choose a different Username.<br />'; unset($_POST['username']); } } // Check passwords match if($_POST["password1"]!=$_POST["password2"]){ $reg_error .= 'Your passwords do not match!<br />'; } // Check email validity function CheckEmail($Email = "") { if (ereg("[[:alnum:]]+@[[:alnum:]]+\.[[:alnum:]]+", $Email)) { return true; } else { return false; } } if(!CheckEmail($_POST["email"])) { $reg_error .= 'Your email appears to be invalid!<br />'; } // Check location validity include_once 'includes/locations.php'; $num = count($locations); if($_POST["location"]>=$num OR $_POST["location"]<0) { $reg_error .= 'Your chosen location is invalid. Try another location!<br />'; } // Act on error checking results if($reg_error == '') { $location = no_inject($_POST["hometown"]); $password = md5(no_inject($_POST["password1"])); $email = no_inject($_POST["email"]); $date = date("d/m/Y H:i:s"); $randpin = rand(1111111,9999999); $ip_addy = GetHostByName($REMOTE_ADDR); $query = "INSERT INTO users (username, password, email, location, joined, pin) VALUES ('$username','$password','$email','$location','$date','$randpin')"; mysql_query($query) or die(mysql_error($query)); // send activation email $userid = mysql_insert_id(); $activationLink = "http://" . $_SERVER['HTTP_HOST'] . "/activate1.php?action=activate&uid=$userid"; $mailto = $_POST['email']; $mailsubj = "Account information for $_POST[username]"; $mailmess = "Thank you $_POST[username], for registering at The Cartel Project.Your account information is as follows:Username: $_POST[username]Password: $_POST[password1]Pin Number: $randpinTo activate your account, please click on the following link, or copy and paste the text into your browser:$activationLink"; if(mail($mailto, $mailsubj, $mailmess,"From: The Cartel Project<noreply@thecartelproject.com>\nX-Mailer: PHP")) { $reg_message = "Your membership information has been emailed to the address supplied: $mailto. Please check it and follow the directions."; } } //end if no errors} //end if submitted form?>[/quote] Quote Link to comment Share on other sites More sharing options...
trq Posted September 7, 2006 Share Posted September 7, 2006 Why would you construct strings over two lines? eg;[code=php:0] $reg_error .= 'Username.';[/code]I mean how you format your code is your bussiness, but IMO that is real [i]untidy[/i]. Quote Link to comment Share on other sites More sharing options...
emehrkay Posted September 7, 2006 Share Posted September 7, 2006 i would define all of the post vars first$username = $_POST['username']; etc...then probably create an assocative array then foreach through that to build the error query$postvals = array('Username'=>$username, 'Password Field 1'=>...foreach($posvals as $key => $vals){if($vals == "") $error .= "<li>".$key ." is empty</li>";}echo "<ul>".$error."</ul>"; or however you'd want to handle itthen id put the functions in their own file, functions.php and include that filetake your email part out, make it a function where you pass a few vars to - or even a class!that would just save some space, probably execute a lil faster too 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.