Writing cleaner PHP code to add additional options

Question

I have a contact form that will perform validation on the server. Right now the form checks if the fields are empty or not. My goal is to expand it following:

Check the fields for particular format, if it is a proper email format, phone number format, etc Set the field as required or not

My form is as below, I appreciate any suggestions on how to clean it up and make it easier to adjust and overall cleaner.

<?php

ini_set("log_errors", 1);
ini_set("error_log", "php-error.log");
error_log( "Errors" );

include "recaptcha.php";
include 'email.php';
use PHPMailer\PHPMailer\PHPMailer;
use PHPMailer\PHPMailer\Exception;

require 'Exception.php';
require 'PHPMailer.php';
require 'SMTP.php';

$errors         = array();      // array to hold validation errors
$data           = array();         // array to pass back data

// validate the variables ======================================================
    // if any of these variables don't exist, add an error to our $errors array

    if (empty($_POST['firstName']))
        $errors['firstName'] = 'First Name is required.';

    if (empty($_POST['lastName']))
        $errors['lastName'] = 'Last Name is required.';

    if (empty($_POST['companyName']))
        $errors['companyName'] = 'Company Name is required.';

    if (empty($_POST['companyAddress']))
        $errors['companyAddress'] = 'Company Address is required.';

    if (empty($_POST['city']))
        $errors['city'] = 'City is required.';

    if (empty($_POST['state']))
        $errors['state'] = 'State is required.';

    if (empty($_POST['emailAddress']))
        $errors['emailAddress'] = 'Email Address is required.';

    if (empty($_POST['comment']))
        $errors['comment'] = 'Comment is required.';

    if (empty($_POST['g-recaptcha-response']))
        $errors['recaptcha'] = 'Captcha is required.';


// return a response ===========================================================

    // Check ReCaptcha Validation

    if(!validateRecaptcha($secret, $_POST['g-recaptcha-response'], $_SERVER["REMOTE_ADDR"]))
    {
        $errors['recaptcha'] = 'Captcha is required.';
    }

    if ( ! empty($errors)) {

        // if there are items in our errors array, return those errors
        $data['success'] = false;
        $data['errors']  = $errors;
    } else {

        // if there are no errors process our form, then return a message

        // DO ALL YOUR FORM PROCESSING HERE
        // THIS CAN BE WHATEVER YOU WANT TO DO (LOGIN, SAVE, UPDATE, WHATEVER)

        // show a message of success and provide a true success variable
        $data['success'] = true;
        $data['message'] = 'Success!';
    }

    // return all our data to an AJAX call
    echo json_encode($data);

//Begin of send email
$mail = new PHPMailer;

//Enable SMTP debugging.
$mail->SMTPDebug = 3;
//Set PHPMailer to use SMTP.
$mail->isSMTP();
//Set SMTP host name
$mail->Host = "smtp.server.com";
//Set this to true if SMTP host requires authentication to send email
$mail->SMTPAuth = true;
//Provide username and password
$mail->Username = "[email protected]";
$mail->Password = "super_secret_password";
//If SMTP requires TLS encryption then set it
$mail->SMTPSecure = "tls";
//Set TCP port to connect to
$mail->Port = 587;

$mail->From = $emailAddress;

$mail->addAddress("[email protected]", "Recepient Name");

$mail->isHTML(true);

$mail->Subject = "Subject Text";
$mail->Body = $emailBody;

if(!$mail->send())
{
  $data['success'] = false;
  $data['errors']  =  "Mailer Error: " . $mail->ErrorInfo;
}
else
{
    $data['message'] = 'Success!';
}

?>

Community · Accepted Answer · 2020-06-10 13:24:26Z

General inconsistency

Basically your code is borderline off topic. Although it could work, it is not a working code in the sense a programmer could take it; but rather a set of copy-pasted code blocks loosely (and wrongly) connected to each other.

The main problem is inconsistent code structure. You have $data['success'] set at the end of the script but... it goes nowhere.

So it basically should be

if ($errors) {
    $data['success'] = false;
    $data['errors']  = $errors;
} else {
    $mail = new PHPMailer;
    // the rest or phpmailer code goes on
    if(!$mail->send())
    {
        $data['success'] = false;
        $data['errors']  =  ["Mailer Error: " . $mail->ErrorInfo];
    }
    else
    {
        $data['success'] = true;
        $data['message'] = 'Success!';
    }
}
// return all our data to an AJAX call
echo json_encode($data);

Note that another error has been fixed. $data['errors'] should be an array, but in case of phpmailer error you were sending a string.

Configuration options

error_log( "Errors" );

Always use the absolute filesystem path to a log file or you'll end up with log files in the every directory.

Also, it's better to configure such settings on the server level, for some errors may occur before the file gets parsed completely. Or at least put these settings into a configuration file and include it in all your php scripts.

Autoload

Instead of requiring files manually, consider using PSR-4 autoload and Composer. It's really easy, you just have to install Composer, then run two commands in console

php composer.phar require google/recaptcha
php composer.phar require phpmailer/phpmailer

and then add just a single require line into your code

require __DIR__ . '/vendor/autoload.php';

and all classes will be loaded automatically!

Validation and sanitization

You may want to verify whether the email address is valid.

if (empty($_POST['emailAddress'])) {
    $errors['emailAddress'] = 'Email Address is required.';
} else {
    $emailAddress = filter_var($errors['emailAddress'], FILTER_SANITIZE_EMAIL);
    if (!filter_var($emailAddress, FILTER_VALIDATE_EMAIL)) {
        $errors['emailAddress'] = 'Email Address is invalid.';
    }
}

and also sanitize the email body,

$emailBody = htmlspecialchars($_POST['comment']);

Stack Exchange Network

Writing cleaner PHP code to add additional options

1 Answer 1

General inconsistency

Configuration options

Autoload

Validation and sanitization

You must log in to answer this question.

Hot Network Questions

Writing cleaner PHP code to add additional options

1 Answer 1

General inconsistency

Configuration options

Autoload

Validation and sanitization

You must log in to answer this question.

Related

Hot Network Questions