is this code ok?

[lukeurtnowski]

Does this php statement make sence?

<?php 
//If the form is submitted
if(isset($_POST['submitted'])) {
    //Check to make sure that the name field is not empty
    if(trim($_POST['contactName']) === '') {
        $nameError = 'You forgot to enter your name.';
        $hasError = true;
    } else {
            $name = trim($_POST['contactName']);
    }
    //Check to make sure sure that a valid email address is submitted
    if(trim($_POST['email']) === '')  {
        $emailError = 'You forgot to enter your email address.';
        $hasError = true;
    } else if (!eregi("^[A-Z0-9._%-]+@[A-Z0-9._%-]+\.[A-Z]{2,4}$", trim($_POST['email']))) {
            $emailError = 'You entered an invalid email address.';
            $hasError = true;
    } else {
            $email = trim($_POST['email']);
    }
    //Check to make sure comments were entered    

    if(trim($_POST['comments']) === '') {
        $commentError = 'You forgot to enter your comments.';
        $hasError = true;
    } else {
        $comments = trim($_POST['comments']);
    }
    //If there is no error, send the email
    if(!isset($hasError)) {
    echo 'email sent';
    $phone = $_POST['phone'];

$to= '[e-mail]';
$subject = 'Contact Form Submission from '.$name;
$body = 'Name: '.$name." \n\n".'Email: '.$email." \n\n".'Comments: '.$comments;
$headers = 'From: My Site <'.$to.'>' . "\r\n" . 'Reply-To: ' . $email;

$emailSent = mail($to, $subject, $body, $headers);
}
} ?> 

[bradgrafelman]

Which statement? You posted several. Also, what is 'sence' - is it anything like 'sense'? 😉

Ignoring the grammar, here are my comments about the code:

1. The spacing/indentation used throughout the code is inconsistent.

   Im  p  r    oper   l y   s pa  ce       d cod e    i s  h ar   d    to r   e     ad  !!   !

2. You use [man]isset/man to check if $_POST['submitted'] is present - which is great - but you never do the same with any of the subsequent external variables. If any of them are not present, you'll be generating E_NOTICE level error messages.

3. This:

   if (!eregi("^[A-Z0-9._%-]+@[A-Z0-9._%-]+\.[A-Z]{2,4}$", trim($_POST['email'])))

   has two major problems:

   1. It uses one of the ereg() functions which have been deprecated for quite some time now in favor of the [man]PCRE[/man] set of functions.
      

   2. It rejects perfectly valid e-mail addresses like '[e-mail]'. Rather than implementing the real regular expression (which is very lengthy, to say the least), it would probably be much wiser to simply use the [man]filter_var/man function with the FILTER_VALIDATE_EMAIL filter.

   
   
   

4. You define a variable here:
   

   $phone = $_POST['phone'];

   but never actually use it.

5. You capture the return value from mail() in the variable $emailSent, but you never check to see if the e-mail was actually accepted for delivery. In other words, this:

   echo 'email sent';

   could be entirely misleading if an error occurs.

[Bonesnap]

The only additional comment I have is there's no ELSE portion of the final IF statement, so if there are errors, there's nothing telling the user what happened.

[lukeurtnowski]

k, is this better then (should is work (as in send an email?)

<?php
/*
Template Name: Bobs Contact Form
*/
?>


<?php get_header(); ?>

<div class="map">
<iframe width="480" height="494" frameborder="0" scrolling="no" marginheight="0" marginwidth="0" src="http://maps.google.com/maps?f=q&amp;source=s_q&amp;hl=en&amp;geocode=&amp;q=Napolitano+Realty,+939+Orange+Avenue,+Coronado,+CA+92118-2609&amp;aq=2&amp;sll=37.0625,-95.677068&amp;sspn=40.409448,93.076172&amp;ie=UTF8&amp;hq=Napolitano+Realty,&amp;hnear=939+Orange+Ave,+Coronado,+San+Diego,+California+92118&amp;ll=32.686543,-117.179229&amp;spn=0.006295,0.006295&amp;output=embed"></iframe>
</div>


<?php

//If the form is submitted
if(isset($_POST['submitted']))
    {
    //Check to make sure that the name field is not empty
    if(!isset($_POST['contactName'])) {
        $nameError = 'You forgot to enter your name.';
        $hasError = true;
    } else {
        $name = trim($_POST['contactName']);
    }

//Check to make sure sure that a valid email address is submitted
$email = $_POST['email'];
if(!filter_var($email, FILTER_VALIDATE_EMAIL))
{
    $emailError = 'You entered an invalid email address.';
    $hasError = true;
} 
//Check to make sure comments were entered    
if(!isset($_POST['comments']))
{
    $commentError = 'You forgot to enter your comments.';
    $hasError = true;
} else {
    $comments = trim($_POST['comments']);
}
//If there is no error, send the email
if(!isset($hasError)) {
echo 'email sent';
$phone = $_POST['phone'];

$to= '[e-mail]';
$subject = 'Contact Form Submission from '.$name;
$body = 'Name: '.$name." \n\n".'Email: '.$email." \n\n".'Phone: '.$phone." \n\n".'Comments: '.$comments;
$headers = 'From: My Site <'.$to.'>' . "\r\n" . 'Reply-To: ' . $email;

$emailSent = mail($to, $subject, $body, $headers);
} else {
echo 'Error';
}
if(isset($emailSent)) {
?>
	<div class="thanks">
		<h1 style="text-transform: none;">Thanks, <?=$name;?></h1>
		<p>Your email was successfully sent. I will be in touch soon.</p>
	</div>

<?php }
} else { ?>


<div class="contact-form" style="padding:25px; float:left; margin-top: 50px;  background-color: rgba(195,166,96,.13); height: 444px; width: 400px;">
<h1 style="text-align:center; margin-left: -25px; margin-top: -25px; width: 420px; color: white; font: 700 30px/1.1 georgia,'times new roman','sans-serif'; text-transform: uppercase;">Contact Form</h1>
		<form action="<?php the_permalink(); ?>" id="contactForm" method="post">

		<ol class="forms" style="margin-top:25px; width:400px; clear:none; margin-left:0">
			<li><label for="contactName">Name</label>
				<input type="text" name="contactName" id="contactName" value="<?php if(isset($_POST['contactName'])) echo $_POST['contactName'];?>" class="requiredField" />
				<?php if($nameError != '') { ?>
					<span class="error"><?=$nameError;?></span> 
				<?php } ?>
			</li>
			<li><label for="email">Email</label>
				<input type="text" name="email" id="email" value="<?php if(isset($_POST['email']))  echo $_POST['email'];?>" class="requiredField email" />
				<?php if($emailError != '') { ?>
					<span class="error"><?=$emailError;?></span>
				<?php } ?>
			</li>
			<li><label for="phone">Phone</label>
				<input type="text" name="phone" id="phone" value="<?php if(isset($_POST['phone']))  echo $_POST['phone'];?>"  />
			</li>
			<li class="textarea"><label for="commentsText">Comments</label>
				<textarea name="comments" id="commentsText" rows="10" cols="30" class="requiredField" style="height:150px"><?php if(isset($_POST['comments'])) { if(function_exists('stripslashes')) { echo stripslashes($_POST['comments']); } else { echo $_POST['comments']; } } ?></textarea>
				<?php if($commentError != '') { ?>
					<span class="error"><?=$commentError;?></span> 
				<?php } ?>
			</li>
			<li class="screenReader"><label for="checking" class="screenReader">If you want to submit this form, do not enter anything in this field</label><input type="text" name="checking" id="checking" class="screenReader" value="<?php if(isset($_POST['checking']))  echo $_POST['checking'];?>" /></li>
			<li class="buttons"><input type="hidden" name="submitted" id="submitted" value="true" /><button type="submit" style="float: right; padding-top: 9px; padding-bottom: 8px; padding-left: 6px; padding-right: 6px; font-family: georgia, 'times new roman',sans-serif; background-color: #F47D3B; color: white; text-transform:uppercase">Submit</button></li>
		</ol>
	</form>
</div>	
<?php 
} 
?>

<?php get_footer(); ?>

[Bonesnap]

Two things:

1) Don't use isset() to check if the email has been sent successfully because the mail() function returns false if there's a problem, and that still means $emailSent is set, so you will always see the success message regardless of the outcome. Instead check for boolean false or true and then go from there.

2) You're using a lot of shorthand PHP tags: <?

This is considered poor practice and not recommended. You should always use the full tags: <?php

[lukeurtnowski]

ok, tried to makethe changer, now the validation doesn't work and when the form is submitted, all I get is the "Error" (So the problem must be in the if statement)
and is that the correct if statement to check if the $emailSent variable is true?

<?php
/*
Template Name: Bobs Contact Form
*/
?>


<?php get_header(); ?>

<div class="map">
<iframe width="480" height="494" frameborder="0" scrolling="no" marginheight="0" marginwidth="0" src="http://maps.google.com/maps?f=q&amp;source=s_q&amp;hl=en&amp;geocode=&amp;q=Napolitano+Realty,+939+Orange+Avenue,+Coronado,+CA+92118-2609&amp;aq=2&amp;sll=37.0625,-95.677068&amp;sspn=40.409448,93.076172&amp;ie=UTF8&amp;hq=Napolitano+Realty,&amp;hnear=939+Orange+Ave,+Coronado,+San+Diego,+California+92118&amp;ll=32.686543,-117.179229&amp;spn=0.006295,0.006295&amp;output=embed"></iframe>
</div>


<?php

//If the form is submitted
if(isset($_POST['submitted']))
    {
    //Check to make sure that the name field is not empty
    if(!isset($_POST['contactName'])) {
        $nameError = 'You forgot to enter your name.';
        $hasError = true;
    } else {
        $name = trim($_POST['contactName']);
    }

//Check to make sure sure that a valid email address is submitted
$email = $_POST['email'];
if(!filter_var($email, FILTER_VALIDATE_EMAIL))
{
    $emailError = 'You entered an invalid email address.';
    $hasError = true;
}
//Check to make sure comments were entered    
if(!isset($_POST['comments']))
{
    $commentError = 'You forgot to enter your comments.';
    $hasError = true;
} else {
    $comments = trim($_POST['comments']);
}
//If there is no error, send the email
if(!isset($hasError)) {
echo 'email sent';
$phone = $_POST['phone'];

$to= '[e-mail]';
$subject = 'Contact Form Submission from '.$name;
$body = 'Name: '.$name." \n\n".'Email: '.$email." \n\n".'Phone: '.$phone." \n\n".'Comments: '.$comments;
$headers = 'From: My Site <'.$to.'>' . "\r\n" . 'Reply-To: ' . $email;

$emailSent = mail($to, $subject, $body, $headers);
} else {
echo 'Error';
}
if($emailSent) {
?>
    <div class="thanks">
        <h1 style="text-transform: none;">Thanks, <?=$name;?></h1>
        <p>Your email was successfully sent. I will be in touch soon.</p>
    </div>

<?php }
} else { ?>


<div class="contact-form" style="padding:25px; float:left; margin-top: 50px;  background-color: rgba(195,166,96,.13); height: 444px; width: 400px;">
<h1 style="text-align:center; margin-left: -25px; margin-top: -25px; width: 420px; color: white; font: 700 30px/1.1 georgia,'times new roman','sans-serif'; text-transform: uppercase;">Contact Form</h1>
        <form action="<?php the_permalink(); ?>" id="contactForm" method="post">

        <ol class="forms" style="margin-top:25px; width:400px; clear:none; margin-left:0">
            <li><label for="contactName">Name</label>
                <input type="text" name="contactName" id="contactName" class="requiredField" />
                <?php if($nameError != '') { ?>
                    <span class="error"><?php echo $nameError; ?></span>
                <?php } ?>
            </li>
            <li><label for="email">Email</label>
                <input type="text" name="email" id="email" class="requiredField email" />
                <?php if($emailError != '') { ?>
                    <span class="error"><?php echo $emailError; ?></span>
                <?php } ?>
            </li>
            <li><label for="phone">Phone</label>
                <input type="text" name="phone" id="phone" />
            </li>
            <li class="textarea"><label for="commentsText">Comments</label>
                <textarea name="comments" id="commentsText" rows="10" cols="30" class="requiredField" style="height:150px"></textarea>
                <?php if($commentError != '') { ?>
                    <span class="error"><?php echo $commentError; ?></span>
                <?php } ?>
            </li>
            <li class="buttons"><input type="hidden" name="submitted" id="submitted" value="true" />
            <button type="submit" style="float: right; padding-top: 9px; padding-bottom: 8px; padding-left: 6px; padding-right: 6px; font-family: georgia, 'times new roman',sans-serif; 
            background-color: #F47D3B; color: white; text-transform:uppercase">Submit</button></li>
        </ol>
    </form>
</div>    

<?php
}
?>

<?php get_footer(); ?> 

[Bonesnap]

lukeurtnowski;10998102 wrote:

and is that the correct if statement to check if the $emailSent variable is true?

Yeah that should work fine.

[lukeurtnowski]

I guess my logic is screwed up, cause all I get is that stupid error thing (so I guess this is the only thing running)

if(!isset($hasError))

and its evaluating to false, why arent the other if statement running?

[lukeurtnowski]

k, changed the code to

<?php
/*
Template Name: Bobs Contact Form
*/
?>


<?php get_header(); ?>

<div class="map">
<iframe width="480" height="494" frameborder="0" scrolling="no" marginheight="0" marginwidth="0" src="http://maps.google.com/maps?f=q&amp;source=s_q&amp;hl=en&amp;geocode=&amp;q=Napolitano+Realty,+939+Orange+Avenue,+Coronado,+CA+92118-2609&amp;aq=2&amp;sll=37.0625,-95.677068&amp;sspn=40.409448,93.076172&amp;ie=UTF8&amp;hq=Napolitano+Realty,&amp;hnear=939+Orange+Ave,+Coronado,+San+Diego,+California+92118&amp;ll=32.686543,-117.179229&amp;spn=0.006295,0.006295&amp;output=embed"></iframe>
</div>


<?php
$hasError = false;

//If the form is submitted
if(isset($_POST['submitted']))
    {
    //Check to make sure that the name field is not empty
    if(!isset($_POST['contactName'])) {
        $formError[0] = 'You forgot to enter your name.';
        $hasError = true;
    } else {
        $name = trim($_POST['contactName']);
    }

//Check to make sure sure that a valid email address is submitted
$email = $_POST['email'];
if(!filter_var($email, FILTER_VALIDATE_EMAIL))
{
    $formError[1] = 'You entered an invalid email address.';
    $hasError = true;
}
//Check to make sure comments were entered    
if(!isset($_POST['comments']))
{
    $formError[2] = 'You forgot to enter your comments.';
    $hasError = true;
} else {
    $comments = trim($_POST['comments']);
}
//If there is no error, send the email
if($hasError) {
echo $hasError;
echo 'Error';
print_r($formError);
} else {
echo 'email sent';
$phone = $_POST['phone'];

$to= '[e-mail]';
$subject = 'Contact Form Submission from '.$name;
$body = 'Name: '.$name." \n\n".'Email: '.$email." \n\n".'Phone: '.$phone." \n\n".'Comments: '.$comments;
$headers = 'From: My Site <'.$to.'>' . "\r\n" . 'Reply-To: ' . $email;

$emailSent = mail($to, $subject, $body, $headers);
}
if($emailSent) {
?>
    <div class="thanks">
        <h1 style="text-transform: none;">Thanks, <?=$name;?></h1>
        <p>Your email was successfully sent. I will be in touch soon.</p>
    </div>

<?php }
} else { ?>


<div class="contact-form" style="padding:25px; float:left; margin-top: 50px;  background-color: rgba(195,166,96,.13); height: 444px; width: 400px;">
<h1 style="text-align:center; margin-left: -25px; margin-top: -25px; width: 420px; color: white; font: 700 30px/1.1 georgia,'times new roman','sans-serif'; text-transform: uppercase;">Contact Form</h1>
        <form action="<?php the_permalink(); ?>" id="contactForm" method="post">

        <ol class="forms" style="margin-top:25px; width:400px; clear:none; margin-left:0">
            <li><label for="contactName">Name</label>
                <input type="text" name="contactName" id="contactName" class="requiredField" />
                <?php if($formError[0] != '') { ?>
                    <span class="error"><?php echo $formError[0]; ?></span>
                <?php } ?>
            </li>
            <li><label for="email">Email</label>
                <input type="text" name="email" id="email" class="requiredField email" />
                <?php if($formError[1] != '') { ?>
                    <span class="error"><?php echo $formError[1]; ?></span>
                <?php } ?>
            </li>
            <li><label for="phone">Phone</label>
                <input type="text" name="phone" id="phone" />
            </li>
            <li class="textarea"><label for="commentsText">Comments</label>
                <textarea name="comments" id="commentsText" rows="10" cols="30" class="requiredField" style="height:150px"></textarea>
                <?php if($formError[3] != '') { ?>
                    <span class="error"><?php echo $formError[3]; ?></span>
                <?php } ?>
            </li>
            <li class="buttons"><input type="hidden" name="submitted" id="submitted" value="true" />
            <button type="submit" style="float: right; padding-top: 9px; padding-bottom: 8px; padding-left: 6px; padding-right: 6px; font-family: georgia, 'times new roman',sans-serif; 
            background-color: #F47D3B; color: white; text-transform:uppercase">Submit</button></li>
        </ol>
    </form>
</div>    

<?php
}
?>

<?php get_footer(); ?> 

But now when it runs I runs, I get
1ErrorArray ( [1] => You entered an invalid email address. )
but when I fill out the form, I get email sent
why does the form submit and not validate if the form is not filled out correctly?

[lukeurtnowski]

ok, I (once again) made an update to the script, so let me know if its sound
here it is

<?php
/*
Template Name: Bobs Contact Form
*/
?>


<?php get_header(); ?>

<div class="map">
<iframe width="480" height="494" frameborder="0" scrolling="no" marginheight="0" marginwidth="0" src="http://maps.google.com/maps?f=q&amp;source=s_q&amp;hl=en&amp;geocode=&amp;q=Napolitano+Realty,+939+Orange+Avenue,+Coronado,+CA+92118-2609&amp;aq=2&amp;sll=37.0625,-95.677068&amp;sspn=40.409448,93.076172&amp;ie=UTF8&amp;hq=Napolitano+Realty,&amp;hnear=939+Orange+Ave,+Coronado,+San+Diego,+California+92118&amp;ll=32.686543,-117.179229&amp;spn=0.006295,0.006295&amp;output=embed"></iframe>
</div>


<?php
$hasError = false;
$name = $_POST['contactName'];
$email = $_POST['email'];
$phone = $_POST['phone'];
$comments = $_POST['comments'];

//If the form is submitted
if(isset($_POST['submitted']))
    {
    //Check to make sure that the name field is not empty
    if(empty($name)) {
        $formError[0] = 'You forgot to enter your name.';
        $hasError = true;
    }
    //Check to make sure sure that a valid email address is submitted
    if(!filter_var($email, FILTER_VALIDATE_EMAIL))
    {
        $formError[1] = 'You entered an invalid email address.';
        $hasError = true;
    }
    //Check to make sure comments were entered    

    if(empty($comments))
    {
        $formError[2] = 'You forgot to enter your comments.';
        $hasError = true;
    }
    //If there is no error, send the email
    if(!isset($hasError)) {
    echo 'email sent';
    $to= '[e-mail]';
    $subject = 'Contact Form Submission from '.$name;
    $body = 'Name: '.$name." \n\n".'Email: '.$email." \n\n".'Phone: '.$phone." \n\n".'Comments: '.$comments;
    $headers = 'From: My Site <'.$to.'>' . "\r\n" . 'Reply-To: ' . $email;

$emailSent = mail($to, $subject, $body, $headers);
}
if($emailSent) {
?>
    <div class="thanks">
        <h1 style="text-transform: none;">Thanks, <?=$name;?></h1>
        <p>Your email was successfully sent. I will be in touch soon.</p>
    </div>

<?php }
} else { ?>


<div class="contact-form" style="padding:25px; float:left; margin-top: 50px;  background-color: rgba(195,166,96,.13); height: 444px; width: 400px;">
<h1 style="text-align:center; margin-left: -25px; margin-top: -25px; width: 420px; color: white; font: 700 30px/1.1 georgia,'times new roman','sans-serif'; text-transform: uppercase;">Contact Form</h1>
        <form action="<?php the_permalink(); ?>" id="contactForm" method="post">

        <ol class="forms" style="margin-top:25px; width:400px; clear:none; margin-left:0">
            <li><label for="contactName">Name</label>
                <input type="text" name="contactName" id="contactName" class="requiredField" />
                <?php if(empty($formError[0])) { ?>
                    <span class="error"><?php echo $formError[0]; ?></span>
                <?php } ?>
            </li>
            <li><label for="email">Email</label>
                <input type="text" name="email" id="email" class="requiredField email" />
                <?php if(empty$formError[1])) { ?>
                    <span class="error"><?php echo $formError[1]; ?></span>
                <?php } ?>
            </li>
            <li><label for="phone">Phone</label>
                <input type="text" name="phone" id="phone" />
            </li>
            <li class="textarea"><label for="commentsText">Comments</label>
                <textarea name="comments" id="commentsText" rows="10" cols="30" class="requiredField" style="height:150px"></textarea>
                <?php if(empty(($formError[2])) { ?>
                    <span class="error"><?php echo $formError[3]; ?></span>
                <?php } ?>
            </li>
            <li class="buttons"><input type="hidden" name="submitted" id="submitted" value="true" />
            <button type="submit" style="float: right; padding-top: 9px; padding-bottom: 8px; padding-left: 6px; padding-right: 6px; font-family: georgia, 'times new roman',sans-serif;
            background-color: #F47D3B; color: white; text-transform:uppercase">Submit</button></li>
        </ol>
    </form>
</div>    

<?php
}
?>

<?php get_footer(); ?>

First, I instanciate all the variables (and the ones submitted from the form).
Then I heck if the $name field has been filled out, if it hasn't assign a value to the array $formError[0] and set a value to $hasError
Then check if the email address is valid, if it isn't set a valur to $formError[1] and set a value to $hasError
Then I check to see if any comments have been filled out, if it hsnt, set a value to $formError[2] and also to $hasError
If all three fields are good, then check if the $formError variable has been set, if it hasnt, send an email to me with mail()
Then I check if the email has been sent,and if it has I output athank you div.
Finally if the form hasn't been submitted I display a form (which check if those 3 fields have been filled out along the way.

Thanks...