OOP Form Validator Class - a good way to architect?

PHP
1

I’m trying to find the best way to organize a validator class, hopefully using OOP best practices.

I thought a good might be to have a general validator class that I extend as the need arises. However, I am unsure of a good way to implement a validateAll method. I am not sure how to “reach” back into a parent class and get its validator methods in a way that wont cause me to have to constantly sync up the parent and child class and is programmatically succinct and extensible (I will want more methods in the child class eventually). Here is some sample code of the problem:




class GeneralValidator
{
	protected $name = null;
	protected $password = null;
	protected $errors = array();

	public function setName($name)
	{
		$this->name = $name;
	}

	public function validateName()
	{
		// stuff here to validate name, return true if okay, false if it fails validation or is null, and set a message in the errors array
	}

	public function setPassword($password)
	{
		$this->password = $password;
	}

	public function validatePassword()
	{
		// stuff here to validate password, return true if okay, return false if it fails validation or is null, and set a message in the errors array
	}

	public function validateAll()
	{
		if ($this->validateName($this->name) AND $this->validatePassword($this->password))
		{
			return true;
		}

		return false;
	}

}

class SpecificValidator extends GeneralValidator
{
	protected $email = null;

	public function setEmail($email)
	{
		$this->email = $email;
	}

	public function validateEmail()
	{
		// stuff here to validate password, return true if okay, return false if it fails validation or is null, and set a message in the error message array
	}

       public function validateAll()
       {
            // how to implement this one?
       }


}

But what if I want a “validateAll” method for SpecificValidator but I also want it to use all all validation methods in the child class as well? I understand it is good practice to make a common interface across related objects, so that all Validators can use a method like >validateAll(). Should I just hard code all the validation routines into SpecificValidator, or is there a better way?

2

Interesting idea. Id like to hear what the expert programmers have to say on this matter. I am not good at designing the architecture of validator class hierarchy either so it will be a good opportunity for me to learn a well.

3

You could probably use Reflection to locate all the methods that start with ‘validate’ and then call them sequentially…

This may be a bit overwhelming but you might want to read some of the documentation on the Symfony 2 validator just to get an idea of somethings you might want to implement. The Symfony 2 validator design is based on a Java design. Which may or may not be a good thing but it is widely used.

https://github.com/symfony/Validator

The “validate all” functionality is implemented by building an array of validators.


use Symfony\\Component\\Validator\\Validation;
use Symfony\\Component\\Validator\\Constraints as Assert;

$validator = Validation::createValidator();

$constraint = new Assert\\Collection(array(
    'name' => new Assert\\Collection(array(
        'first_name' => new Assert\\Length(array('min' => 101)),
        'last_name'  => new Assert\\Length(array('min' => 1)),
    )),
    'email'    => new Assert\\Email(),
    'simple'   => new Assert\\Length(array('min' => 102)),
    'gender'   => new Assert\\Choice(array(3, 4)),
    'file'     => new Assert\\File(),
    'password' => new Assert\\Length(array('min' => 60)),
));

$violations = $validator->validateValue($input, $constraint);
4

Let me show you the structure of the validation class that I use. It is pretty simple and it has worked pretty well for me. I don’t use this class to validate each individual value but to validate objects that contain all the values (usually from a form) to validate. In my case the objects are part of my simple ORM class but I this is not important here.

The base class:


class BaseValidator {
	
	protected $objToValidate;
	private $validationErrors;
	private $defaultErrorMsg;
	
	// types of predefined validators:
	const REQUIRED = 1;
	const UNIQUE = 2;
	const INT = 3;
	const EMAIL = 4;
	const NUMBER = 5;
	const POSITIVE_NUMBER = 6;
	
	public function __construct() {
		$this->defaultErrorMsg = array(
			self::REQUIRED => "This field must be filled in",
			self::UNIQUE => "This value is already in use",
			self::INT => "Positive integer value is required",
			self::EMAIL => "Invalid e-mail address format",
			self::NUMBER => "Number is required",
			self::POSITIVE_NUMBER => "Positive number is required",
		);
	}
	
	/**
	 * Validate single field value with one of the predefined validators.
	 * If not validated then add error to $this->validationErrors
	 *
	 * @param $fieldName string
	 * @param $type int One of the constants for predefined validator
	 * @param $message string Error message to be displayed or NULL for default message
	 * @param $params array Additional params for validators that requre them, e.g.
	 * min and max values for a range validator, etc.
	 * @return bool Whether value has been validated correctly
	*/
	public function validateField($fieldName, $type, $message = null, $params = null) {
		// here I use a switch statement on $type and perform all the necessary
		// validation checks using regular expressions and other methods
		// ...
	}
	
	/**
	 * Add error to this object (to $this->validationErrors).
	 * Used for manually validating what can't be validated
	 * using the predefined validators.
	 *
	 * @param $fieldName string
	 * @param $message string Error message to be displayed
	*/
	public function addError($fieldName, $message) {
		// ...
	}

	/**
	 * Get validation errors for all fields that didn't pass validation.
	 * The result is an array where keys are field names and values are
	 * validation messages.
	 *
	 * @return array|NULL
	*/
	public function getValidationErrors() {
		return $this->validationErrors;
	}
}

And when I create validators for specific objects I extend the base class. In my case I normally have just one class like this where I put a method for each object to validate - or multiple methods per object if I need different validator behaviour for different contexts like validating a user form in the administration and validating a user on the public site. You could have a separate validator class for each object if you prefer, of course:


class Validator extends BaseValidator {
	public function validateProductInAdmin(Product $product) {
		$this->objToValidate = $product;
		
		$this->validateField('name', self::REQUIRED);
		$this->validateField('name', self::UNIQUE, "Product name must be unique");
		$this->validateField('price', self::REQUIRED);
		$this->validateField('price', self::POSITIVE_NUMBER);
		
		// custom validator
		if ($product->discounted_price > $product->price) {
			$this->addError('discounted_price', "Discounted price cannot be higher than regular price");
		}
	}
	
	public function validatePage(Page $page) {
		$this->objToValidate = $product;
		
		$this->validateField('title', self::REQUIRED, "Page title must be provided");
		$this->validateField('content', self::REQUIRED);
	}
}

And this is how I use it:


// Let's assume $product is an object with values from form
$validator = new Validator;
$validator->validateProductInAdmin($product);
$errors = $validator->getValidationErrors();

if ($errors) {
	// display the form again and pass $errors to the template
	// so they can be displayed to the user next to appropriate fields
	// ...
}

// validation passed
// ...

The comments should explain it all. You could, of course, pass data to validate as array but objects are useful for some special cases, for example the unique validator needs to query the proper table in the database and it also needs to know whether you are inserting new data or editing existing data. You can embed all those necessary pieces of information in your object and pass it to the validator class and the class will do its magic for you.

5

I wonder what people think about using Reflection - personally, I don’t like it because Reflection is magic and magic means hidden mechanisms that are not immediately obvious when looking at the code - there are ‘methods’ that do not really exist and in order to know about their existence we have to either read through the documentation or delve into debugging the source code. Not nice. What benefits does really Reflection give us in this case? Can’t a validation class be designed without it?

I haven’t used the Symphony validation class and I’m sure it is very capable of what it is supposed to do. But looking at the code I get a feeling that there’s too much fluff for the simplest things - including the configuration style with metadata. And yes, it looks like Java style :). I’m curious about opinions of other people who used this.

6

It probably depends on which of the metadata styles you choose. The annotation style is very simple and easy.

class User
{
    /**
     * @Assert\\Length(min = 3)
     * @Assert\\NotBlank
     */
    private $name;

    /**
     * @Assert\\Email
     * @Assert\\NotBlank
     */
    private $email;
}
7

Ha, I was just wondering how these annotation are used and does it appear that my worst suspicions are true? I mean I am very much against using anything within comments to affect the behaviour of an application and this is what appears to be the case here! You change comments and the validators work differently? Not nice. Annotations should be used for annotating not for configuring. I remember TomB was writing about it somewhere and the reasoning looked very sound to me. I mean the metadata style indeed looks simple but in my opinion it just feels wrong :nono:

I generally smell overuse of Reflection here… :shifty:

8

I’m not a fan of annotations myself. Yaml works for me:


Acme\\BlogBundle\\Entity\\User:
    properties:
        name:
            - NotBlank: ~
            - Length { min: 3 }
        email:
            - NotBlank: ~
            - Email:
                message: The email "{{ value }}" is not a valid email.
                checkMX: true
            - DatabaseTableColumnValueDoesNotExist: { table: users, column: email }

And of course you can just use straight PHP as well if you really enjoy typing.

9

In principle, I totally get where you’re coming from. Comments shouldn’t be code. But in practice, the idea has been around for a long time now, and it’s proven itself to be a very handy way to provide contextual information, whether it’s Javadoc comments, JSHint configuration, or PHP annotations, among others.

10

Please consider making a Reflection thread. I think it would be a good topic.

11

Had my head underwater with work until today, but here’s an update on what I tried to do.

I first tried the reflection API but ran into errors.

I then tried generic class methods like so which seemed to have the functionality I needed:

(this method would be in the SpecificValidator class)


public function validateAll()
	{

		
		$class_name = get_class($this);
		
		$parent_class_name = get_parent_class($this);
		
		$current_class_methods[] = get_class_methods($this);
	
		$parent_class_methods[] = get_class_methods($parent_class_name); // array
		// combine parent and child classes methods
		$combined_methods_from_all_classes = array_merge($current_class_methods, $parent_class_methods);

		// It's necessary to flatten the array for the function array_unique to work or else this would be *much* easier and cleaner.

// Function to flatten array.
	
		function array_flatten($array) {
			if (!is_array($array)) {
				return false;
			}
			$result = array();
			foreach ($array as $key => $value) {
				if (is_array($value)) {
					$result = array_merge($result, array_flatten($value));
				} else {
					$result[$key] = $value;
				}
			}
			return $result;


		}
		$flattened_array = array_flatten($combined_methods_from_all_classes);
// get unique methods from all classes
		$unique_methods_from_all_classes = array_unique($flattened_array);
	

		// prepare the array
		$validation_methods = array();
// search for methods with validate in the name and add them to the $validation_methods array
		foreach ($unique_methods_from_all_classes as $method)
		{
			if (strstr($method, 'validate'))
			{
				$validation_methods[] = $method;
			}
		}

// run each validation method
		foreach ($validation_methods as $individual_validation_method)
		{
			if ($this->$individual_validation_method) // gives undefined property errors ?
			{
				return true;
			}
			else
			{
				echo $individual_validation_method; // the echo is for debugging
			}
		}


	}

The code would also need something to protect against running the validateAll function again (and thereby getting into a loop), but as I was getting undefined property errors from the last foreach (and couldn’t figure out why) I put this little project aside (work :() as the undefined property errors made me worry that this might not be possible since the properties were there, but they were just in a parent class. Also there was the problem that I didn’t know if the $this in the parent class’s methods was referring to the calling class (SpecificValidator) or the parent class itself (BaseValidator).

12

Perhaps something like:


class AbstractValidator
{
    public function validateAll()
    {
        $reflectionClass = new ReflectionClass($this);
        $methods = $reflectionClass->getMethods();
        foreach($methods as $method)
        {
            $methodName = $method->name;
            if (substr($methodName,0,strlen('validate')) == 'validate')
            {
                if ($methodName != 'validateAll')
                {
                    $this->$methodName();
                }
            }
        }
    }
}
class GeneralValidator extends AbstractValidator
{
    public function validateName()     { echo "Validating name\
"; }
    public function validatePassword() { echo "Validating password\
"; }
}
class SpecificValidator extends GeneralValidator
{
    public function validateEmail()    { echo "Validating email\
"; }
}
$validator = new SpecificValidator();
$validator->validateAll();

Output:
$ php validate.php
Validating email
Validating name
Validating password
13

I’ll try that right away - thanks! I hadn’t considered making an abstract class with the validateAll function. You opened my eyes here. (OOP is still pretty unintuitive to me.)

14

Come to think about it, I consider Form Validators as validators on the controller-side. I also heard that its also necessary to have validators on the model-side, although this means that at times you will be double-validating your data if the form data is used in model setter methods. I wonder though, is it really a good practice to have validators on both model and the controller side? And if so, how are model validators supposed to be implemented?

15

I think it’s very good practice to have validators on both the model and the controller side - provided you have time and resources to do it. From the security point of view it’s best to have another line of validation defence just before the data gets to the database. If the model is supposed to guard what gets written into the database it cannot rely on the controller to validate because it doesn’t even know if a controller exists so the model can’t really trust any incoming data to be validated. I think model validators should throw an exception if validation fails since this means something abnormal has happened in the application because bad data somehow sneaked through controller validation or got around it altogether.

If we use any kind of modern database we already get at least some minimal model validation done by the database - by using foreign keys, value constrains or even specific column data types. I would consider database validation being part of model validation. But here it can get even more complicated if we want to do it ideally because even if we do model validation in PHP bad data can get inserted in other ways so ultimately we would need to embed all model validation into the database. I don’t know if it’s really possible - maybe some advanced db systems would be able to do it.

But I think most online applications don’t need two kinds of validators - not enough benefits to warrant the additional work. If I were to write a banking application then sure this is the way to go but normally it’s just too much work.

16

Thanks for the explanation. I wonder though, in an application that you have validators on both model and the controller side, what responsibilities should go into these two different validators. In other words, how do I know if a certain validation belongs to the model side or the controller side? Is there a rule of thumb for this?