5
\$\begingroup\$

I created a simple function for swiftmailer to send two different messages for support and client. Not sure i did it the rigth way as I am using two different instances for message and one mailer instance. Everything works just fine but am I doing it the right way or I have to create separate mailer instance for each message? Here is my code, if someone could point out if it is ok or could be done better:

<?php

if ((isset($_POST['email'])) && (strlen(trim($_POST['email'])) > 0)) {
    $email = stripslashes(strip_tags($_POST['email']));
} else {$email = 'No data';}

require_once '/home/sitename/public_html/inc/swift/lib/swift_required.php';

function send_email($info){     

    $transport = Swift_MailTransport::newInstance();

    $mailer = Swift_Mailer::newInstance($transport);   

    //Create the message for support
    $message = Swift_Message::newInstance()
        ->setSubject('Subject 1')
        ->setFrom(array('info@domain.com' => 'Info'))
        ->setTo(array(
            'name1@domain.com',
            'name2@domain.com',
        ))
        ->setBody($info['message_support'],'text/html');

    //Create the message for client    
    $message2 = Swift_Message::newInstance()
        ->setSubject('Subject 2')
        ->setFrom(array('info@domain.com' => 'Info'))
        ->setTo(array(
            $info['email']          
        ))
        ->setBody($info['message_client'],'text/html');

    if ($mailer->send($message) && $mailer->send($message2)) 
        return true;
    else
        return false;               
}

$info = array(
    'email' => $email,
    'message_support' => '<p>Dear Support</p><p>Hello World</p>',
    'message_client' => '<p>Dear Client</p><p>Hello World</p>'  
);

if (send_email($info))
{
  header("Location:/thank-you");  
}
else
{
  header("Location:/error");
}

?>
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Note: this is intended to be a Community wiki since it was taken from a deleted answer by yannis; The suggestion to do this was here


Using two message instances is fine as is most of your code. There is one major issue though, you don't validate the email, you just check if the $_POST variable was set and is not empty but you don't check if the email format is "name@domain.tld". The simplest thing you could do is:

$email = 
    isset($_POST["email"]) && !empty($_POST["email"]) 
    ? filter_var($email, filter_var($_POST["email"], FILTER_SANITIZE_EMAIL) )
    : "No data";

The filter extension greatly helps you sanitize and validate variables. It's on by default since PHP 5.2, if you are working on an earlier version, you can substitute it with an email validation regular expression. By the way, if you are unfamiliar with the conditional format I'm using, it's called ternary operator and it goes something like this:

$variable = 
    ( condition )
    ? ( value if condition is true )
    : ( value if condition is false );

When actually sending the email, you should do:

if( $email != "No Data" && send_email($info) ) {
  header("Location:/thank-you");  
} else {
  header("Location:/error");
}

That way and since the check $email != "No Data" is first, send_email($info) will never execute for a missing or invalid email. So the 2 instances of a hefty library will never instantiate needlessly.

Lastly, a very minor change, that might be considered a matter of style. Here:

if ($mailer->send($message) && $mailer->send($message2)) 
    return true;
else
    return false;  

You are returning a boolean value based on the boolean return of $mailer->send($message) && $mailer->send($message2). You can simply rewrite as:

return $mailer->send($message) && $mailer->send($message2);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.