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);
}
}
}
?>
2 Answers 2
- 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
orvalue
. - some of your variables are undefined, such as
error
,err_message
,val
, etc. very long
andtoo short
will not be very informative for the user. I would add the max length (egInput 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 tovalidate_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);
}
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;
}
Explore related questions
See similar questions with these tags.
test_input
function? \$\endgroup\$