Form validation for sporting events

Question

I am coding a site where users submit sporting events.

When submitting an event there are a lot of fields that need to be validated.

This is the method I wrote to valid the fields:

class event{

        private $dbh;
        private $post_data;


        public function __construct($post_data, PDO $dbh){
                $this->dbh = $dbh;
                $this->post_data = array_map('trim', $post_data);

        }
        // long ass method that checks all the input fields are valid 
        public function checkValidInput(){
            if(!empty($this->post_data['event-name']) && (strlen($this->post_data['event-name']) < 101)){
                if(!empty($this->post_data['event-description']) && (strlen($this->post_data['event-description']) < 2001)){
                    if(!empty($this->post_data['event-location-name']) && (strlen($this->post_data['event-location-name']) < 51)){
                        if(!empty($this->post_data['event-location-town']) && (strlen($this->post_data['event-location-town']) < 21)){
                            if(isset($this->post_data['event-county'])){
                                if(!empty($this->post_data['event-postcode']) && (strlen($this->post_data['event-postcode']) < 21)){
                                    if(isset($this->post_data['event-age-restriction'])){
                                        if(!empty($this->post_data['event-booking-address']) && (strlen($this->post_data['event-booking-address']) < 301)){
                                            if(!empty($this->post_data['event-organiser']) && ($this->post_data['event-organiser'] < 51) ){
                                                if(isset($this->post_data['event-select-difficulty'])){
                                                    if(isset($this->post_data['event-select-type'])){
                                                        if(isset($this->post_data['event-select-environment'])){
                                                            if(isset($this->post_data['event-select-traffic'])){
                                                                return TRUE;
                                                            }else{
                                                                throw new Exception('Please select the type of traffic.');
                                                            }
                                                        }else{
                                                            throw new Exception('Please select the type of environment.');
                                                        }
                                                    }else{
                                                        throw new Exception('Please select the type of event.');
                                                    }
                                                }else{
                                                    throw new Exception('Please select a difficulty rating for this event.');
                                                }
                                            }else{
                                                throw new Exception('Please enter an organiser name that is no longer than 50 characters.');
                                            }
                                        }else{
                                            throw new Exception('Please enter the booking page url for this event. Or the home website url.');
                                        }
                                    }else{
                                        throw new Exception('Please select appropriate age restriction.');
                                    }
                                }else{
                                    throw new Exception('Please enter a location post code that is no longer than 10 characters.');
                                }
                            }else{
                                throw new Exception('Please select a location county.');
                            }
                        }else{
                            throw new Exception('Please enter a location town name that is no longer than 20 characters.');
                        }
                    }else{
                        throw new Exception('Please enter a location name that is no longer than 50 characters.');
                    }
                }else{
                    throw new Exception('Please enter a description that is no longer than 2000 characters.');
                }
            }else{
                throw new Exception('Please enter a name that is no longer than 100 characters.');
            }
        }
    }

As you can see its a pretty big, ugly looking block of code.

Is there a prettier way of writing this that could half the amount of code?

Answer 1

Yeah that's ugly :)

You do not have to nest the ifs because when you throw an error, the function will stop executing right there so you can safely do

try {
    if(!empty($this->post_data['event-name']) && (strlen($this->post_data['event-name']) <   101)){
        throw new Exception("Event name is not valid");
    }
    if((strlen($this->post_data['event-description']) < 2001){
        throw new Exception('event-description is not valid'); 
    }
    return TRUE;
} catch (Exception $e) {
    $error = 'Caught exception: ',  $e->getMessage(), "\n";
    return FALSE;
}

But in most cases you want to collect all errors and send all of them errors to client

$errors = array();
if(!empty($this->post_data['event-name']) && (strlen($this->post_data['event-name']) <   101)){
   $errors[] = "Event name is not valid"; // push to the array
}
if((strlen($this->post_data['event-description']) < 2001){
   $errors[] = "Event description is not valid";
}
if(sizeof($errors) === 0) {
    return TRUE;
}else{
    // do something with errors
}

Answer 2

Using exceptions to handle this type or error are generally not preferred. Exceptions should be reserved for situations that are generally unrecoverable such as: no disc space, mathematical errors (divide by 0), network resources are unavailable or perhaps a database schema is missing an expected column. User input can generally be 'fixed'.

A goal of cutting the code in half isn't always the best idea if it sacrifices clarity.

An improved solution is to implement some type of grouped form filtering and validation. Many frameworks have a form class which usually allows for attaching a set of filters to remove unwanted input as well as validators to ensure that the remaining input conforms to your expectations. Let's use your 'event-name' as an example. Such a setup could work roughly like:

$form = new Form();
$eventNameField = new FormField('event-name');
$eventNameField->setFilters(array('strip-tags', 'string-trim')); //Exclude any tags and whitespace
$eventNameField->setValidator('alpha'); //Only allow A-Z (upper or lower) as the name
$eventNameField->setValidator(
    array(
        'length' => array(
            'min' => 3, 
            'max' => 32
        )
    )
); //Only allow names between 3 and 32 characters
$form->addField($eventNameField); //Add this field, with this set of filters/validators to the form

//Assuming you are receiving the POST data
$form->setPostData($_POST); //The Form class will handle setting the correct $_POST data in the correct field.
if ($form->isValid()) {
    //Process data
} else {
    //There are errors in the form
    $this->view->errors = $form->getErrors();
}

//Assuming you are in the view
foreach ($this->errors as $error) {
    echo $error . "\n";
}

There are other components that allow for even more flexibility in the use of such a design. Setting up fields individually and then pulling a set together to compose a form allows for reuse of a configured field. There are few cases where you would want to have different filters and validators for a given form field. The requirements for an event name should be the same in one part of the application as in another.