PHP form to database fit for purpose

Question

This code is intended to take input from a form and append it to a database table. The same data is sent to a separate function for subsequent mailing to the respondent. I'm self-taught and it works so overall I'm quite happy.

It just starts to seem a little flabby around the binding and creation of the $mail_params. I imagine there's no real performance cost here but are there more efficient ways of setting multiple variables?

As my first outing into MySQLi functions I would be grateful for a weather eye on the prepared statement and its suitability in preventing SQL injection.

function submit_form ($db_params) {

// connect
$mysqli = new mysqli($db_params['host'], $db_params['username'],
                     $db_params['password'], $db_params['dbname']);

// check
if ($mysqli->connect_errno) {
    echo "Failed to connect to MySQL: (" . $mysqli->connect_errno . ") " . $mysqli->connect_error;
    return false;
}

// prepare 
if (!($stmt = $mysqli->prepare("INSERT INTO interface_response(issue,
                                                               comment,
                                                               email,
                                                               reporting_organisation,
                                                               reporting_dept_prac,
                                                               causing_organisation,
                                                               causing_dept_prac)
                                VALUES (?, ?, ?, ?, ?, ?, ?)"))) {
    echo "Prepare failed: (" . $mysqli->errno . ") " . $mysqli->error;
    return false;
}

// bind
if (!$stmt->bind_param("sssssss", $issue, $comment, $email,
                                  $reporting_organisation, $reporting_dept_prac,
                                  $causing_organisation, $causing_dept_prac)) {
    echo "Binding parameters failed: (" . $stmt->errno . ") " . $stmt->error;
    return false;
}

$email = $_POST['email'];
$mail_params['email'] = $email;

$issue = $_POST['issue'];
$mail_params['issue'] = $issue;

$comment = $_POST['comment'];
$mail_params['comment'] = $comment;

$reporting_organisation = $_POST['reporting_organisation'];
$mail_params['reporting_organisation'] = $reporting_organisation;

$reporting_dept_prac = department_or_practice($_POST['reporting_department'],
                                              $_POST['reporting_practice']);
$mail_params['reporting_dept_prac'] = $reporting_dept_prac;

$causing_organisation = $_POST['causing_organisation'];
$mail_params['causing_organisation'] = $causing_organisation;

$causing_dept_prac = department_or_practice($_POST['causing_department'],
                                            $_POST['causing_practice']);
$mail_params['causing_dept_prac'] = $causing_dept_prac;

// execute
if (!$stmt->execute()) {
    echo "Execute failed: (" . $stmt->errno . ") " . $stmt->error;
    return false;
} else {
    return send_mail($mail_params);
}

// close
$stmt->close();

$mysqli->close();

}

Answer 1

First of, yes, prepared statements are the way to go to prevent SQL injection (and you are using them correctly).

Reducing Code Length

The assignment of POST values to a variable as well as an array takes a lot of space, and it makes your code hard to read. There are a lot of solutions for this, one would be to move the assignment before bind, and then use the array values for binding:

$mail_params['email'] = $_POST['email'];
$mail_params['issue'] = $_POST['issue'];
[...]
if (!$stmt->bind_param("sssssss", [...], $mail_params['email'], $mail_params['issue'], [...])) {
    [...]
}

If you create one more function, it might also be a bit better structured:

submitCommentForm() {
    [extract POST values]
    insertComment([extracted post values]);
    sendMail([same extracted post values]);
    [render result]
}

insertComment($db, $comment_values) {
    [the database stuff]
}

sendMail($comment_values) {
    [send mail]
}

Misc

• Your spacing and indentation isn't ideal, which makes your code a bit hard to read. You can probably use any IDE to fix this.
• Don't echo in a function (it makes your code very inflexible). Just throw an exception.
• Don't create your mysqli object in each function you need it, but pass it to the function as an argument (that way, you avoid duplicate code, and you make your functions more flexible and testable).