consecutive if's in PHP

Go To StackoverFlow.com

0

I'm doing some validation now in PHP, and I'm running allot of conditional statements, for example:


if ($this->email->isValid($email))
return false;
if ($this->username->isValid($username))
return false;

ect..

Is there a nice way to do this? Or do I just run ten If statements like the ones above? I can't use switch obviously, but I'm looking for that type of solution..

P.S... I'm using Zend Framework for validation

2009-06-16 09:14
by NoName
Shouldn't this be !$this->email->isValid (i.e. is /not/ valid) - Matthew Flaschen 2009-06-16 09:20
Just a point for other people that may come to this question/answers later - sometimes several 'if() {return;}' statements are just what you need to make it clear you want to exit a function quickly, rather than storing a state before you check and return after that - Alister Bulman 2009-06-16 09:24


4

You could OR them like this:

if(cond1||
   cond2||
   cond3||
   cond4)
{
  return false;
}
2009-06-16 09:17
by Blindy
i have to know where something went wrong.. so that won't wor - NoName 2009-06-16 09:19
Daniel, with your current code, you won't know what's going wrong either. Can you post a better example - Matthew Flaschen 2009-06-16 09:23


3

Doing something like this is called a Guardian clause. It can be improved so clauses that return the same value should be grouped together.

if ($this->email->isValid($email) || $this->username->isValid($username))
{
    return false;
}

Or like this, if there are many (btw how I format the if is just so it can read better, any other way should be fine as well)

if (
    $this->email->isValid($email) || 
    $this->username->isValid($username) || 
    $this->somethingelse()
   )
{
    return false;
}
2009-06-16 10:58
by Ólafur Waage
+1 for mention of "Guardian Clause - Zachary Young 2011-11-17 17:09


0

If this is data from a form, take a look at Zend_Form, as recommended by a poster in your other question

I posted some example code which details adding validators to form elements

2009-06-16 09:17
by David Snabel-Caunt
two different daniels. - NoName 2009-06-16 09:18
Just a coincidence that you write your questions in the same style, and use similar code then - David Snabel-Caunt 2009-06-16 09:40


0

Just make sure that you put the most obvious validation at the top, so if its going to fail it will trip before it runs through every statement. Also, I don't like if statements without curly brackets, but then thats just a matter of opinion.

2009-06-16 09:17
by Christian


0

if ($this->email->isValid($email) || 
    $this->username->isValid($username))
        return false;
2009-06-16 09:18
by Matthew Flaschen


0

It is always good to use some var than multiple returns

In case these are the only return conditions. We dont need to check for all conditions. like check only for true conditions return false by default or vice-versa.

$result = false; //return false by default.

if (cond1 || cond2) { //check only for conditions which result in returning true. :)

  $result = true;

}

return $result;

Cheers,

-Ratnesh

2009-06-16 09:26
by Ratnesh Maurya
be careful of that word: "always - nickf 2009-06-16 12:32
yeah.. u r right. thanks! - Ratnesh Maurya 2009-06-18 12:03


0

Maybe something like this:

foreach( $form->getElements as $element => $value )
{ 
    if( !$element->isValid( sanitize($value))){
       return false;
    }
}

BUT if you are using ZF this is your oneliner answer because you check all the form in one not individual fields:

$form = new My_Zend_Form(); // in the controller action that you send the form to

if ( $form->isValid($_POST)) {
    // Success /// do whatever you want with the data
    $data = $form->getValues();
} else {
    //error
}
2009-06-16 10:18
by Elzo Valugi


0

I would make them data members of my class. Obviously here you will have to have a form driven class. So here for example the email could be wrapped into a class and initialised with in the constructor of the class that has them as member variables. Now the email wrapper class will do validation for email on initialisation/construction.

IMHO that would look less cluttered and you can wrap up any validation or specific methods to the email-wrapper class.

I know it might be a sledge hammer in some context; so choose wisely!

2009-06-16 12:30
by ϹοδεMεδιϲ


0

Do something like this:

$errlvl = 0;

if($errlvl == 0 && $this->email->isValid($email)){
    $errlvl++;
}

if($errlvl == 0 && $this->username->isValid($username)){
    $errlvl++;
}

// your validation list keeps going

if($errlvl > 0){
    return false;
}

this will

  1. Reduce redundancy because if there's an error in front, the following will not be checked.
  2. you can keep adding on to the list
  3. if you want to know how many errors occurred, you can remove $errlvl == 0 in the statements
2009-08-05 11:41
by mauris


0

Add all objects to an array and the iterate it (I guess you or Zend are using an interface "validator" or something like that each component (username, email, etc) implements the "isValid" method):

$validators = array();
$validators[] = $this->email;
$validators[] = $this->username;

foreach ($validators as $validator)
{
    if (!$validator->isValid())
    {
        return false;
    }
}
2009-08-05 14:58
by inakiabt