HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Loading objects for users, roles, and groups from a query with LEFT OUTER JOINs

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromleftgroupsobjectsroleswithqueryforloadingand

Problem

I have this function that returns me a list of users with their roles and groups. As you can see, this is how I fetch and create list of objects. I'm wondering whether this is a good approach and what parts should be improved. I'm not that experienced with PHP so I would appreciate code samples.

I'm also wondering how good an approach it is to first get all users and then make another prepared statement to get user roles and groups. That would mean I will have big number of database calls, so I think it's a bad idea.

```
$stmt = $mysqli->prepare("SELECT u.id, u.firstName, u.lastName, u.email,
u.phoneNumber, u.address, u.birthDate, ur.roleName, cg.id, cg.name FROM users as u
LEFT OUTER JOIN user_role as ur ON u.id = ur.userId
LEFT OUTER JOIN user_group as ug on ug.userId = u.id
LEFT OUTER JOIN control_group as cg on cg.id = ug.groupId WHERE u.id != ?");
$stmt->bind_param("i", $_SESSION["id"]);
$stmt->execute();
$stmt->bind_result($id, $firstName, $lastName, $email, $phoneNumber,
$address, $birthDate, $roleName, $groupId, $groupName);
$users = array();

while ($stmt->fetch()) {
if (empty($users[$id])) {
$users[$id] = array(
'id' => $id,
'firstName' => $firstName,
'lastName' => $lastName,
'email' => $email,
'phoneNumber' => $phoneNumber,
'address' => $address,
'birthDate' => $birthDate,
'roles' => array(),
'groups' => array()
);
}
if ($roleName) {
$found = false;
foreach ($users[$id]['roles'] as $role) {
if($role['roleName'] == $roleName){
$found = true;
break;
}
}
if($found == false)
$users[$id]['roles'][] = array(
'roleName' => $roleName
);

Solution

For the exact case you posted here, I would suggest to use group_concat() in the query. It will dramatically reduce the amount of code required, as well as the amount of data delivered from database.

Besides, I see a very little use for the prepared statement here, so the amount of code could be dramatically reduced on this account as well.

$sql = "SELECT u.id, u.firstName, u.lastName, u.email, u.phoneNumber, 
    u.address, u.birthDate,group_concat(ur.roleName), roles group_concat(cg.name) groups 
    FROM users as u 
    LEFT OUTER JOIN user_role as ur ON u.id = ur.userId 
    LEFT OUTER JOIN user_group as ug on ug.userId = u.id 
    LEFT OUTER JOIN control_group as cg on cg.id = ug.groupId 
    GROUP BY u.id";
$res = $mysqli->query($sql);
$users = array();
while ($row = $res->fetch_assoc()) {
    if ($row['id'] == $_SESSION["id"]) {
        continue;
    }
    $row['groups'] = explode(",", $row['groups']);
    $row['roles'] = explode(",", $row['roles']);
    $users[] = $row;
}


This is the response example which I consider most usable and clear.

But note it would take no trouble to make it exactly as in your example above, if you wish.

On a second thought, thinking one move ahead, I suppose you may wish to make groups and roles some sort of interactive - allowing clicks or other interactions with them. And for this purpose you will need group and roled IDs inevitably.

Therefore, to make this code robust and usable in the real life app, you have to supply group and role id as well. For this purpose I can't help it but use PDO, as its helper methods do A LOT of your job. For example, PDO::FETCH_KEY_PAIR gives you nice key-value pairs right out of SQL query:

$groups = $pdo->query("SELECT id, name FROM control_group")->fetchAll(PDO::FETCH_KEY_PAIR);
$roles  = $pdo->query("SELECT id, roleName FROM user_role")->fetchAll(PDO::FETCH_KEY_PAIR);

$sql = "SELECT u.id, u.firstName, u.lastName, u.email, u.phoneNumber, 
    u.address, u.birthDate,group_concat(ur.roleName), roles group_concat(cg.name) groups 
    FROM users as u 
    LEFT OUTER JOIN user_role as ur ON u.id = ur.userId 
    LEFT OUTER JOIN user_group as ug on ug.userId = u.id 
    LEFT OUTER JOIN control_group as cg on cg.id = ug.groupId 
    GROUP BY u.id";
$res = $pdo->query($sql);
$users = array();
while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
    if ($row['id'] == $_SESSION["id"]) {
        continue;
    }

    $user_groups = array();
    foreach(explode(",", $row['groups'] as $id) {
        $user_groups[$id] = $groups[$id]
    }
    $row['groups'] = $user_groups;

    $user_roles = array();
    foreach(explode(",", $row['roles']) as $id) {
        $user_roles[$id] = $roles[$id]
    }

    $row['roles'] = $user_roles;
    $users[] = $row;
}


This is a response example.

As you can see, here we're first selecting all the group and role names, while in the main query selecting only ids. Then in the short loop finally creating groups and roles arrays.

Code Snippets

$sql = "SELECT u.id, u.firstName, u.lastName, u.email, u.phoneNumber, 
    u.address, u.birthDate,group_concat(ur.roleName), roles group_concat(cg.name) groups 
    FROM users as u 
    LEFT OUTER JOIN user_role as ur ON u.id = ur.userId 
    LEFT OUTER JOIN user_group as ug on ug.userId = u.id 
    LEFT OUTER JOIN control_group as cg on cg.id = ug.groupId 
    GROUP BY u.id";
$res = $mysqli->query($sql);
$users = array();
while ($row = $res->fetch_assoc()) {
    if ($row['id'] == $_SESSION["id"]) {
        continue;
    }
    $row['groups'] = explode(",", $row['groups']);
    $row['roles'] = explode(",", $row['roles']);
    $users[] = $row;
}
$groups = $pdo->query("SELECT id, name FROM control_group")->fetchAll(PDO::FETCH_KEY_PAIR);
$roles  = $pdo->query("SELECT id, roleName FROM user_role")->fetchAll(PDO::FETCH_KEY_PAIR);

$sql = "SELECT u.id, u.firstName, u.lastName, u.email, u.phoneNumber, 
    u.address, u.birthDate,group_concat(ur.roleName), roles group_concat(cg.name) groups 
    FROM users as u 
    LEFT OUTER JOIN user_role as ur ON u.id = ur.userId 
    LEFT OUTER JOIN user_group as ug on ug.userId = u.id 
    LEFT OUTER JOIN control_group as cg on cg.id = ug.groupId 
    GROUP BY u.id";
$res = $pdo->query($sql);
$users = array();
while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
    if ($row['id'] == $_SESSION["id"]) {
        continue;
    }

    $user_groups = array();
    foreach(explode(",", $row['groups'] as $id) {
        $user_groups[$id] = $groups[$id]
    }
    $row['groups'] = $user_groups;

    $user_roles = array();
    foreach(explode(",", $row['roles']) as $id) {
        $user_roles[$id] = $roles[$id]
    }

    $row['roles'] = $user_roles;
    $users[] = $row;
}

Context

StackExchange Code Review Q#156369, answer score: 2

Revisions (0)

No revisions yet.