CodeIgniter Forums

Full Version: No idea how to refactor this into Code Igniter
You're currently viewing a stripped down version of our content. View the full version with proper formatting.

El Forum

[eluser]chobo[/eluser]
I'm trying to convert an existing site into CodeIgniter, and I'm making pretty good progress, but I'm kind of stuck on this file. To give you a brief intro the file just handles the contact form information by doing validation, producing appropriate error messages, and sending the email. Users would never actually see this file and it is loaded when the user hits the submit form.

Hold you breath the code is nasty Smile. I don't expect anyone to refactor it all, I just need some pointers on how to handle some things.

Issues:

1. I am using the phpMailer class and don't plan on changing. Which folder should I put this class in and how should I load it?

2. When the user clicks on the form, should I handle all the logic in a controller, including the validation? I have excluded the part with the mail credentials but it is called from the mail.php file.

Overall I just need to know where stuff should go, especially the part when a user clicks the form button, I'm not even sure how to handle that. Any help is appreciated, thanks.


Code:
<?php
function validate_input($from_name, $from_address, $subject, $body) {

// Validate User Name
if(strlen($from_name) > 50) {
  echo 'Your name is too long. Please keep you name under 50 characters';
  echo '<a href = "[removed]history.back()">  Go back  </a>';
      exit();
} else if (strlen($from_name) == 0) {
     echo 'Please fill in the Name field';
     echo '<a href = "[removed]history.back()">  Go back  </a>';
     exit();
}
    

// Validate User Address
if(strlen($from_address) > 50) {
      echo 'Your email address is too long. Please use an address under 50 characters';
      echo '<a href = "[removed]history.back()">  Go back  </a>';
      exit();
} else if (strlen($from_address) == 0) {
     echo 'Please fill in the Address field';
      echo '<a href = "[removed]history.back()">  Go back  </a>';
      exit();
}


// Validate Subject
if(strlen($subject) > 50) {
    echo 'Your email address is too long. Please use an address under 50 characters';
  echo '<a href = "[removed]history.back()">  Go back  </a>';
     exit();
} else if (strlen($subject) == 0) {
  echo 'Please select a subject';
      echo '<a href = "[removed]history.back()">  Go back  </a>';
      exit();
}
  

// Validate
if(strlen($body) > 1000) {
     echo 'Your message is too long. Please keep your message under a 1000 characters';
     echo '<a href = "[removed]history.back()">  Go back  </a>';
     exit();
} else if (strlen($body) < 10) {
      echo 'Your message is too short please revise it';
      echo '<a href = "[removed]history.back()">  Go back  </a>';
     exit();
}
}

require_once 'con-link.php';
require_once 'p-link.php'; // path to mail settings
$from_name = mysqli_escape_string($con, $_POST['name']);
$from_address =  mysqli_escape_string($con, $_POST['address']);
$subject =  mysqli_escape_string($con, $_POST['subject']);
$body =  mysqli_escape_string($con, $_POST['message']);

validate_input($from_name, $from_address, $subject, $body);

require("phpmailer/class.phpmailer.php");

// mail settings
require_once INCLUDE_PATH . 'mail.php';

if(!$mail->Send())
{
   $message = "There was a problem sending your email, please try again later.";
   $message .= "Mailer Error: " . $mail->ErrorInfo;
}
else
{
   $message = "Your message has been sent successfully!";
}
?&gt;
&lt;?php
require_once 'link.php';
?&gt;
<div id="col_container">
  <div id="left_column">
   <h1 class="g_header">Affiliates</h1>
   <img src="images/affiliates.gif" height="600px" width="160px" />
  </div>
  <div id="right_column">
   <div>
    <h1 class="sent_message">&lt;?php echo $message; ?&gt;</h1>
   </div>
  </div>  
</div>
&lt;?php
require_once INCLUDE_PATH . 'footer.php';
?&gt;

El Forum

[eluser]zscott[/eluser]
Have you read the doc for validation library?

El Forum

[eluser]chobo[/eluser]
Actually I just went over that and it was a big help. Right now I put everything into the contact controllers index() method (code below).

I'm still trying to figure out where I should put the phpmailer classes and how I should load it. I'm also concerned that I am "dumping" too much into the index() controller it just seems messy. I did try making a new method called sendmail, but I had issues with pre-populating fields and loading helpers, since that was done in the sendmail() function it would cause errors when I loaded up the vanilla index() one.

Code:
&lt;?php

require_once 'template_engine.php';

class Contact extends TemplateEngine {


public function __construct()
{
  parent::__construct();
}

/* Displays default page */
public function index()
{  
  $this->load->helper(array('form', 'url'));
  $this->load->library('validation');
  
  //set rules
  $rules['name'] = "required|min_length[2]|max_length[35]";
  $rules['email'] = "required|valid_email";
  $rules['message'] = "required|min_length[10]|max_length[1000]";
  $this->validation->set_rules($rules);

  // if it fails set the fields
  $fields['name'] = 'Name';
  $fields['email'] = 'Email Address';
  $fields['message'] = 'Message';
  $this->validation->set_fields($fields);
  
  parent::set_section('header', $this->load->view('layouts/default_header', $this->header, true));
  parent::set_section('left', $this->load->view('left_ad_view', '', true));
  parent::set_section('footer', $this->load->view('layouts/default_footer', '', true));
  
  // run validation and see if it passes
  if ($this->validation->run() == FALSE)
  {      
   parent::set_section('content', $this->load->view('contact_view', '', true));
  }
  else
  {
   $from_name = mysqli_escape_string($con, $_POST['name']);
   $from_address =  mysqli_escape_string($con, $_POST['address']);
   $subject =  mysqli_escape_string($con, $_POST['subject']);
   $body =  mysqli_escape_string($con, $_POST['message']);
  
   require_once 'phpmailer/class.phpmailer.php';
   require_once INCLUDE_PATH . 'mail.php';  
  
   //init $message var?
   if(!$mail->Send())
   {
    $message = "There was a problem sending your email, please try again later.";
    $message .= "Mailer Error: " . $mail->ErrorInfo;
   }
   else
   {
    $message = "Your message has been sent successfully!";
   }

  
   parent::set_section('content', $this->load->view('message_sent_view', '', true));
  }

  // build page
  parent::build_template();
}
}
?&gt;

El Forum

[eluser]esra[/eluser]
In earlier versions, CI supported a libraryname_init.php file stored under init/ and the feature was removed some time ago. The library itself was stored under application/libraries. Adding certain libraries such as ADODB was less of a problem back then.

You might take a look at the approach used by DanFreak to integrate SwiftMail with CI.

http://ellislab.com/forums/viewthread/47582/

I believe that Daniel successfully used similar approaches to integrate other libraries.

El Forum

[eluser]chobo[/eluser]
I found a pretty good way to add external classes. I add a constant in the index.php that builds off of the other paths already defined. Later I will try to centralize all my paths into one file, and maybe even the base_url, so everything is in one place.

Now onto the contact form Smile I actually got it to work, but it's still far from done in my view.

Q1: I use to use mysqli_escape_string() on the POST variables, but this doesn't work unless I require it. First, is it even worth doing, or is escaping just for inserting and interacting with databases?

Second, if it is worth doing, how can I get access to the $con (connection variable) for the mysqli connection, so I can pass it to the mysqli_escape_string()? I have tried looking for a simple escape function, but they are a little overkill like xss_clean, or they require objects like escape_str().

Q2: I'm trying to improve my coding, so could someone give me a few pointers on how I could refactor the following code? Thanks!

Code:
&lt;?php

require_once 'template_engine.php';

class Contact extends TemplateEngine {


public function __construct()
{
  parent::__construct();
}

/* Displays default page */
public function index()
{  
  $this->load->helper(array('form', 'url'));
  $this->load->library('validation');
  
  //set rules
  $rules['name'] = "required|min_length[2]|max_length[35]";
  $rules['email'] = "required|valid_email";
  $rules['message'] = "required|min_length[10]|max_length[1000]";
  $this->validation->set_rules($rules);

  // if it fails set the fields
  $fields['name'] = 'Name';
  $fields['email'] = 'Email Address';
  $fields['message'] = 'Message';
  $this->validation->set_fields($fields);
  
  parent::set_section('header', $this->load->view('layouts/default_header', $this->header, true));
  parent::set_section('left', $this->load->view('left_ad_view', '', true));
  parent::set_section('footer', $this->load->view('layouts/default_footer', '', true));
  
  // run validation and see if it passes
  if ($this->validation->run() == FALSE)
  {      
   parent::set_section('content', $this->load->view('contact_view', '', true));
  }
  else
  {
   $from_name = ($_POST['name']);
   $from_address = ($_POST['email']);
   $subject = ($_POST['subject']);
   $body = ($_POST['message']);
  
   require_once EXT_LIB . 'phpmailer/class.phpmailer.php';
   $mail = new PHPMailer();
   $mail->IsSMTP(); // telling the class to use SMTP
   $mail->Host = "smtp.isp.net"; // SMTP server  
   $mail->FromName = $from_name; // Sets the name of the emailer
   $mail->From = $from_address;  // Sets the email address of the sender
   $mail->AddAddress("[email protected]");  // Sets the To Address
   $mail->Subject = $subject; // Sets the subject heading
   $mail->Body = $body;
   $mail->WordWrap = 50;  
  
   //init $message var?
   if(!$mail->Send())
   {
    $message = "There was a problem sending your email, please try again later.";
    $message .= "Mailer Error: " . $mail->ErrorInfo;
   }
   else
   {
    $message = "Your message has been sent successfully!";
   }
  
   parent::set_section('content', $this->load->view('message_sent_view', '', true));
  }

  // build page
  parent::build_template();
}
}
?&gt;

El Forum

[eluser]Rick Jolly[/eluser]
mysqli_escape_string() is for mysql. It escapes special database characters (') and prevents sql injection. If you need to display the $_POST data then you should use xss_clean or html purifier. If you trust your users, then at the very least use htmlspecialchars() so your html doesn't get corrupted.

Your code looks great. I'd just put the email stuff in a library.

El Forum

[eluser]chobo[/eluser]
Hi, I just finished refactoring please let me know what you think. I moved the email settings into a custom config file, and broke the controller into more discrete functions. Hi Rick, you mentioned that I should move my email into a library, could you please elaborate on that, I'm still kind a super newb, so I don't know how to take advantage of that or what the benefits are. Anyways here is my latest revision, any feedback is appreciated.


Code:
&lt;?php

require_once 'template_engine.php';

class Contact extends TemplateEngine {


/* Load email config and helpers */
public function __construct()
{
  parent::__construct();
  $this->config->load('email_config');
  $this->load->helper(array('form', 'url'));
  $this->load->library('validation');
}


/* Displays default page */
public function index() {  

  parent::set_section('header', $this->load->view('layouts/default_header', $this->header, true));
  parent::set_section('left', $this->load->view('left_ad_view', '', true));
  parent::set_section('footer', $this->load->view('layouts/default_footer', '', true));
  
  // run validation and see if it passes
  if ($this->validate() == FALSE) {  
   parent::set_section('content', $this->load->view('contact_view', '', true));
  } else {
   $error['message'] = $this->sendMail();
   parent::set_section('content', $this->load->view('message_sent_view', $error, true));
  }

  parent::build_template();
}


/* Runs validation rules (returns true or false) */
private function validate()
{
  //set rules
  $rules['name'] = "required|min_length[2]|max_length[35]";
  $rules['email'] = "required|valid_email";
  $rules['message'] = "required|min_length[10]|max_length[1000]";
  $this->validation->set_rules($rules);

  // if it fails set the fields
  $fields['name'] = 'Name';
  $fields['email'] = 'Email Address';
  $fields['message'] = 'Message';
  $this->validation->set_fields($fields);
  
  return $this->validation->run();
}

/* Send email off */
private function sendMail()
{
  $from_name = ($_POST['name']);
  $from_address = ($_POST['email']);
  $subject = ($_POST['subject']);
  $body = ($_POST['message']);
  
  require_once EXT_LIB . 'phpmailer/class.phpmailer.php';
  $mail = new PHPMailer();
  $mail->IsSMTP(); // telling the class to use SMTP
  $mail->Host = $this->config->item('smtp_server'); // SMTP server
  //$mail->Username = $this->config->item('user');;
  //$mail->Password = $this->config->item('password');;
  //$mail->SMTPAuth = $this->config->item('auth');  // turn on SMTP authentication
  
  $mail->FromName = $from_name; // Sets the name of the emailer
  $mail->From = $from_address;  // Sets the email address of the sender
  $mail->AddAddress($this->config->item('to'));  // Sets the To Address
  $mail->Subject = $subject; // Sets the subject heading
  $mail->Body = $body;
  $mail->WordWrap = 50;  
  
  //init $message var?
  if(!$mail->Send())
  {
   $message = "There was a problem sending your email, please try again later.";
   $message .= "Mailer Error: " . $mail->ErrorInfo;
  }
  else
  {
   $message = "Your message has been sent successfully!";
  }
  
  return $message;
}
}
?&gt;