2
\$\begingroup\$

Can someone help me furnish my PHP validation class? I want to reduce its size and make it easier to use.

<?php
class validate
{
 function check_empty($name,$value)
    {
      if(!empty($value))
        { 
          $msg="";
          return array($name=>$value,$error=>0,$err_message=>$msg);
        }
     else
        {
          $value="";
          $msg=$name.'field is empty';
          return array($name=>$value,$error=>1,$err_message=>$msg);
        }
    }
 function validate_email($value)
    {
      $value=test_input($value);
      if (!filter_var($email, FILTER_VALIDATE_EMAIL))
        { 
          $msg='Invalid Email';
          return array($val=>$value,$error=>1,$err_message=>$msg);
        }
      else
       {
          $msg='';
          return array($val=>$value,$error=>0,$err_message=>$msg);
       }              
    }
 function validate_strings($value,$maxsize,$minsize)
    {
     if(strlen($value)>$maxsize)
        {
         $msg='input data is very long';
         return array($val=>$value,$error=>1,$err_message=>$msg);
        }
     else if(strlen($value)<$minsize)
        {
         $msg='input data is too short';
         return array($val=>$value,$error=>1,$err_message=>$msg);
        }
     else
        {
          if(preg_match('/[^a-z\s-\']/i',$value))
            {
             $msg='';
             return array($val=>$value,$error=>0,$err_message=>$msg); 
            }
          else
            {
             $msg='The string should only contain letters';
             return array($val=>$value,$error=>1,$err_message=>$msg);
            }
        }
    }
 function validate_numbers($value)
    {
       if(preg_match('/[0-9]+/'))
        {
          $msg='';
          return array($val=>$value,$error=>0,$err_message=>$msg);  
        }
     else
        {
         $msg='Invalid input,only numbers required';
         return array($val=>$value,$error=>1,$err_message=>$msg);   
        }
    }
} 
?>
\$\endgroup\$
1
  • \$\begingroup\$ What is the test_input function? \$\endgroup\$
    – tim
    Mar 18 '15 at 21:43
2
\$\begingroup\$
  • your indent style doesn't seem follow any general PHP convention, and the amount of spaces used isn't even internally consistent, which makes your code a bit hard to read.
  • using more spaces would also increase readability (eg after ,, around =, =>, >, ., etc).
  • you can get rid of variables that are only used once, such as msg or value.
  • some of your variables are undefined, such as error, err_message, val, etc.
  • very long and too short will not be very informative for the user. I would add the max length (eg Input was longer than X).
  • your validate_strings regex allows , ', and -, which seems like unexpected behavior (especially because ' is often filtered out for security reasons). I would add good documentation and possibly rename the function to validate_strings_unsafe. It might also be a good idea to just pass a regex as argument (this avoids the security issue, and makes your function more flexible).
  • I would probably also document that the user input is added to the error message, so that when you echo it, you know that you have to sanitize it (or preferably just don't add it to the error message, as it's contained in the returned array anyways).
  • your return value is always structured the same (an array with three elements).

If you extract the building of the return array to its own function, it will be easier to change, and result in easier to read code.

private function build_error($key, $value, $message) {
    return array($key => $value, $error => 1, $err_message => $message);
}

private function build_success($key, $value) {
    return array($key => $value, $error => 0, $err_message => '');
}

And then use it like this:

function check_empty($name, $value) {
    if(empty($value)) { 
        return build_error($name, $value, $name . ' field is empty');
    } else {
        return build_success($name, $value);
    }
}

In your validate_strings function you can also return early to avoid one level of nesting:

function validate_strings($value,$maxsize,$minsize) {
    if(strlen($value) > $maxsize) {
        return build_error($val, $value, 'input data is very long');
    }
    if(strlen($value) < $minsize) {
        return build_error($val, $value, 'input data is too short');
    }

    if(!preg_match('/[^a-z\s-\']/i',$value)) {
        return build_error($val, $value, 'The string should only contain letters (and some other stuff)');
    }

    return build_success($val, $value);
}
\$\endgroup\$
0
\$\begingroup\$

The regex is to allow names like ng'ang'a and the white-space and comma for multiple elements in a text field. The data then would be passed in the following function.

function sanitize($name,$value)
        {
          $value=htmlspecialchars(strip_tags($value));
          $magic_quotes_active = get_magic_quotes_gpc();
          $new_enough_php = function_exists( "mysql_real_escape_string" ); 
          if( $new_enough_php )
            {
              if( $magic_quotes_active )
                {  
                  $value = stripslashes( $value ); 
                }
             $value = mysql_real_escape_string( $value );
            } 
         else 
            { 
             if( !$magic_quotes_active ) 
               {
                $value = addslashes( $value ); 
               }

            }
        $I=array($name=>$value);

        return $I;
    }
\$\endgroup\$

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.