Validator Readability and Efficiency

Question

This is my first validator I have created myself and I was wondering if there is anything about it that I could improve as far as:

• Code readability
• Code efficiency

Note: The reason why I put an underscore before each validation method name is so that non-validation methods cannot be called. E.g. setRule() can't be called if it's specified in the rule unless there is an actual validation method called "_setRule".

Any other suggestions are also welcome!

Validator class:

class Validator
{
    private $rules  = [];
    private $errors = [];

    public function setRule($id, Array $methods)
    {
        if (!array_key_exists($id, $this->rules)) {
            $this->rules[$id] = $methods;
        } else {
            throw new Exception('Rule "' . $id . '" already exists.');
        }
    }

    public function setRules(Array $rules) {
        foreach ($rules as $key => $value) {
            $this->setRule($key, $value);
        }
    }

    public function validate($rule, $input)
    {
        foreach ($this->rules[$rule] as $key => $value) {
            if (is_int($key)) { //$key => rule
                $value     = '_' . $value;
                $method    = $value;
                $arguments = [$input];
            } else {            //rule => [args]
                $key       = '_' . $key;
                $method    = $key;
                $arguments = array_merge([$input], is_array($value) ? $value : [$value]);
            }

            return call_user_func_array([$this, $method], $arguments);
        }
    }

    public function setError($error)
    {
        return $this->errors[] = $error;
    }

    public function getErrors()
    {
        return $this->errors;
    }

    private function _required($input)
    {
        return empty($input) ?: $this->errors[] = substr(__FUNCTION__, 1);
    }

    private function _min($input, $min)
    {
        return mb_strlen($input, 'utf8') >= $min ?: $this->errors[] = substr(__FUNCTION__, 1);
    }

    private function _max($input, $max)
    {
        return mb_strlen($input, 'utf8') <= $max ?: $this->errors[] = substr(__FUNCTION__, 1);
    }

    private function _between($input, $min, $max)
    {
        if ($return = $this->_min($input, $min) !== true) {
            return $this->errors[] = $return;
        }

        if ($return = $this->_max($input, $max) !== true) {
            return $this->errors[] = $return;
        }

        return true;
    }

    private function _email($input)
    {
        $sanitizedInput = filter_var($input, FILTER_SANITIZE_EMAIL);
        $vInput         = filter_var($sanitizedInput, FILTER_VALIDATE_EMAIL);

        return $input === $sanitizedInput && $vInput ?: $this->errors[] = substr(__FUNCTION__, 1);
    }

    private function _alnum($input)
    {
        return ctype_alnum($input) ?: $this->errors[] = substr(__FUNCTION__, 1);
    }
}

Sample usage:

$v = new Validator();

$v->setRule('email', ['required', 'between' => [5, 255], 'email']);
$v->setRule('password', ['required']);

$vEmail        = $v->validate('email', $_POST['email']);
$vEmailConfirm = $v->validate('email', $_POST['emailConfirm']);
$vPassword     = $v->validate('password', $_POST['password']);

if ($vEmailConfirm === true && $_POST['email'] !== $_POST['emailConfirm']) {
    $vEmailConfirm = $v->setError('match');
}

if (!$v->getErrors()) {
    # Input(s) have passed validation
} else {
   # Input(s) have not passed validation
   # $vEmail, $vEmailConfirm and $vPassword contain the errors
   # You can do something with that, like displaying an error message
}

Answer 1

Although you used a Validator class, this is not really object orientated. Actually I'm missing a Rule class. Your rules are applied by comparing strings right now, which is error prone and really ugly if you need to refactor it. Extending your code with new rules also requires changing the Validator itself.

class Rule {
     /**
      * @return bool
      */
     abstract function check($value);
     /**
      * @return string
      */
     abstract function getErrorMessage($value)
}

class BetweenRule extends Rule{ //Maybe there is also an IntegerRule to collect some operations.
     private $min;
     private $max;

     public function __construct ($min,$max) {...}

     public check($value)
     {
          return this->min<=$value && $this->max>=$value;
     } 

     public function getErrorMessage($value) { return $valus." is not between ..." ;}
}
...

And the usage:

$v = new Validator();
$r=new RequiredRule();

$v->setRule('email', [$r, new BetweenRule(5,255), new EmailRule()]);
$v->setRule('password', [$r]);

if (!$v->getErrors()) {  //either as a list of string or list of map fields and rules
     ...
} else {
     ...
}

As you can't type hint the content of an array, you might also want to replace setRule by addRule and append new rules to the existing ones, instead setting the array of rules.

$v = new Validator();
$v->addRule('email', new RequiredRule());
$v->addRule('email', new BetweenRule(5,255));
$v->addRule('email', new EmailRule());