clean up / tidy

[Blackicer]

hi, i was wondering if there is anything i can do to clean up/tidy up this code...

[quote]<?PHP
require_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: $randpin


To 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<[email protected]>\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]

[trq]

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].

[emehrkay]

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 it

then id put the functions in their own file, functions.php and include that file

take 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