View Full Version : better code review? please

10-20-2013, 01:00 PM
Hi All,
I'm trying to making this code more efficient, ideas please!
Thanks in advance!

if ((isset($_POST["brofrm"])) && ($_POST["brofrm"] == "form1")) {

if ($_POST['thename'] == '' || $_POST['email'] == '' || $_POST['company'] == '' || $_POST['method'] == '' || $_POST['address'] == '' || $_POST['tel'] == '' || $_POST['brochure_type'] == '' ){

// echo errors if above is true
$mess .= "Please complete the form below, <strong>*</strong> important!<br>";

if (empty($_POST['thename'])) {$mess .="You did not supply a name.<br>"; }

if (empty($_POST['email'])) {$mess .="You did not supply an email address.<br>"; }

if (empty($_POST['method'])) {$mess .="You did not specify which format you would prefer to receive the brochure, we offer a physical paperback copy or an electronic copy. We need this to respond to your request.<br>"; }

if (empty($_POST['address'])) {$mess .="You did not supply an address. We need this to respond to your request. Please go back and complete the necessary field(s).<br>"; }

if (empty($_POST['tel'])) {$mess .="You did not supply an telephone number. We need this to respond to your request. Please go back and complete the necessary field(s).<br>"; }

if (empty($_POST['brochure_type'])) {$mess .="You did not specify which brochures you wanted to receive.<br>"; }


// add variables
if (isset($_POST['thename'])) {$name=$_POST['thename'];}
if (isset($_POST['email'])) {$email=$_POST['email'];}
if (isset($_POST['company'])) {$company=$_POST['company'];}
if (isset($_POST['method'])) {$method=$_POST['method'];}
if (isset($_POST['address'])) {$address=$_POST['address'];}
if (isset($_POST['tel'])) {$tel=$_POST['tel'];}
if (isset($_POST['brochure_type'])) {$brochure_type=$_POST['brochure_type[]'];}

// get checkbox items
if (is_array($_POST['brochure_type'])) {
$brochure_type = implode(', ', $_POST['brochure_type']);}

// message
$messge="<b><u>Name:</u></b> ".$_POST['thename']."<br /><b><u>Method:</u></b> ".$_POST['method']."<br /><b><u>Company:</u></b> ".$_POST['company']."<br /><b><u>Address:</u></b> ".$_POST['address']."<br /><b><u>Telephone No:</u></b> ".$_POST['tel']."<br /><b><u>Email Address:</u></b> ".$_POST['email']."<br /><b><u>Selected Brochures:</u></b> ".$brochure_type;


echo $messge;
echo "<br>";
echo "Your message has been successfully sent. <a href=\"../../index.php\" title=\"Return to our homepage\">Return To Homepage</a><br />You will receive the brochure(s) you requested as soon as possible.</p>";
$mess = " ";

<!doctype html>
<meta charset="utf-8">
<title>Untitled Document</title>


<p style="text-align:center;"><?php if(!empty($mess)){echo '<em><strong>'.$mess.'</em></strong>';?><br><a href="javascript:history.go(-1)">&laquo; Go Back</a></p>

<?php }else{?>
<form action="brochure.php" method="post" id="frmsubmit" name="frmsubmit">
<table class="contacts" style="width:600px;">
<td width="300"><p style="font-weight:bold;margin:5px;">*Name:</p></td>
<td width="300"><input name='thename' type='text' id="thename" value="<?php echo((isset($_POST["thename"]))?trim(stripslashes($_POST["thename"])):""); ?>" size='40' /></td>
<td width="300"><p style="font-weight:bold;margin:5px;">*Format:</p></td>
<td width="300"><select name='method' id="method" style="width:267px;" title="<?php echo $_POST['method']; ?>">
<option value='Hard Copy'>Hard Copy</option><option value='Electronic Copy'>Electronic Copy</option>
<td width="300"><p style="font-weight:bold;margin:5px;">Company:</p></td>
<td width="300"><input name='company' type='text' id="company" value="<?php echo((isset($_POST["company"]))?trim(stripslashes($_POST["company"])):""); ?>" size='40' /></td>
<td width="300" valign="top"><p style="font-weight:bold;margin:5px;">*Address:</p></td>
<td width="300"><textarea name='address' cols='21' rows='5' id="address" style='resize:none;width:260px;'><?php echo((isset($_POST["address"]))?trim(stripslashes($_POST["address"])):""); ?></textarea></td>
<td width="300"><p style="font-weight:bold;margin:5px;">*Telephone:</p></td>
<td width="300"><input name='tel' type='text' id="tel" value="<?php echo((isset($_POST["tel"]))?trim(stripslashes($_POST["tel"])):""); ?>" size='40' /></td>
<td width="300"><p style="font-weight:bold;margin:5px;">*Email:</p></td>
<td width="300"><input name='email' type='text' id="email" value="<?php echo((isset($_POST["email"]))?trim(stripslashes($_POST["email"])):""); ?>" size='40' /></td>
<td><p style="font-weight:bold;margin:5px;">*Brochures:</p></td>
<td><p>Please select which brochure(s) you would like a copy of:<br /><br /><input <?php if (!(strcmp($_POST['brochure_type[]'],"1"))) {echo "checked=\"checked\"";} ?> name='brochure_type[]' type='checkbox' id="brochure_type[]" value='Brochure 1' /> Brochure 1<br />
<input <?php if (!(strcmp($_POST['brochure_type[]'],"1"))) {echo "checked=\"checked\"";} ?> name='brochure_type[]' type='checkbox' id="brochure_type[]" value='Brochure 2' /> Brochure 2</p></td>
<td colspan="2">
<input name="brofrm" type="hidden" id="brofrm" value="form1">
<td><input type='submit' value='Submit' /><input type='reset' value='Clear' /></td>
</body><?php }?>

10-29-2013, 07:34 AM
you should have syntax errors. fix 'em.

10-29-2013, 07:45 AM
also, you can tell a checkbox is on only by its existence using isset(), and you should read up on the articles referring to form element arrays. read this: http://php.net/manual/en/reserved.variables.post.php
and look in the comments for examples of how to iterate across the array of checkbox elements returned. you must first know how many there are, because if they are all unchecked, you will have all undefined elements. in fact the $_POST array may only contain the ones that are checked.
I learned this the hard way from testing.

10-29-2013, 07:57 AM
Thanks 4 your reply, will do!