Validation system asking for advice ....

[whisher06]

Hi.
This is a rough copy of the system
I would implements:

<?php
error_reporting(E_ALL | E_STRICT);
define("IS_REQUIRED", '"%s" è un campo obbligatorio') ;
define('NOT_NUM', '"%s" non contiene un numero valido') ;
class ValidatorFactory{
	public function createValidator($validator,$field,$value){
		if(!class_exists($validator)){
		throw new Exception('Invalid object parameters');
	}
   		return new $validator($field,$value);
   }
}
abstract class AbstractValidator{
	public static $error= array();
	abstract function validate();
	public function setError($msg){
		self::$error[]= $msg;
	}
	public static function getErrors(){
        return self::$error;
    }
	public static function getError(){
        return array_shift(self::$error);
    }
	public static function isValid(){
    	return !(bool)count(self::$error) ;
  	}
}//
class IsEmpty extends AbstractValidator{
	private $value= '';
    private $field= '';
	public function __construct($field,$value){
		$this->field= $field;
		$this->value= $value;
	}
	public function validate(){
		if(empty($this->value)){
			parent::setError(sprintf(IS_REQUIRED, $this->field));
		}
	}
}//
class IsNumeric extends AbstractValidator{
	private $value= '';
    private $field= '';
	public function __construct($field,$value){
		$this->field= $field;
		$this->value= $value;
	}
	public function validate(){
		if(!is_numeric($this->value)){
			parent::setError(sprintf(NOT_NUM, $this->field));
		}
	}
}//
try{
  	$valFactory=new ValidatorFactory();
	$valFactory->createValidator('IsEmpty','Username','')->validate();
	$valFactory->createValidator('IsNumeric','Age','xxx')->validate();
	if(AbstractValidator::isValid()){
		//insert data
	}
	else{
		foreach(AbstractValidator::getErrors() as $error)
		{
			echo "<p>".$error."</p>";
		}
	}
}
catch(Exception $e){
  echo $e->getMessage();
  exit();
}


?>

What do you think about ?

Bye.

[Shrike]

Just a couple of things which come to mind:

• validate() in each class does not have a return value - to my mind it should return true or false. Perhaps include a getError() method for the error message.

• Static methods: why bother? They just limit reusability. It would be preferable to use instance methods.

• Exceptions: how about using using a typed Exception rather than the standard Exception class - this would allow client code to distinguish between different types of exception.

[whisher06]

- Exceptions: how about using using a typed Exception rather than the standard Exception class - this would allow client code to distinguish between different types of exception.

Thanks for the tip 😉

- validate() in each class does not have a return value - to my mind it should return true or false. Perhaps include a getError() method for the error message.
- Static methods: why bother? They just limit reusability. It would be preferable to use instance methods.

If I don't use static method/property I've got
a system like this:

<?php
error_reporting(E_ALL | E_STRICT);
define("IS_REQUIRED", 'The following (required) field is empty : %s') ; 
define('NOT_NUM', 'The following field isn\'t a number: %s') ; 
class ValidatorFactory{ 
    public function createValidator($validator,$field,$value){ 
        if(!class_exists($validator)){ 
        throw new Exception('Invalid object parameters'); 
    } 
           return new $validator($field,$value); 
   } 
}
abstract class AbstractValidator
{
	protected $error;
    function __construct(){}
	abstract function validate();
	abstract function setError($msg);
	public function getErrors(){
        return $this->error;
    }
}//
class IsEmpty extends AbstractValidator{
	private $value;
	private $field;
	public function __construct($field,$value){
		$this->field = $field;
		$this->value = $value;
	}
	public function validate(){
		$isValid = TRUE;
		if(empty($this->_value))
		{
			$isValid = FALSE;
			$this->setError(sprintf(IS_REQUIRED, $this->field));
		}
		return $isValid;
	}
	public function setError($msg){
		$this->error = $msg;
	}
}//
class IsNumeric extends AbstractValidator{
	private $value;
	private $field;
	public function __construct($field,$value){
		$this->field = $field;
		$this->value = $value;
	}
	public function validate(){
		$isValid = TRUE;
		if(!is_numeric($this->value))
		{
			$isValid = FALSE;
			$this->setError(sprintf(NOT_NUM, $this->field));
		}
		return $isValid;
	}
	public function setError($msg){
		$this->error = $msg;
	}
}//
$objs= array();
$valFactory=new ValidatorFactory(); 
$objs[] = $valFactory->createValidator('IsEmpty','Username',''); 
$objs[] = $valFactory->createValidator('IsNumeric','Age','xxx'); 
$errors= array();
foreach($objs as $obj){
	if(!$obj->validate())
	{
		 $errors[]= $obj->getErrors()."<br>";
	}
}

if((bool)count($errors)){
	foreach($errors as $err){
		echo $err;
		}
		exit();
}
else{
	// insert data
}
?>

It was also an old thread, you took part in 😉

I find the former useful even if I agree with you
that static methods limit the code reusability.

Have you got any tips ?

Bye.

[Shrike]

You might encapsulate the collection of validators with another class, e.g.

class ValidatorCollection
{
  private
    $validators, $errors;
  public function add( Validator $v )
  {
    $this->validators[] = $v;
  }
  public function getErrors()
  {
    return $this->errors;
  }
  public function isValid()
  {
    $valid = true;
    foreach( $this->validators as $v )
    {
      if( !$v->isValid() )
      {
        $valid = false;
        $this->errors[] = $v->getError();
      }
    }
    return $valid;
  }
}
$collection = new ValidatorCollection;
$collection->add( new IsEmptyValidator( 'Not null' ));
$collection->add( new IsEmptyValidator( null ));
if( $collection->isValid() )
{
  // Continue processing
}
else
{
  // Do something with $collection->getErrors()
}

If I was using this with an HTML form, I would want to associate values with keys. You might want to work that in somehow too.

[whisher06]

Thanks very much buddy 😉

Waiting for tips I made quite the same
think:

<?php
error_reporting(E_ALL | E_STRICT);
define("IS_REQUIRED", 'The following (required) field is empty : %s') ; 
define('NOT_NUM', 'The following field isn\'t a number: %s') ;
class InvalidArgException extends Exception{
   	public function __construct($message, $code = 0){
      parent::__construct($message, $code);
  	}
	public function __toString(){
       return " ".__CLASS__ . "<br /> Line : <b>[{$this->line}]</b><br /> Code :<b>[{$this->code}]</b><br /> Msg : {$this->message}<br /> File : <b>{$this->file}</b>\n";
   	}
} 

abstract class ValidatorFactory{
	private static $errors= array(); 
    public static function createValidator($validator,$field,$value){ 
        if(!class_exists($validator)){ 
       		throw new InvalidArgException("Invalid object parameters class {$validator} doesn't exist in class <b>[".__CLASS__."]</b>"); 
    } 
		$validator= new $validator($field,$value);
		if(!$validator->validate()){
 			self::$errors[]= $validator->getErrors();
		}
    } 
 	public static function isValid(){
		 return !(bool)count(self::$errors);
	}
	public static function getValidators(){
		return self::$errors;
	}
}

abstract class AbstractValidator{
	protected $error= '';
    abstract protected function validate();
	abstract protected function setError($msg);
	public function getErrors(){
        return $this->error;
    }
}//

class IsEmpty extends AbstractValidator{
	private $value= '';
	private $field= '';
	public function __construct($field,$value){
		$this->field= $field;
		$this->value= $value;
	}
	public function validate(){
		$isValid= TRUE;
		if(empty($this->value))
		{
			$isValid= FALSE;
			$this->setError(sprintf(IS_REQUIRED, $this->field));
		}
		return $isValid;
	}
	public function setError($msg){
		$this->error= $msg;
	}
}//

class IsNumeric extends AbstractValidator{
	private $value= '';
	private $field= '';
	public function __construct($field,$value){
		$this->field= $field;
		$this->value= $value;
	}
	public function validate(){
		$isValid= TRUE;
		if(!is_numeric($this->value))
		{
			$isValid= FALSE;
			$this->setError(sprintf(NOT_NUM, $this->field));
		}
		return $isValid;
	}
	public function setError($msg){
		$this->error= $msg;
	}
}//

try{
	ValidatorFactory::createValidator('IsEmpty','Username',''); 
	ValidatorFactory::createValidator('IsEmpty','Username2',''); 
	ValidatorFactory::createValidator('IsNumeric','Age','xxx'); 

if(!ValidatorFactory::isValid()){
	foreach(ValidatorFactory::getValidators() as $err){
		echo $err."<br />";
	}
		exit();
}
else{
	// insert data
}
}
catch(InvalidArgException $e){ 
  echo $e; 
  exit(); 
} 
?>

But all in all I prefer your code 😉

Bye.

[whisher06]

Yeah, far better your solution 😃
Thanks again.

Bye.

Taken http://www.blogial.net/index.php?cat=3 😉