CODEWITHSUNDEEP
phpfunction
Majid Fouladpour
Majid Fouladpour

Reputation: 30272

PHP - Validation function to return true|false, AND a message if false

I have a validation function which returns either true or false.
However, I want it to provide info as to what the problem is, when there is one.

Let's say the function is like this:

function is_valid($val) {
  $result = true;
  if( rule_1_not_met ) $result = false;
  if( rule_2_not_met ) $result = false;
  return $result;
}

Which is used like this

$val = $_GET['some_param'];
if(!is_valid($val)) $out .= 'Not so helpful feedback.';
...

I thought I could change it like this:

function is_valid($val) {
  $result = array(true, array());
  if( rule_1_not_met ) $result[1][] = 'Reason 1';
  if( rule_2_not_met ) $result[1][] = 'Reason 2';
  if(count($result[1]) > 0) $result[0] = false;
  return $result;
}

And use it like this:

$val = $_GET['some_param'];
$validation_result = is_valid($val);
if(!$validation_result[0]) $out .= implode('<br/>', $validation_result[1]);
...

My question is

P.S. Would make this community wiki

Upvotes: 8

Views: 42160

Answers (5)

Your Common Sense
Your Common Sense

Reputation: 158007

This question is old, and makes a showcase of bad and outdated practices. Using global is frowned upon, and using references in this context is just the same.

Only Cave Johnson's answer makes it straight, but still the usage could be confusing. A better solution would be to write a class, but not as silly one as in the Brad Thomas's answer.

class NumberValidator
{
    protected $errors;
    public function validate($number)
    {
        if(!is_numeric($number))
        {
            $this->errors[] = "The value provided is not numeric";
            return false;
        }
        if($number < 10)
        {
            $this->errors[] = "The number is less than 10";
            return false;
        }
        return true;
    }
    public function getErrors()
    {
        return $this->errors;
    }
}

and then it can be used like this

$validator = new NumberValidator();
if($validator->validate($number)) {
    /*success*/ 
}

and then $validator->getErrors() can be used elsewhere

Upvotes: 0

Cave Johnson
Cave Johnson

Reputation: 6788

In my opinion, your proposed solution works fine. The only problem with it is that you have to remember that $validation_result[0] is the status and $validation_result[1] contains the messages. This might be OK with you but it will be hard to maintain if other people use your code. One thing you can do is when you call your function, you can use array destructuring to at least store the results with meaningful variable names. For example:

[$valid, $errors] = is_valid($val);
if(!$valid) $out .= implode('<br/>', $errors);

For the reason mentioned above, I like Brad Thomas's solution of creating a specialized class that contains the messages and status. Since the properties are named, you don't have to guess how to access the validation information. Also, most good IDEs will autocomplete when you try to access their properties.

I also have an alternate solution. Instead of including a boolean true or false. Just return the array of messages. The caller would just have to check if the returned array has a non-zero number of errors. Here is an example:

function get_errors($val) {
    $errors = array();
    if( rule_1_not_met ) $errors[] = 'Reason 1';
    if( rule_2_not_met ) $errors[] = 'Reason 2';
    return $errors;
}

Then the caller would use it like this:

$val = $_GET['some_param'];
$validation_result = get_errors($val);
if (count($validation_result) > 0) $out .= implode('<br/>', $validation_result);

Upvotes: 2

Brad Thomas
Brad Thomas

Reputation: 37

You could use a Result object that encapsulates return data, a message and a status.

i.e. 

class Result( $bResult, $sMessage, $mData ) {
    public function __construct() {
        $this->bResult = $bResult;
        $this->sMessage = $sMessage;
        $this->mData = $mData;
    }
}

In Your code:

$result = new Result(true, 'some helpful message here', null);

Upvotes: 1

Mark Tomlin
Mark Tomlin

Reputation: 8943

$reasons = array();
function is_valid($val)
{
    global $reasons;
    if ( rule_1_not_met ) $reasons[] = 'Reason 1';
    if ( rule_2_not_met ) $reasons[] = 'Reason 2';
    if ( count($reasons) == 0 )
        return TRUE;
    else
        return FALSE;
}

if (!is_valid($condition))
{
    echo 'Was not valid for these reasons<br />';
    foreach($reasons as $reason)
        echo $reason, '<br>';
}
else
    echo 'Is valid!';

Upvotes: 0

Shakti Singh
Shakti Singh

Reputation: 86476

You are in the right track but I would like to do this in this way

function is_valid($val,&$mes) {
  $result = true;
  if( rule_1_not_met ) { $mes[]='message one'; $result = false; }
  if( rule_2_not_met ) { $mes[]='Message two'; $result = false; }
  return $result;
}

$mes=array();
if(isvalid($val,$mes) ===false)  $out .= implode('<br/>', $mes);

Upvotes: 11

Related Questions