Posting required fields to database

PHP
1

Hi, I am currently following along the sitepoint PHP & mySQL book by Kevin Yank.

I decided in order to learn more, I would be better off creating my own fictional website so that I am more or less creating all the code. I decided to make a job listing site, which has similar functions to the joke site in the book but goes a bit further.

I have working code that takes the info entered in the form and posts it to the database. Some of the information is in the form of required fields and they go to two tables, “author” and “job”.

In order to further my learning, I was hoping someone could look at my code and see if there is an easier, less cluttered way to do this, so here it is:

//Add job listing to database
if (isset($_GET['add']))
{
    include $_SERVER['DOCUMENT_ROOT'] . '/studentjobs/includes/db.inc.php';
    include $_SERVER['DOCUMENT_ROOT'] . '/studentjobs/includes/helpers.inc.php';
    
    $error = ''; //Set $error to '' to avoid notice/error.
    
    if ($_POST['name'] == '')
    {
        $error = 'You must enter your name.';
        include 'addjob.html.php';
        exit();
    }
    if ($_POST['email'] == '')
    {
        $error = 'You must enter your e-mail address.';
        include 'addjob.html.php';
        exit();
    }
    if ($_POST['title'] == '')
    {
        $error = 'You must enter a job title.';
        include 'addjob.html.php';
        exit();
    }
    if ($_POST['description'] == '')
    {
        $error = 'You must enter a job description.';
        include 'addjob.html.php';
        exit();
    }
    
    $name = mysqli_real_escape_string($link, $_POST['name']);
    $email = mysqli_real_escape_string($link, $_POST['email']);
    $title = mysqli_real_escape_string($link, $_POST['title']);
    $description = mysqli_real_escape_string($link, $_POST['description']);
    
    $authorsql = "INSERT INTO author SET
        name='$name',
        email='$email'";
    
    if ($_POST['company'] != '')
    {
        $company = mysqli_real_escape_string($link, $_POST['company']);
        $sql .= ",company='$company'";
    }
    if ($_POST['phone'] != '')
    {
        $phone = mysqli_real_escape_string($link, $_POST['phone']);
        $sql .= ",phone='$phone'";
    }
    if ($_POST['address'] != '')
    {
        $address = mysqli_real_escape_string($link, $_POST['address']);
        $sql .= ",address='$address'";
    }
    
    if (!mysqli_query($link, $authorsql))
    {
        $error = 'Unable to add author to the database. ' . mysqli_error($link);
        include 'error.html.php';
        exit();
    }
    
    $authorid = mysqli_insert_id($link);
        
    $jobsql = "INSERT INTO job SET
        title='$title',
        description='$description',
        date=CURDATE(),
        authorid='$authorid'";
    if (!mysqli_query($link, $jobsql))
    {
        $error = 'Unable to add job listing to the database. ' . mysqli_error($link);
        include 'error.html.php';
        exit();
    }
    
    header('Location: .');
    exit();
    
}

I imagine the form that the information is taken from wont be needed but if it is let me know and I will post the code for that too.

As you can see it’s a bit all over the place, hoping you can help, thanks.

2

I would change the validation and error handling so that all the inputs are validated in one go andall the error messages are displyed at the end of validation rather than display an error and exit as soon as a single error is encountered.

Doing all the validation in one go ends up telling the user all the errors at the one time.

I would do something similar to this:

 
 
<?php
//Add job listing to database
if (isset($_GET['add']))
{
include $_SERVER['DOCUMENT_ROOT'] . '/studentjobs/includes/db.inc.php';
include $_SERVER['DOCUMENT_ROOT'] . '/studentjobs/includes/helpers.inc.php';
 
$errors = array();  //array containing any error messages
 
if ($_POST['name'] == '') {
 $errors[] = 'You must enter your name.';
 include 'addjob.html.php';
}
 
if ($_POST['email'] == '') {
 $errors[] = 'You must enter your e-mail address.';
 include 'addjob.html.php';
}

if ($_POST['title'] == '') {
 $errors[] = 'You must enter a job title.';
 include 'addjob.html.php';
}

if ($_POST['description'] == '') {
 $errors[] = 'You must enter a job description.';
 include 'addjob.html.php';
}
 
//display any error messages and then exit
if(count($errors) > 0) {
    foreach($errors as $errMsg) {
       echo '<p>'.$errMsg.'</p>';
    }
    die();
}
 
$name = mysqli_real_escape_string($link, $_POST['name']);
$email = mysqli_real_escape_string($link, $_POST['email']);
$title = mysqli_real_escape_string($link, $_POST['title']);
$description = mysqli_real_escape_string($link, $_POST['description']);
$authorsql = "INSERT INTO author SET
name='$name',
email='$email'";
if ($_POST['company'] != '') {
 $company = mysqli_real_escape_string($link, $_POST['company']);
 $authorsql .= ",company='$company'";
}
if ($_POST['phone'] != '') {
 $phone = mysqli_real_escape_string($link, $_POST['phone']);
 $authorsql .= ",phone='$phone'";
}
if ($_POST['address'] != '') {
 $address = mysqli_real_escape_string($link, $_POST['address']);
 $authorsql .= ",address='$address'";
}
if (!mysqli_query($link, $authorsql)) {
$error = 'Unable to add author to the database. ' . mysqli_error($link);
include 'error.html.php';
exit();
}
$authorid = mysqli_insert_id($link);
$jobsql = "INSERT INTO job SET
title='$title',
description='$description',
date=CURDATE(),
authorid='$authorid'";
if (!mysqli_query($link, $jobsql)) {
$error = 'Unable to add job listing to the database. ' . mysqli_error($link);
include 'error.html.php';
exit();
}
header('Location: .');
exit();
} 
?>
3 

if ($_POST['some_var'] == '')

can cause E_NOTICE errors when $_POST[‘some_var’] is not defined.

it’s better to use


if (isset($_POST['some_var']) && $_POST['some_var'] == '')
4

Building a bit on Kalons solution where he quite rightly argues you could collect up all the errors in an array, not just fail at the first one…



if ($_POST['name'] == '') {
 $errors[] = 'You must enter your name.';
 include 'addjob.html.php';
}
 
if ($_POST['email'] == '') {
 $errors[] = 'You must enter your e-mail address.';
 include 'addjob.html.php';
}
 
if ($_POST['title'] == '') {
 $errors[] = 'You must enter a job title.';
 include 'addjob.html.php';
}
 
if ($_POST['description'] == '') {
 $errors[] = 'You must enter a job description.';
 include 'addjob.html.php';
}

One thing screaming at me, and should you too, is that if you alter your target include page name, you have to change it now in 4 places.

In principle I’d try and refactor that so it is only kept in one place, maybe somewhere more obvious, like at the top of the page.


$add_job_url = 'addjob.html.php';

// checking ....

//.. then finally

if( count( $errors ) ){
include $add_job_url ;
}

untested

That might seem picky, but its a good way to start thinking - especially if yourself working on a similar form with say, a dozen input form elements.

HTH