contact form class

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
the_cheat
Forum Newbie
Posts: 5
Joined: Tue Oct 12, 2010 4:09 pm

contact form class

Post by the_cheat »

here is a class i made that creates and processes a contact form. I would appreciate it it you could look at it and critique it and give me any notes. Sorry it's not documented at all, I hope that's not a problem. thanks

Code: Select all

<?php
	ini_set('display_errors', 1);

	class ContactForm
	{
		var $form_block;
		var $name_err;
		var $address_err;
		var $message_err;
		var $name_min;
		var $name_max;
		var $address_min;
		var $address_max;
		var $message_min;
		var $message_max;
		var $send_status;
		var $receiver_address;
		
		function __construct($receiving_address)
		{
			$this->form_block = "
    			<form method=\"post\" action=\"$_SERVER[PHP_SELF]\">
    			<p><bold>Your name:</bold><br />
    			<input type=\"text\" name=\"txt_sender_name\" value=\"$_POST[txt_sender_name]\" /></p>
    			<p><bold>Your email address:</bold><br />
    			<input type=\"text\" name=\"txt_sender_address\" value=\"$_POST[txt_sender_address]\" /></p>
    			<p><bold>Message:</bold><br />
    			<textarea name=\"txta_message\" cols=30 rows=20></textarea></p>
    			<input type=\"hidden\" name=\"submit_pressed\" value=\"yes\" />
    			<p><input type=\"submit\" name=\"btn_submit\" value=\"Send\" /></p></form>";
    		$this->name_err = "";
			$this->address_err = "";
			$this->message_err = "";
			$this->name_min = 0;
			$this->name_max = 100;
			$this->address_min = 0;
			$this->address_max = 50;
			$this->message_min = 0;
			$this->message_max = 5000;
			$this->send_status = true;
			$this->receiver_address = $receiving_address;
    		
    			
    	}
    	
    	function perform() 
    	{
    		if ($_POST[submit_pressed] != "yes") {
    			echo $this->form_block;
    		} else if ($_POST[submit_pressed] == "yes") {
    			$this->validate_name($this->name_min, $this->name_max);
    			$this->validate_email($this->address_min, $this->address_max);
    			$this->validate_message($this->message_min, $this->message_max);
    			
    			if ($this->send_status == false) {
    				echo $this->name_err;
    				echo $this->address_err;
    				echo $this->message_err;
    				echo $this->form_block;
    			} else if ($this->send_status == true) {
    				$email_info = $this->build_email();
    				$this->send_form($email_info);
    			}
    		}
    	}
    	
    	function validate_name($min=0, $max=100) 
    	{
    		if ($_POST[txt_sender_name] == "") {
    			$this->name_err = "<font color=\"red\">Please enter your name</font><br />";
    		} else if ($_POST[txt_sender_name] < $min) {
    			$this->name_err = "<font color=\"red\">Your name must be larger than {$min} characters.</font><br />";
    		} else if ($_POST[txt_sender_name] > $max) {
    			$this->name_err = "<font color=\"red\">Your name cannot be longer than {$max} characters.</font><br />";
    		}
    		if ($this->name_err != "") {
    			$this->send_status = false;
    		}
    	}
    	
    	function validate_email($min=0, $max=50)
    	{
    		if ($_POST[txt_sender_address] == "") {
    			$this->address_err = "<font color=\"red\">Please enter your E-Mail Address.</font><br />";
    		} else if ($_POST[txt_sender_address] < $min) {
    			$this->address_err = "<font color=\"red\">Your E-Mail Address must be larger than {$min} characters.</font><br />";
    		} else if ($_POST[txt_sender_address] > $max) {
    			$this->address_err = "<font color=\"red\">Your E-Mail Address cannot be longer than {$max} characters.</font><br />";
    		} else if (!preg_match("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,6})$^", $_POST[txt_sender_address])) {
    			$this->address_err = "<font color=\"red\">The E-Mail Address you entered is not valid, please enter a valid E-Mail Address.</font><br />";
    		}
    		if ($this->address_err != "") {
    			$this->send_status = false;
    		}
    	} 
    	
    	function validate_message($min=0, $max=5000) 
    	{
    		if ($_POST[txta_message] == "") {
    			$this->message_err = "<font color=\"red\">Please enter the message you would like to send.</font><br />";
    		} else if ($_POST[txta_message] < $min) {
    			$this->message_err = "<font color=\"red\">Your message must be at least {$min} characters long.</font><br />";
    		} else if ($_POST[txta_message] > $max) {
    			$this->message_err = "<font color=\"red\">Your message can be no longer than {$max} characters.</font><br />";
    		}
    		if ($this->message_err != "") {
    			$this->send_status = false;
    		}
    	}
    	
    	function build_email()
    	{
    		$msg = "Sender's name:		$_POST[txt_sender_name]\n";
    		$msg .= "Sender's address:	$_POST[txt_sender_address]\n";
    		$msg .= "Message:\n\n  \t$_POST[txta_message]\n\n";
    		$headers = "From:		My Web Site\n";
    		$headers .= "Reply-To:	$_POST[txt_sender_address]\n\n";
    		
    		return array($headers, $msg);
    	}
    	
    	function send_form($info)
    	{
    		$msg = $info[0];
    		$headers = $info[1];
    		$subject = "Contact Form From Website";
    		$result = mail($this->receiver_address, $subject, $msg, $headers);
    		if ($result) {
    			echo "Mailed successfully";
    		} else {
    			echo "Mailed unsuccessfully";
    		}
    	}
    }
?>
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: contact form class

Post by Eran »

I'll be blunt - there's quite a few problems here. I'll go over what I see in a glance -
1. You mix PHP4 syntax with PHP5 syntax - using 'var' for properties (which is PHP4 - instead of PHP5 visibility keywords) but __construct() (which is PHP5). I suggest stick with one, preferably PHP5 since it has been out for over 6 years now.
2. You don't need to assign static values in the constructor, put those in the property list directly. Use the constructor to pass new values to the class properties. I suggest using an array for that.

Code: Select all

public function __construct($config = array())
{
    foreach( $config as $key => $val ) {
         if(isset($this -> $key)) {
              $this -> $key = $val;
         }
    }
}
3. You don't use array indexes properly. It should be either integers or strings. So this

Code: Select all

$_POST[submit_pressed]
Should really be this

Code: Select all

$_POST['submit_pressed']
4. Your final function, send_form(), outputs instead of just returning a result (boolean or string). If you returned instead of outputting you can send different output depending on the situation, or do other things with it (like logging silently, redirecting, etc)
User avatar
Christopher
Site Administrator
Posts: 13592
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: contact form class

Post by Christopher »

Eran has pointed out several more problems than I identified from a quick look at the code. Agree with all of it.

On the general design front, this seems like it should be multiple classes. I see at least a template script, but maybe a class to display the form and success/error messages. The you have the configuration and the perform() method which seem like the core form processing class. Then there are validators, which could each be a class to make the system more modular. And finally you send an email, which should probably be a separate class (or use something like SwiftMailer).
(#10850)
Post Reply