Makes sense as a validation class?

<?php 
class Validation{
	private $_fails = [];
	
public function validator($input, $validation = []){
	
	foreach($input as $inputs => $input_value){
		if(key_exists($inputs, $validation)){
			foreach($validation[$inputs] as $tests => $test_value){
				
				if(is_int($tests)){
					$tests = $test_value;
					
					switch($tests){
						case 'required':
						if(empty($input_value) && $test_value)
						{
							$this->addError($inputs,ucwords($inputs). ' required');
						}
						break;
						
							case 'minLength':
						if(empty($input_value) > $test_value)
						{
							$this->addError($inputs, ucwords($inputs). ' should be minimum '.$rule_value. ' characters');
						}
						break;
						
						case 'maxLength':
						if(empty($input_value) > $test_value)
						{
							$this->addError($inputs, ucwords($inputs). ' Cannot be more than '.$rule_value. ' characters');
						}
					}
						
				}
			}
			
		}
	}
	
}

 private function addError($inputs, $errors){
    $this->_fails[$inputs][] = $errors;
}


public function error(){
    if(empty($this->_fails)) return false;
    return $this->_fails;
}
}

A switch() inside an if() inside a foreach() inside an if() inside a foreach() is a definite "code smell". Therefore, if I understood exactly what you were trying to do with all those conditions etc., I'd want to refactor it, maybe adding a few methods to compartmentalize things?

I suspect you want strlen() instead of empty() for the minLength and maxLength cases (plus probably a > for that latter)?

Why do you do $tests = $test_value;? You do not appear to use $tests after that.

cluelessPHP foreach($input as $inputs => $input_value){
if(key_exists($inputs, $validation)){

If the above is blindly looping over all the input data, then testing if there's a validation rule, you need to reverse the logic, and loop over an expected list of inputs and test/validate the corresponding input. Hackers/bots can submit 1000's of inputs. You don't want to loop over all of them as this uses resources and will trigger errors, which is what hackers want.

cluelessPHP if(is_int($tests)){

is_int() doesn't do what you think. It tests if the variable is of type integer. By definition, all external data, $_GET, $_POST, $_COOKIE are string variables, regardless of what is in them, so, using is_int() won't ever be true for external data.

I don't see anywhere that you are trimming the values before validating them. This is the one 'alteration' to input data that you should do.

      pbismad

      Ah thanks, wasn't sure about that

      I don't see anywhere that you are trimming the values before validating them. This is the one 'alteration' to input data that you should do.

      I thought I could do something like

      $post = filter_input_array(INPUT_POST, FILTER_SANITIZE_STRING);

          NogDog

          A switch() inside an if() inside a foreach() inside an if() inside a foreach() is a definite "code smell". Therefore, if I understood exactly what you were trying to do with all those conditions etc., I'd want to refactor it, maybe adding a few methods to compartmentalize things?

          Think I got myself tied in knows, I still have no idea what's expected of a junior developer tbh. I've asked this question in various places over time and never actually gotten an answer, kind of frustrating

              Not saying this is exactly how I'd do it, but trying to keep your general idea and just breaking things up into more atomic pieces, maybe:

              <?php

class FormValidation {
    protected $errors = [];
    public function validateFields(Array $fields, Array $rules) {
        $this->errors = [];
        foreach($fields as $field => $value) {
            if(isset($rules[$field])) {
                foreach($rules[$field] as $rule => $param) {
                    $this->validate($field, $value, $rule, $param);
                }
            }
        }
        return empty($this->errors);
    }
    public function errors() {
        return $this->errors;
    }
    protected function validate($name, $input, $validationType, $validationValue=null) {
        if(!$this->$validationType($input, $validationValue)) {
            $this->errors[$name][$validationType] = $validationValue;
        }
    }
    protected function required($input, $value=null) {
        return !empty(trim($input));
    }
    protected function minLength($input, $length) {
        return $this->length($input) >= $length;
    }
    protected function maxLength($input, $length) {
        return $this->length($input) <= $length;
    }
    protected function length($input) {
        return strlen(trim($input));
    }
}

// TEST
$validationRules = [
    'first_name' => [
        'required' => true,
        'minLength' => 2,
        'maxLength' => 40
    ],
    'last_name' => [
        'required' => true,
        'minLength' => 2,
        'maxLength' => 40
    ],
    'middle_name' => [
        'required' => false,
        'maxLength' => 40
    ]
];

$validator = new FormValidation();

$testData = [
    'first_name' => 'Nog',
    'last_name' => 'Dog',
    'middle_name' => 'X',
];

echo "Should pass:\n";
$test = $validator->validateFields($testData, $validationRules);
var_dump($test);
var_export($validator->errors());

$testData = [
    'first_name' => '   ',
    'last_name' => 'Dog',
    'middle_name' => 'Xdkdkdkdkdkdkdkdkdkdkdkdddddddasdfasdfasdfasdfasdfasdfasdfadfsasdfasdfasdf',
];

echo "Should fail:\n";
$test = $validator->validateFields($testData, $validationRules);
var_dump($test);
var_export($validator->errors());

              Output:

              $ php validator.php
Should pass:
bool(true)
array (
)
Should fail:
bool(false)
array (
  'first_name' =>
  array (
    'required' => true,
    'minLength' => 2,
  ),
  'middle_name' =>
  array (
    'maxLength' => 40,
  ),
)
              Write a Reply...