How eliminate duplicate cases from a switch statement in PHP

Question

I'm making a function to return whether or not the given user_id is a staff member of the site. This is what I have and it works, however I feel like it can be greatly improved.

public function isUserStaff($uid) {
    $stmt = $this->conn->prepare("SELECT user_role FROM users WHERE user_id=:user_id");
    $stmt->execute(array(':user_id'=>$uid));
    $userRow = $stmt->fetch(PDO::FETCH_ASSOC);

    $role = $userRow['user_role'];

    switch($role) {
        case 3:
            return true;
            break;
        case 4:
            return true;
            break;
        case 5:
            return true;
            break;
        case 6:
            return true;
            break;
        case 7:
            return true;
            break;
        default:
            return false;
            break;
    }
}

I hope someone can help me out and describe how I can make my code better. I think there are too many case's and I'm looking for something smaller to use.

Done @AlexanderO'Mara – Justin Verhoeven Dec 13 '17 at 02:31 — Justin Verhoeven, Dec 13 '17 at 02:31

score 1 · Accepted Answer · answered Dec 13 '17 at 02:22

If you have switch cases that are the same, you can combine them by omitting the return and break lines of the earlier cases.

In the following, 3, 4, 5 and 6 all take the return value of case 7 (true):

switch($role) {
    case 3:
    case 4:
    case 5:
    case 6:
    case 7:
        return true;
        break;
    default:
        return false;
        break;
}

Although having said that, considering everything seems to return the same way apart from your default, you might be better off making use of a simple if condition. You can even specify that the roles should be between 3 and 7:

if ($role >= 3 && $role <= 7) {
    return true;
}
else {
    return false;
}

Hope this helps! :)

I didn't know you could do that, it certainly helps! Thanks :) — Justin Verhoeven, Dec 13 '17 at 02:26

mickmackusa · Answer 2 · 2017-12-13T03:46:23.583

A lookup array can be used to eliminate condition statements. Because in_array() is much slower than isset() it is advisable to set up your lookup are to store values as keys.

Code:

public function isUserStaff($uid) {
    $stmt = $this->conn->prepare("SELECT user_role FROM users WHERE user_id=:user_id");
    $stmt->execute(array(':user_id'=>$uid));
    $userRow = $stmt->fetch(PDO::FETCH_ASSOC);

    $lookup=[3=>'',4=>'',5=>'',6=>'',7=>'']; // or array_flip(range(3,7))
    return isset($lookup[$userRow['user_role']]);
}

This solution becomes even more useful when your identifying values are non-sequential -- you just list them in an array: $lookup=[3=>'',6=>'',9=>'']; This approach is very easy to maintain as your requirements grow/change.

Lastly, you don't need to write break after a return because the return halts all actions inside the function.

JBES · Answer 3 · 2017-12-13T02:32:07.317

0

Ternary operator - all in one line:

public function isUserStaff($uid){
    $stmt=$this->conn->prepare("SELECT user_role FROM users WHERE user_id=:user_id");
    $stmt->execute(array(':user_id'=>$uid));
    $userRow=$stmt->fetch(PDO::FETCH_ASSOC);

    return $userRow['user_role']<3 && $userRow['user_role']>7 ? false : true;
}

edited Dec 13 '17 at 02:32

answered Dec 13 '17 at 02:26

JBES

1,512
11
18

Looks clean, but throws `PHP Parse error: syntax error, unexpected 'return'` – Justin Verhoeven Dec 13 '17 at 02:29
This is not something you should do to production code. This is difficult to read quickly. – castis Dec 13 '17 at 02:31
And if you're going to return a boolean, just flip the logic and return the result of the expression. `return $userRow['user_role'] >= 3 && $userRow['user_role'] <= 7;` – castis Dec 13 '17 at 02:32
Already edited. Ternary operators are perfectly valid, that's why they exist. Legibility is the individual's issue. – JBES Dec 13 '17 at 02:34

score 0 · Answer 4 · answered Dec 13 '17 at 02:33

You could use in_array like this:

public function isUserStaff($uid) {
    $stmt = $this->conn->prepare("SELECT user_role FROM users WHERE user_id=:user_id");
    $stmt->execute(array(':user_id'=>$uid));
    $userRow = $stmt->fetch(PDO::FETCH_ASSOC);

    $role = $userRow['user_role'];

    return in_array($role, [
        3,
        4,
        5,
        6,
        7
    ], true);
}

How eliminate duplicate cases from a switch statement in PHP

4 Answers4

Linked