Form validator class and child classes

Question

I am creating a form class for validating form data, and would like some advice on how to refactor it for best practices. Keep in mind that this is a rough draft.

Basically, there is a form abstract class (lev_form), which you would extend. In the extended classes, you would write a __construct method that creates all of the form fields, and form field validations. You could then instantiate that extended class, and run its validate_form() method to validate $_POST and $_FILES data against all of the child form fields, and their child form field validations.

It somewhat mimics the way jQuery form validation is operated, and the eventual plan is to give the class the ability to print out a jQuery validation class for the form, based on the same data that the backend validation uses.

Here is the form class:

<?php

    abstract class lev_form {

        protected $fields = array();

        abstract public function __construct() {
            // set up form fields and field vlaidations here

        }

        protected function create_field($name, $label) {
            lev::load_library('lev_forms/lev_field');
            $this->fields[] = new lev_field($name, $label);
        }

        // validates POST and FILES data
        public function validate_form() {
            try {
                foreach($this->fields as $field_object) {
                    $field_object->validate_field();
                }
            } catch (Exception $e) {
                return $e->getMessage();
            }
            return true;
        }

        // validates form using jQuery
        public function print_jquery_validator($selector) {

        }
    }
?>

Here is the class for form fields:

<?php

    class lev_field {

        protected $name;
        protected $label;
        protected $validations = array();

        public function __construct($name, $label) {
            $this->name = $name;
            $this->label = $label;
        }

        public function create_validation($type, $argument = null, $message = null) {
            lev::load_library('lev_forms/lev_field_validation');
            $this->validations[] = new lev_field_validation($type, $argument, $message, $this->name, $this->label);
        }

        public function validate_field() {
            foreach ($this->validation as $validation_object) {
                $validation_object->validate();
            }
        }
    }
?>

And here is the form field validation class:

<?php

    class lev_field_validation {

        protected $type;
        protected $argument;
        protected $message;
        protected $field_name;

        protected static $default_validation_error_messages = array(
            'required' => '[field_label] is a required field.',
            'minlength' => '[field_label] must be at least [validation_argument] characters in length.',
            'maxlength' => '[field_label] must be at most [validation_argument] characters in length.',
            'min' => '[field_label] must be at least [validation_argument].',
            'max' => '[field_label] must be at most [validation_argument].',
            'email' => '[field_label] must be a valid email address.',
            'url' => '[field_label] must be a valid URL.',
            'date' => '[field_label] must be a valid date.',
            'number' => '[field_label] must be a numeric value.',
            'digits' => '[field_label] must only contain digits.',
            'boolean' => '[field_label] must be true or false.',
            'equalto' => '[field_label] must be equal to [validation_argument].',
            'file_accept_extensions' => '[field_label] must be a valid file type.',
            'file_max_size' => '[field_label] must have a file size no greater than [validation_argument] bytes.'
        );

        public function __construct($type, $argument, $message, $field_name, $field_label) {
            $this->type = $type;
            $this->argument = $argument;
            $this->field_name = $field_name;
            if ($message) {
                $this->message = $message;
            } else if (array_key_exists($type, self::$default_validation_error_messages)) {
                $this->message = preg_replace(array('\[field_label\]', '\[validation_argument\]'), array($field_label, $argument), self::$default_validation_error_messages[$type]);
            } else {
                trigger_error('No set error message or default error message for form validation "' . $type . '"', E_USER_ERROR);
            }
        }

        public function validate() {
            $this->error_check_validation();
            $this->check_validation();
        }

        protected function error_check_validation() {
            if (array_search($this->field_name, $_POST) === true) {
                if ($this->type == 'file_accept_extensions' || $this->type == 'file_max_size') trigger_error('Use of invalid form validation "' . $this->type . '" on non-file field.', E_USER_ERROR);
            } else if (array_search($this->field_name, $_FILES) === true) {
                if ($this->type != 'file_accept_extensions' && $this->type != 'file_max_size') trigger_error('Use of invalid form validation "' . $this->type . '" on file field. You may only use this validation on non-file fields.', E_USER_ERROR);
            } else {
                trigger_error('Form field "' . $this->field_name . '" not found in $_POST or $_FILES array even though it exists in this form\'s class.', E_USER_ERROR);
            }
        }

        protected function check_validation() {
            switch ($validation->type) {
                case 'required':
                    if (!$this->required()) throw new Exception($this->message);
                    break;
                case 'callback':
                    if ($_POST[$this->field_name] !== '' && $this->argument($_POST[$this->field_name])) throw new Exception($this->message);
                    break;
                case 'minlength':
                    if ($_POST[$this->field_name] !== '' && strlen($_POST[$this->field_name]) < $this->argument) throw new Exception($this->message);
                    break;
                case 'maxlength':
                    if ($_POST[$this->field_name] !== '' && strlen($_POST[$this->field_name]) > $this->argument) throw new Exception($this->message);
                    break;
                case 'min':
                    if ($_POST[$this->field_name] !== '' && $_POST[$this->field_name] > $this->argument) throw new Exception($this->message);
                    break;
                case 'max':
                    if ($_POST[$this->field_name] !== '' && $_POST[$this->field_name] < $this->argument) throw new Exception($this->message);
                    break;
                case 'email':
                    if ($_POST[$this->field_name] !== '' && !filter_var($_POST[$this->field_name], FILTER_VALIDATE_EMAIL)) throw new Exception($this->message);
                    break;
                case 'url':
                    if ($_POST[$this->field_name] !== '' && !filter_var($_POST[$this->field_name], FILTER_VALIDATE_URL)) throw new Exception($this->message);
                    break;
                case 'date':
                    if ($_POST[$this->field_name] !== '' && !strtotime($_POST[$this->field_name])) throw new Exception($this->message);
                    break;
                case 'number':
                    if ($_POST[$this->field_name] !== '' && !is_numeric($_POST[$this->field_name])) throw new Exception($this->message);
                    break;
                case 'digits':
                    if ($_POST[$this->field_name] !== '' && !is_int($_POST[$this->field_name]) && $_POST[$this->field_name] >= 0) throw new Exception($this->message);
                    break;
                case 'boolean':
                    if ($_POST[$this->field_name] !== '' && !filter_var($_POST[$this->field_name], FILTER_VALIDATE_BOOLEAN)) throw new Exception($this->message);
                    break;
                case 'equalto':
                    if ($_POST[$this->field_name] !== '' && $_POST[$this->field_name] !== $_POST[$this->argument]) throw new Exception($this->message);
                    break;
                case 'file_accept_extensions':
                    $extension = preg_replace('/.+?\.(.+)$/', '$1', $_FILES[$this->field_name]['name']);
                    if (array_search($extension, $this->argument) === false) throw new Exception($this->message);
                    break;
                case 'file_max_size':
                    if (filesize($_FILES[$this->field_name]['tmp_name']) > $this->argument) throw new Exception($this->message);
                    break;
                default:
                    trigger_error('Validation "' . $this->type . '" does not exist. This validation was called by field "' . $this->field_name . '".', E_USER_ERROR);
            }
            return false;
        }

        protected function required() {
            if ($this->argument && !$_POST[$this->argument]) return true;
            return ($_POST[$this->field_name] === '') ? false : true;
        }
    }
?>

Answer 1

I jumped straight down to lev_field_validation as it's the most interesting class.

1. Whenever I see a giant switch() block, I immediately suspect there are a lot of subclasses trying to break out. Each validation type should be a different subclass with an appropriate override of check_validation() (and error_check_validation() in a couple cases).
2. You repeat $_POST[$this->field_name] !== '' twelve times which is a sure sign that this should be encapsulated in its own method. For one thing, you should check if the field exists using array_key_exists() before grabbing its value.
3. The idiom $x ? false : true can be replaced by !$x. The second line of required() thus becomes return $_POST[$this->field_name] !== '';.
4. Add some high-level comments explaining what is going on overall. For example, given its name I would expect required() to tell me if the value is required, yet it appears to mean that the value was passed in and must be validated. Shouldn't a "> 5" validation fail if I don't provide a value? If not, explain why or at least point that out.

Answer 2

• The lack of comments is definitely a concern.
• The code under the case statements is relatively unreadable, and contains a great deal of duplication. Think about how to rewrite this without so much duplicated code. The previous response suggests subclassing, I would suggest avoiding OO entirely.
• The fact that you've opted to use OO is measurably adding a lot of pretty pointless bloat to your real code. Consider implementing the same thing without OO and comparing the results as a personal exercise.
• Architectural concern: what if the user wants to perform custom validation?
• Your code is not internationalisation aware. While it's good to see that you have a dedicated structure for most of the messages you have written, the triggererror() lines contain user-oriented strings right in to your code, which prevents easy internationalisation.