Validator Class in PHP

Question

I am trying to create a validation class that can be used to validate many forms of my webpage in general. It makes use of the database as well to verify records.

The problem with it is that it looks bad and somewhat inefficient. I couldn't find any alternatives and I have used too many if-else if statements. And also dob makes use of an arbitrary variable. Overall the code seems bad, I know. But I couldn't find an alternative. Any help is extremely appreciated. I would also be happy if you could comment on the design and efficiency. I am willing to rewrite the whole code needed. This works, but it's all messed up. Thanks!

    Class Validate {
    //Take the user input into the data put it to a class.
    protected $data; //Function argument
    private $err; // boolean    
    protected $empty; 
    private $error;
    private $clean;
    private $query;
    public function __construct() { 
        $this->query= new DBhandler();
    }

    private function basicSanitize($data) { //Basic level input santization to be used by other functions only.

        $data=htmlspecialchars($data,  ENT_QUOTES);
        $data=trim($data);    
        return $data;
    }

    public function sanitize($data) { // Sanitization at a massive level to sanitize a lot of inputs at one go.
        foreach ($data as $k =>$v) {
            $data[$k]= $this->basicSanitize($v);
        }

        return $this->clean=$data;
    }

    public function showError($type,$data) {
        return $this->error[$type.$data];
    }

    private function basicEmptyCheck($data) {

        if($data==""|| empty($data) || !isset($data) || $data==-1) {
            return true;
        }
    }
    public function isEmpty($data) {
        foreach ($data as $k =>$v) {
            if($this->basicEmptyCheck($v)){
                $this->error['empty']="There is an empty field";
                return true;
            }
        }
    }

    private function checkLength($data,$min,$max) {

        if(strlen($data)>=$min && strlen($data)<=$max) {
            return true;
        }
    }

    private function lengthErr($data,$min,$max) {
        if(strlen($data)>$max) {
            return "too long";
        }

        else if (strlen($data)<$min) {
            return "too short";
        }
    }

    public function name($data) {

        if(!$this->checkLength($data,3,100)) {
            $this->error['name'.$data]="Name ".$this->lengthErr($data,3,100);
        }

        else  if(!preg_match("/^[a-zA-Z ]+$/",$data)) {
            $this->error['name'.$data] = "Please don't include special characters in names";
        }

        else return true;
    }


    private function isUniqueEmail($data) {

        $sql= "SELECT stuff1 FROM tablename WHERE stuff2=?";
        if(!$this->query->checkIfExists($sql, "s", 'jells')) {
            return true; //the value set here "jells" is for testing purposes only. This will be replaced by $data
        }
    }


    public function email($data) {

        if (!$this->checkLength($data,8,254)) {
            $this->error['email'.$data]= "Enter a valid email";
        }
        else if (!filter_var($data, FILTER_VALIDATE_EMAIL)) {
            $this->error['email'.$data] = "Enter a valid email"; 
        }
        else if (!$this->isUniqueEmail($data)) {
            $this->error['email'.$data]= "This email already exists";
        }


        else return true;
    }

    public function password($data) {

        if (!$this->checkLength($data,8,128)) {
            $this->error['pass'.$data]='Password '.$this->lengthErr($data, 8, 128);
        }
        else if (count(array_keys($this->clean,$data))>1) {
            $this->error['pass'.$data]='Password is too obvious';
        }
        else return true;        
    }

    private function ageCheck($month,$day,$year,$limit) {
        # object oriented
        $date=$year."-".$month."-".$day;
        $from = new DateTime($date);
        $to   = new DateTime('today');
        if($some=$from->diff($to)->y>=$limit) {
            return 1;
        }
    }

    public function age($data,$month, $day, $year,$limit) {

        if(!is_numeric($day) || !is_numeric($month) || !is_numeric($year)) {
            $this->error['age'.$data] = "Please enter a valid date";
        }
        else if(!checkdate($month, $day, $year)){        
            $this->error['age'.$data] = "Please enter a valid date";
        }
        else if(!$this->ageCheck($month,$day, $year,$limit)) {
            $this->error['age'.$data] = "You must have a minimum of ".$limit." of age to sign up";
        }
        else return true;
    }

    public function sex($data) {
        if($data=='M' || $data=='F') {
            return true;
        }
        else {
            $this->error['sex'.$data]='Add a valid sex';
        }
    }
    public function errorExists() {
        if(isset($this->error)) {
            return true;
        }
    }        
}

Answer 1

I have used too many if-else if statements

I don't think so. Your code isn't deeply nested, and the if-else that exist mostly exist for a good reason. In some cases you could return directly instead of checking and returning true, eg this:

if($data==""|| empty($data) || !isset($data) || $data==-1) {
    return true;
}

could be this:

return $data==""|| empty($data) || !isset($data) || $data==-1;

which also has the benefit of explicitely returning false.

Validation Functions

Functions like sanitize and basicSanitize do not make sense. For someone using them it is completely unclear what they sanitize and when they should be used.

In this case, htmlspecialchars should be used on output in most situations. A name like encodeForHTMLContext would be more fitting, but it doesn't really belong in a validator but instead in an encoding class.

Structure

A basic validator class should only contain basic validator methods. If you add specific validator methods, the class will grow very quickly and be difficult to manage.

Those specific validation methods should be in the own validator. For example a PersonValidator may contain checks for age and sex.

A generic validator should have funcions that can be used by multiple specific validators, such as length check, alphanum check, empty check, and so on.

Comments

I would suggest PHPDoc style comments instead of your inline comments.

Database interaction

First, a database handler isn't a query, the variable name should be changed.

Also, you should think about injecting the handler to make your code more flexible. Personally, I would hand it over to the specific methods, there is no need for a field.

But I'm not too happy with a validator performing database interactions in general. Database queries should be located in the specific models or data access objects, so that all the database queries are in the same place (instead of all over the application). You could then pass that object which performs the actual check on to the validator and delegate the check to it, so your validation methods would still all be in the same place.

To give you a general idea, here is some (pseudo)code. Depending on the rest of your application, you may want to change it, but it does help to put all your query code in one place.

class PersonDAO {

    // the database would be either passed via constructor 
    // or directly to the methods below

    public getByID($id) {...}
    // various other person related data methods

    public isUniqueEmail($email) { [perform the query]}
    // alternatively, you may want to use a generic getByEmail function and check the return value
}

class PersonValidator {

    public function isUniqueEmail($personDAO, $email) {
        return $personDAO->isUniqueEmail($email);
    }
}

Answer 2

$validator = new Validator(); looks better then $validator = new Validate();

Then, the functions, let's go step by step.

I would leave in this class only functions that really do validation stuff.basicSanitize and sanitize should not be here. Input passed to this class should be already sanitized.

Then, your basicEmptyCheck is equal to empty(), so you can use empty method directly. The same goes for isEmpty;

Also, this method

private function checkLength($data,$min,$max) {
    if(strlen($data)>=$min && strlen($data)<=$max) {
            return true;
    }
}

looks better if it's done like this:

private function validLength($data, $min, $max) {
    $lenth = strlen($data);
    return $length < $max && $length > $min;
}

In this version, you're doing only 1 call to strlen.

Answer 3

Additionally, to previous reporters, can say that for constructions like this:

public function sex($data) {
    if($data=='M' || $data=='F') {
        return true;
    }
    else {
        $this->error['sex'.$data]='Add a valid sex';
    }
}

you can use early return:

public function sex(string $data): bool
{
    if($data=='M' || $data=='F') {
        return true;
    }
    
    $this->error['sex'.$data]='Add a valid sex';
     
    return false;       
}

And type-hinting is strongly recommended.