Simplify a long IF statement in PHP

Question

Is there any simpler format for the following if statement below?

if((!empty($_SESSION['email'])) && (!empty($_SESSION['serial'])) && (!empty($_SESSION['name'])) && (!empty($_SESSION['number'])) && (!empty($_SESSION['institution'])) && (!empty($_SESSION['address'])) && (!empty($_SESSION['item2']))){ 
    echo "OK u can proceed";
} else {
    echo "U didn't complete the requested info";
}

Sorry if this is too simple for you, but really appreciate any pro suggestion.

This isn't equivalent, @Akintunde. What if there's some more information in `$_SESSION`? It won't be empty, but maybe the required values are. — Jordi Nebot, Nov 18 '17 at 10:39
I tried !empty($_SESSION), it didn't work like what I expect. I think it means : as long as it is not empty, even 1 out of 10 parameters exist. But what I want is 10 out out 10 parameters must be fulfilled. — P. Lau, Nov 18 '17 at 11:12
I should ask if you are aware of `empty()`'s assumptions about zero-ish, empty-ish values. Is there any chance that valid values will be passed via these elements that will cause an unintended failure by `empty()`? Please read the manual if you are unsure what I mean: http://php.net/manual/en/function.empty.php — mickmackusa, Nov 18 '17 at 11:32

mickmackusa · Accepted Answer · 2017-11-18T11:48:16.183

Because conditional statement will "short circuit" on the first failure, re-writing your process using an iterator will be less performant.

If you only want to provide generalized feedback to the user, use your current method without the surplus parentheticals.

if(!empty($_SESSION['email']) &&
   !empty($_SESSION['serial']) &&
   !empty($_SESSION['name']) &&
   !empty($_SESSION['number']) &&
   !empty($_SESSION['institution']) &&
   !empty($_SESSION['address']) &&
   !empty($_SESSION['item2'])){
    // valid
}

Alternatively (same same):

if(empty($_SESSION['email']) ||
   empty($_SESSION['serial']) ||
   empty($_SESSION['name']) ||
   empty($_SESSION['number']) |
   empty($_SESSION['institution']) ||
   empty($_SESSION['address']) ||
   empty($_SESSION['item2'])){
    // invalid
}

Only if you wish to specify the cause of the invalidation should you bother to set up an iterative process. Furthermore, if you are going to perform iterations to validate, you can take the opportunity to remove any unwanted elements from the $_SESSION array that evil-doers may have injected to take advantage of any future loops on the superglobal.

$valid_keys=['email','serial','name','number','institution','address','item2'];
$validated=true; // default value
foreach($valid_keys as $key){
    if(!empty($_SESSION[$key])){
        $session_data[$key]=$_SESSION[$key];
    }else{
        echo "Oops, there was missing data on $key";
        $validated=false;
        break;
    }
}
if($validated){...
// from this point, you can confidently run loops on `$session_data` or slap it directly into a pdo call knowing that it has been validated and ordered.

All that said, if there is even the slightest chance that your $_SESSION elements could hold zero-ish/false-y/empty values, then isset() is the better call (only NULL will get caught). Another good thing about isset() is that it allows multiple variables to be written into the single call and maintains the same/intended performance.

if(isset($_SESSION['email'],$_SESSION['serial'],$_SESSION['name'],$_SESSION['number'],$_SESSION['institution'],$_SESSION['address'],$_SESSION['item2'])){
    // valid
}

or

if(!isset($_SESSION['email'],$_SESSION['serial'],$_SESSION['name'],$_SESSION['number'],$_SESSION['institution'],$_SESSION['address'],$_SESSION['item2'])){
    // invalid
}

Is the 3rd method the one that can tell user which part is not completed? — P. Lau, Nov 18 '17 at 12:07
Yes, that is the option to use if you want to be highly informative about the cause of the invalidation. There are cases when you want to be informative and cases when you want to keep this info secret for security purposes. — mickmackusa, Nov 18 '17 at 12:09

caramba · Answer 2 · 2017-11-18T11:56:08.067

1

You could build a function where you pass all the keys you want to check. I've changed $_SESSION to $SESSION to cheat the array with some values ...

<?php

$SESSION = [
    'email' => 'xx',
    'serial' => 'xx',
    'name' => 'xx',
    'number' => 'xx',
    'institution' => 'xx',
    'address' => 'xx',
    'item2' => 'xx'
];

$keys = [
    'email',
    'serial',
    'name',
    'number',
    'institution',
    'address',
    'item2'
];

if(isNotEmpty($SESSION, $keys)) {
    echo "OK u can proceed";
} else {
    echo "U didn't complete the requested info";
}

function isNotEmpty($array, $keys) {
    foreach($keys as $key) {
        if(empty($array[$key])) {
            return false;
        }
    }

    return true;
}

PHP Fiddle

edited Nov 18 '17 at 11:56

answered Nov 18 '17 at 10:51

caramba

21,963
19
86
127

No early `break;` in the loop? – mickmackusa Nov 18 '17 at 10:52
@mickmackusa what do you think, better? – caramba Nov 18 '17 at 11:09
That's clean. ...wait a second, do you need to pass SUPERGLOBALS to the function or is it already available. – mickmackusa Nov 18 '17 at 11:11
I think if using the real `$_SESSION` we do not need to pass it as it will be available in the function. But to get the PHP Fiddle to work I had to pass it in.... – caramba Nov 18 '17 at 11:12
Yep, that `isEmpty` function is much more clean than mine. Thanks @caramba – Jordi Nebot Nov 18 '17 at 11:13
Thanks @JordiNebot appreciate! – caramba Nov 18 '17 at 11:13
1

Also, passing `$_SESSION` to the function allows the function to be reused to check other arrays. – Jordi Nebot Nov 18 '17 at 11:14
That's a fair point, better to name the parameter `$array` so as not to misinform. – mickmackusa Nov 18 '17 at 11:19
1

@mickmackusa just happend as you commented! – caramba Nov 18 '17 at 11:20

splash58 · Answer 3 · 2017-11-18T10:48:32.820

0

Just "mind games"

$arr = array_flip('email', 'serial', 'name','number'...);
if (count(array_filter(array_intersect_key($_SESSION, $arr))) == count($arr)) {

edited Nov 18 '17 at 10:48

answered Nov 18 '17 at 10:37

splash58

26,043
3
22
34

I do not recommend relying on `array_filter()`'s default/greedy behavior to filter the `$_SESSION` array. – mickmackusa Nov 18 '17 at 10:51
What the problem Do you expect? – splash58 Nov 18 '17 at 10:54
Monkey-wrenching value: `$_SESSION['number']='0';` – mickmackusa Nov 18 '17 at 10:55
perhaps you could reduce function calls like this: `if(array_intersect_key($arr,$_SESSION)==$arr){` ? – mickmackusa Nov 18 '17 at 10:57
um, wait, my bad. That is the default behavior of `empty()` too. There is no unintended loss. – mickmackusa Nov 18 '17 at 10:59
@mickmackusa about comparing arrays - it's not the same due to different values in arr and session – splash58 Nov 18 '17 at 12:01
You are right again, kind of. I am comparing `$arr` to `$arr` (changed param order in `array_intersect_key()`). http://sandbox.onlinephpfunctions.com/code/d73f5a0be17fcfb5bad5ae1b2862536084de82fd My suggestion does not maintain the same functionality. – mickmackusa Nov 18 '17 at 12:14

Jordi Nebot · Answer 4 · 2017-11-18T11:17:18.853

This is just my opinion, but I think your expression is fine. Maybe you could format it in another way to make it more readable:

if ( !empty($_SESSION['email']) && 
     !empty($_SESSION['serial']) && 
     !empty($_SESSION['name']) && 
     !empty($_SESSION['number']) && 
     !empty($_SESSION['institution']) && 
     !empty($_SESSION['address']) &&
     !empty($_SESSION['item2']) ) {

     /* Proceed */

} else {

     /* Don't proceed */

}

Or maybe use a custom validation function:

/* My original version */
function filled_required_fields($fields, $array) {
    $valid = true;
    foreach ($fields as $field) {
        $valid = $valid && !empty($array[$field]);
        if (!$valid) return false;
    }
    return $valid;
}

/* Cleaner solution based on @caramba's answer */
function filled_required_fields($fields, $array) {
    foreach ($fields as $field) {
        if (empty($array[$field])) return false;
    }
    return true;
}


if (filled_required_fields(['email', 'serial', 'name', 'number', 'institution', 'address', 'item2'], $_SESSION) {

    /* Proceed */

} else {

    /* Don't proceed */
}

...good a break point in the loop, but it could be written cleaner. — mickmackusa, Nov 18 '17 at 10:54
Thank you for your comments @mickmackusa! I'm sure it could be cleaner. I encourage you to write an answer so that we all could learn something! :) — Jordi Nebot, Nov 18 '17 at 10:59
I don't know, not mine. Probably shouldn't copy someone else's method though. — mickmackusa, Nov 18 '17 at 11:46

Simplify a long IF statement in PHP

4 Answers4