1

I use the clas below to validate user input. Originally it was just a collection of static functions grouped together.

However, I modifed it to an object style and added in a private memeber to hold the user input array. What is the next step to making this class adaptable, i.e. more generic so that it can be used by others as part of a library?

$message is the text displayed to the user on a validation fail.

Library Code:

class validate
  {
  private $input;
  function __construct($input_arg)
    {
    $this->input=$input_arg;
    } 
  function empty_user($message)    
    {
    if((int)!in_array('',$this->input,TRUE)) return 1;
    echo $message;return 0; 
    }
  function name($message)          
    {
    if(preg_match('/^[a-zA-Z-\.]{1,40}$/',$this->input['name'])) return 1;
    echo $message;return 0; 
    }
  function email($message)
    {
    if(preg_match('/^[a-zA-Z0-9._s-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{1,4}$/',$this->input['email'])) return 1;
    echo $message;return 0; 
    }
  function pass($message)
    {
    if(preg_match('/^[a-zA-Z0-9!@#$%^&*]{6,20}$/',$this->input['pass'])) return 1;
    echo $message;return 0;
    }
  }

Application Code:

  function __construct()
    {
    parent::__construct();
    $obj=check_new($this->_protected_arr);
    $a='<si_f>Please enter both an email and a password!';
    $b='<si_f>Please enter a valid email!';
    $c='<si_f>Please enter a valid password!';
    if($obj->empty_user($a) && $obj->email($b) && $obj->pass($c) && self::validate())
      {
      self::activate_session();
      echo "<si_p>";
      }
    }
5
  • First, I wouldn't echo anything out when validating; store error messages in a private $errors = array(); variable within the class and check if (count($this->errors) > 0) within a public function is_error() method after validating. Or something along those lines. There's really several different ways, but I wouldn't do it this way in a class. Commented Sep 30, 2011 at 20:23
  • That way you can return all of the errors to the user, so they don't go one by one (possibly) getting each wrong. That would be very frustrating. If they've submitted the entire form that could be validated, validate it and tell the user each error they need to fix. Commented Sep 30, 2011 at 20:28
  • I'm correlating of facebook.com...this is how they do it, b.c. the form is small and not large perhaps this is o.k. Commented Sep 30, 2011 at 20:29
  • 2
    If I submit a form with six fields I've filled (or not filled) in, and I got four wrong according to your class, I don't want to have to click submit on the form four times just to learn what the next error is. Commented Sep 30, 2011 at 20:30
  • Are you sure Facebook is not using client-side validation? Meaning, JS, that doesn't require a roundtrip to the server? Commented Sep 30, 2011 at 20:31

2 Answers 2

1

I'd not write all these function in a generic class. I'd rather have separate functions that perform specific checks, and maybe a specific class that calls these checks on my specific input.

This class now echo's, which is never a good solution for a class like this. Just let it perform the check and raise an exception if something's wrong. If exceptions are too hard, or don't fit in your achitecture, let your function return false, and set a property that can be read afterwards.

About your specific checks:

  • Your e-mail check is very strict and doesn't allow all e-mail addresses. The domain, for instance, can be an IP address too, and the username (the part before the @) can include many obscure characters, including an @. Yes, really!

  • Why must a password be 6 characters at least? And why on earth would you limit the password to 20 characters? I use passwords of over 20 characters, and I know many other people that do too. Just let everybody type everything they want. Anything. Let them post 3MB of text if they like. Let them include unicode characters if they want. What is a better protection that having a bunch of chinese characters as a password? And if they want to enter just a, that's their responsibility too. You should never ever store the password itself anyway, so just hash whatever they input and store the 32 characters that gives you (if you use MD5 hashing). The only password that you may refuse is an empty password. Anything else should go.

  • Same goes for name. 40 characters? Really. I can imagine people having names that long. Add a little more space. Bytes aren't that expensive, and it's not that you're gonna have 2 billion users.

Sign up to request clarification or add additional context in comments.

8 Comments

I agree with everything but the one character minimum on passwords.
At least 60 characters. The longest name on a birth certificate in England apparently is 'Rhoshandiatellyneshiaunneveshenk Koyaanisquatsiuth Williams', 57 characters. But why not make it a 100? The longest valid e-mail address I believe is 321 characters. 256 for the user, 64 for the domain, and 1 for the @, but you should check if that's indeed still valid.
When used in an Ajax call, still let this class just output a status to the program, so raise an exception or return a boolean or a code. If you echo, it is hard to do all checks upfront or combine various checks, because each checks already put their status in the output of the page. What if you don't want to check the e-mail, but want to check if it doesn't exist in your database too? By not echoing, you can combine checks and execute them in the order you like. Then, using their result, compose and output that can be used by your page. That will help you keep your code structured.
And please don't remove question-comments when I answer them. That will make my replies look silly.
Don't worry, @GolezTrol, I saw them. :D
|
1

Maybe it's worth having a look at Zend Validate? Or any other PHP frameworks validate classes. Just then extend them to add the functionality you want.

In answer to your question, is it worth having another variable in the class so you can check the error?

  class validate
  {
  private $input;
  public $error = false;
  function __construct($input_arg)
    {
    $this->input=$input_arg;
    } 
  function empty_user($message)    
    {
    if((int)!in_array('',$this->input,TRUE)) return 1;
    echo $message;$this->error = "Empty message";
    }
 ... else
  }
$validate = new validate($empty_message);
if( !$validate->empty_user('this input is empty') === false)
{
    echo "Was not empty";
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.