2
\$\begingroup\$

This is the first time I've tried this. I'd like some feedback on how I did, including any bad practice warnings. For example is it a really bad idea to allow the code to recreate the table if it doesn't "think" it exists? Would it be better to simply create the table once in a different file?

<?php

/*
Database and mail functionality
*/

// get user credentials
$config = parse_ini_file('../config.ini'); // path may vary depending on setup

// Create connection
$conn = new mysqli('localhost', $config['username'], $config['password'],$config['dbname']);

// Check connection
if ($conn->connect_error){
    die('Connection failed. ' . $conn->connect_error);
}

// if table not made yet, create it
if(!$conn->query ("DESCRIBE visitors")) {
    // sql to create table
    $sql = 'CREATE TABLE visitors(
        id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY,
        name VARCHAR(30) NOT NULL,
        email VARCHAR(50) NOT NULL,
        message VARCHAR(500),
        reg_date TIMESTAMP
    )';

    if (!$conn->query($sql)){
        die ('Sorry there was an error. Please try again later.');
    }

}

//insert data into table
//clean data for SQL query
$name = mysqli_real_escape_string($conn, $data['name']);
$email = mysqli_real_escape_string($conn, $data['email']);
$message = mysqli_real_escape_string($conn, $data['message']);

$sql = "INSERT INTO visitors (name, email, message)
    VALUES ('$name', '$email', '$message')";

if ($conn->query($sql) === TRUE) {
    $usrMsg = 'Thank you we\'ll be in touch when we have some news';
} else {
    die ("Sorry, there was an Error. Please try again later.");
}

// close connection
$conn->close();

// email
// addresses and default subject
$to = ''; // add details
$from = ''; // add details
$subject = 'New form entry on website';

// prepare message variables - !not sure how to make quotes etc. display properly in email body
$message = wordwrap($data['message']);
$body = <<<_END
Name: {$data['name']}
Email: {$data['email']}
Message: $message
_END;

// send
mail($to, $subject, $body, $from);

?>
\$\endgroup\$
  • 1
    \$\begingroup\$ Good practice to use CREATE TABLE IF NOT EXISTS as opposed to a simple CREATE TABLE, since the former will only issue a warning on a duplicate table creation, while the latter will return an error. \$\endgroup\$ – user120636 Jun 6 '16 at 18:26
1
\$\begingroup\$

Since, as you mentioned, you are pretty new to this, i will start with some novice level advice:

You should refactor your code into seperated functions, both in order to seperate functionality with different purpose, and to be able to reuse.

function connect_db($config) 
{
// Create connection
    $conn = new mysqli('localhost', $config['username'], $config['password'],$config['dbname']);

    // Check connection
    if ($conn->connect_error){
        die('Connection failed. ' . $conn->connect_error);
    }
    return $conn;

}

function prepare_visitors_table($conn) 
{

    // if table not made yet, create it
    if(!$conn->query ("DESCRIBE visitors")) {
        // sql to create table
        $sql = 'CREATE TABLE visitors(
            id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY,
            name VARCHAR(30) NOT NULL,
            email VARCHAR(50) NOT NULL,
            message VARCHAR(500),
            reg_date TIMESTAMP
        )';

        if (!$conn->query($sql)){
            die ('Sorry there was an error. Please try again later.');
        }

    }

}

function save_visitor($conn, $name, $email, $message)
{

    //insert data into table
    //clean data for SQL query
    $name = mysqli_real_escape_string($conn, $name);
    $email = mysqli_real_escape_string($conn, $email);
    $message = mysqli_real_escape_string($conn, $message);

    $sql = "INSERT INTO visitors (name, email, message)
        VALUES ('$name', '$email', '$message')";
    return $conn->query($sql) ;
}

function send_visitor_notification($name, $email, $message)
{


    // email
    // addresses and default subject
    $to = ''; // add details
    $from = ''; // add details
    $subject = 'New form entry on website';

    // prepare message variables - !not sure how to make quotes etc. display properly in email body
    $message = wordwrap($message);
    $body = <<<_END
    Name: {$name}
    Email: {$name}
    Message: $message
    _END;

    // send
    mail($to, $subject, $body, $from);

}

$config = parse_ini_file('../config.ini'); // path may vary depending on setup

$conn = connect_db($config);

prepare_visitors_table($conn);
if (save_visitor($conn, $data['name'], $data['email'], $data['message'])) {
        $usrMsg = 'Thank you we\'ll be in touch when we have some news';
    } else {
        die ("Sorry, there was an Error. Please try again later.");
    }
send_visitor_notification($data['name'], $data['email'], $data['message']);


$conn->close();
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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