Validating email addresses and phone numbers

I have the following code that I need help with refactoring:

• Validation class for objects you may create:

• Email address

• URL

• Norwegian Mobile Number (must start with 4, 9 or 59 and be 8 digits long)

```
$data_obj) {

switch ($data_obj['rule']) {
case 'EMAIL':
if (!self::is_valid_email($data_obj['value'], $obj_name)) {
$validation_result = FALSE;
}
break;
case 'URL':
if (!self::is_valid_url($data_obj['value'], $obj_name)) {
$validation_result = FALSE;
}
break;
case 'NorwayMobile':
if (!self::is_norwegian_number($data_obj['value'], $obj_name)) {
$validation_result = FALSE;
}
break;
default :
self::$validation_errors[] = "INVALID_RULE_ERROR_MESSAGE";
$validation_result = FALSE;
break;
}
}
return $validation_result;
}

/**
* function for return validation Error
* @return array Error Array
* @access public
*/
public static function get_validation_error_message() {
return self::$validation_errors;
}

/**
* function for Validate Email Value;
* @param String $email_str Email Text value
* @return Boolean
* @access Public
*/
public function is_valid_email($email_str, $obj_name) {
if (!filter_var($email_str, FILTER_VALIDATE_EMAIL)) {
self::$validation_errors[$obj_name] = "INVALIED_EMAIL_ERROR_MESSAGE";
return FALSE;
}
return TRUE;
}

/**
* Function for Validate URL Value
* @param String $url_str URL Text Value
* @return boolean
* @access public
*/
public function is_valid_u

There is too many comments. Write comments when code can't explain this part. But instead you commented every line. These comments create a mess, and do not help.

Of course I'm not about phpdoc.

Your phpdoc is invalid for this functions: is_valid_email, is_valid_url, is_norwegian_number. You pass 2 arguments, but you phpdoc contain only 1.

Write primitives, like a string, array, boolean and others in lowercase. Because when I see String I can think about some wrapper for string primitive. But indeed it isn't.

@param type $input_value type? Your code doesn't contain a class with name type. Rename it to array. Same in @return Output Testing Result; This function doesn't return anything, so write @return void.

BUT will be better if this function return true/false depends of passed/failed tests. BUT more better -- to place test correctly in separate file, and run via phpunit.

foreach ($input_value as $obj_name => $data_obj) {
        switch ($data_obj['rule']) {

You named your variable $data_obj, but indeed it's array. You should rename it. When I see $obj, I'm thinking about stdClass, but not about an array.

You should use non-static methods there. Really I don't see a reason why you've used static methods. I think better way will be -- set all is_... methods private, pass your rules/data in constructor and then use do_validation method.

You code will look like this:

$validator = new ValidationSystem($data);
if ($validator->do_validation) {
} else {
    $validator->getErrors();
}

article about static method


any recommend tools or design pattern for re-factoring this legacy code.

Martin Fowler. Refactoring: Improving the Design of Existing Code.

Also your code contain a lot of typos.

StackExchange Code Review Q#68512, answer score: 2