HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Processing a form with PHP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withformprocessingphp

Problem

I've seen few ways of processing forms with PHP. Whenever I am working with a form, I typically have two files: one for displaying the information (e.g. form.php) and the other for processing information (e.g. process_form.php). The latter uses sessions and simply redirects to the appropriate page depending on if the information submitted has been validated correctly or not. Below is a simple example of how I normally do things.

Form.php


  
  ">What is your name?
  " name="" 
    value="" maxlength="60" />
  
  ">
  

  
  ">What is your age?
  " name="" 
    value="" maxlength="2" />
  
    ">
  

  
  ">What is your email?
  " name="" 
    value="" maxlength="100" />
  
    ">
  

             
  


Process_form.php

$redirect_page = 'form.php';

$form   = array();
$errors = array();

if ($_SERVER['REQUEST_METHOD'] == 'POST') {

    $hid_submit = isset($_POST['hid_submit']) ? $_POST['hid_submit'] : NULL;

    if ($hid_submit) {

        $form['name'] = isset($_POST['name']) ? trim($_POST['name']) : '';
        $form['age'] = isset($_POST['age']) ? trim($_POST['age']) : '';
        $form['email'] = isset($_POST['email']) ? trim($_POST['email']) : '';

        // validate fields
        if (!valid($form['name'], 'name')) $errors['name'] = 'Please enter a valid name.';
        if (!valid($form['age'], 'age')) $errors['age'] = 'Please enter a valid age.';
        if (!valid($form['email'], 'email')) $errors['email'] = 'Please enter a valid email.';

        if (count($errors)) {
            $_SESSION['form']   = $form;
            $_SESSION['errors'] = $errors;
        } else {
            $redirect_page = 'another_page.php';
        }   
    }
}

redirect($redirect_page);


I find the following approach has worked well in most situations, however I am always looking for better ways to improve.

Solution

That method is fine, and is the one I use. As for a better way? I don't know, seems just fine to me. However, there are some other things that could be improved with this script, and that may be why you feel there might be a better way.

For one, that's some really messy HTML... I would try and separate your PHP from your HTML as much as possible. This is a little harder to do when not using MVC, but can still be accomplished by abstracting your larger bits of code. Also, you are using PHP short tags, but then not using PHP template syntax, this seems counter productive. The purpose of the short tags is to make your code cleaner and make it non-PHP user friendly. Of course, I'm not endorsing PHP short tags, I think you should avoid them, as you are not always guaranteed to have a server that will support them. Lastly, your code is pretty repetitive, the best thing for repetitive stuff like this is loops. For example, here's a sample of how my version of this form would look.


">What is your ?
" value="" maxlength="60" />

">


Of course, even this seems really messy to me, though I can't think of much more, besides using MVC, to do to this to make it cleaner. Also, notice I removed the "id" attribute from your input tag. When your tag has a name, it doesn't need an id, you can just call it with the name. Example:

alert( document.form.name.value );//may not work, dont think "form" is valid name


Take a look at PHP's filter_input() function. Its available as long as your PHP version is >= 5.2. It also has some validation flags if you're interested.

$form[ 'name' ] = filter_input( INPUT_POST, 'name', FILTER_SANITIZE_STRING );


Keep in mind the "Don't Repeat Yourself" (DRY) principle. There are a lot of violations of it here. I demonstrated how to avoid it with your form above, you can do something similar for the processing script.

Code Snippets

<?php
$fields = array(
    'name',
    'age',
    //etc...
);
?>

<?php foreach( $fields AS $field ) : ?>

<?php $value = isset( $_SESSION[ 'form' ] [ $field ] ) ? $_SESSION[ 'form' ] [ $field ] : ''; ?>

<label for="<?php echo $field; ?>">What is your <?php echo $field; ?>?</label>
<input type="text" name="<?php echo $field; ?>" value="<?php echo $value; ?>" maxlength="60" />

<?php if( ! isset( $_SESSION[ 'errors' ] [ $field ] ) ) : ?>

<label class="error" for="<?php echo $field; ?>"><?php echo $_SESSION[ 'errors' ] [ $field ]; ?></label>

<?php endif; ?>

<?php endforeach; ?>
alert( document.form.name.value );//may not work, dont think "form" is valid name
$form[ 'name' ] = filter_input( INPUT_POST, 'name', FILTER_SANITIZE_STRING );

Context

StackExchange Code Review Q#15013, answer score: 3

Revisions (0)

No revisions yet.