1
\$\begingroup\$

I'm relatively new to PHP OOP, currently I'm practising how to use it, however I am not really certain I'm building my classes correctly, I think perhaps I'm missing something out but I'm not entirely sure what, any advice would be appreciated

Class

class Validation 
{
    private 
    $password,
    $repeatPassword,
    $username,
    $email;

    private static
    $minPassword = 7,
    $confirmPassword,
    $minUsername = 3,
    $maxUsername = 14,
    $validEmail;

    public static function validateEmail($email)
    {   
        return (filter_var($email, FILTER_VALIDATE_EMAIL) != self::$validEmail);
    }   
    public static function validatePassword($password)
    {
        return(strlen($password) >= self::$minPassword);
    }

    public static function validatecheckbox($agree)
    {
        return(!empty($terms) >= self::$checked);
    }
    public static function validateRepeatPassword($repeatPassword,$password) 
    { 
            return $repeatPassword === $password; 

    }  
    public static function validateUsername($username)
    {
            return strlen($username) >= self::$minUsername 
            && (strlen($username)) <= self::$maxUsername 
            && (filter_var($username ,  FILTER_VALIDATE_REGEXP,["options"=> [ "regexp" => "/^[\p{L}0-9\s]+$/u"]]) == TRUE);
    }
}

Usage

    $errors = array();

 $fields = array( 
  'username' => array( 
    'validator' => 'validateUsername', 
    'message'   => 'What is your username?' 
  ), 
  'email' => array( 
    'validator' => 'validateEmail', 
    'message'   => 'Please enter a valid email', 
  ), 
  'password' => array( 
    'validator' => 'validatePassword', 
    'message'   => 'Password must be a minimum of seven characters' 
  )
);  

if(!Validation::validateRepeatPassword($password, $repassword)) 
{ 
    $errors[] = ["name" => "repassword", "error" => "Passwords must match"]; 
}
foreach($post as $key => $value) 
    {
  if(isset($fields[$key])) 
    {
            if(!Validation::{$fields[$key]['validator']}($value)) 
            {
                $errors[] = ['name' => $key, 'error' => $fields[$key]['message']];
            }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Yes towards the bottom foreach($post as $key => $value) { if(isset($fields[$key])) \$\endgroup\$ – confusedGibbon Sep 10 '18 at 16:35
2
\$\begingroup\$

Well, your class is good and does what it is supposed to do, except in validalidatecheckbox() (you are using variables that doesn't exist).

Also I would do this changes:

  • Remove this code (it's unused):

    private $password, $repeatPassword, $username, $email;

  • Change this to constants, so you can use it elsewhere:

    private static $minPassword = 7, $confirmPassword, $minUsername = 3, $maxUsername = 14, $validEmail;

So your class should look like this:

class Validation
{
    const MIN_PASSWORD_LENGTH = 7;
    const MIN_USERNAME_LENGTH = 3;
    const MAX_USERNAME_LENGTH = 14;

    // rest of your code

Also, I strongly recommend you to check PSR-1 and PSR-2 to make your code look standard-like.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you I honestly wasn't sure if const would be be the correct thing to use, I will read over both links too \$\endgroup\$ – confusedGibbon Sep 15 '18 at 1:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.