First, let's start with the way your query is formatted. I would strongly recommend you get in the habit of breaking your query across lines when they get long like this. I oftentimes use heredoc/nowdoc synteax for this.
For example:
$sql = <<<EOT
SELECT
r.contact_id,
contact_name,
group_id,
group_name,
phone_id,
phone_number
FROM contact r
LEFT JOIN contact_group s
ON r.contact_group = s.group_id
LEFT JOIN phone_numbers t
ON r.contact_id = t.contact_id
ORDER BY group_id, contact_id, phone_id
EOT;
This gives you a nice, clean visual presentation for your query in your code.
I think your query as written is a little odd, in that you specify the table for contact_id
but not for any of the other columns you are selecting. Some of your column names may be ambiguous across tables, so I try to make it a practice when writing join queries to specify the table names for ALL columns to remove ambiguity for the reader.
I would also suggest aliasing the columns. If you want to refer to contact_id
in your PHP code like you are doing, then you MUST alias r.contact_id
to this like r.contact_id AS contact_id
.
Finally, you should consider meaningful table aliases if using them (I actually typically do not use them except for sub-selects that need to be aliased). What do r
, s
, and t
mean? Why not c
, cg
, pn
or similar?
That could leave you with a query like:
$sql = <<<EOT
SELECT
contact.contact_id AS contact_id,
contact.contact_name AS contact_name,
contact_group.group_id AS group_id,
contact_group.group_name AS group_name,
phone_numbers.phone_id AS,
phone_numbers.phone_number AS phone_number
FROM contact
LEFT JOIN contact_group
ON contact.contact_group = contact_group.group_id
LEFT JOIN phone_numbers t
ON contact_group.contact_id = phone_numbers.contact_id
ORDER BY group_id, contact_id, phone_id
EOT;
This is a little more verbose, and might take you a minute or two of extra typing when initially writing the query, but it has the effect of removing any ambiguity for what your query is doing for the reader.
Outside of format, your query seems odd in that it doesn't seem to match the relations you have noted. You said the relationships are:
Contact Group ---(has)---> Contact ---(has)---> Phone number
Yet your joins would seem to suggest relations of:
Contact ---(has)---> Contact Group ---(has)---> Phone number
Can a contact exist that does not belong to a contact_group? Your query suggests this is the case (due to LEFT JOIN), which would beg the question, how are you handling the nulls that could be returned for contact_group fields in such a case?
You are taking a very standard approach to populating a multi-dimensional array from a results set that represents one-to-many relationships.
I do question however if this is an appropriate approach given the fact that you are using LEFT JOINs here. As noted above, you could have null values for the contract group information and the phone number information. You don't seem to be considering this at all.
This is especially critical given the fact that the contact group ID is the very first dimension in your array. Are you really intending to group on null
in this first dimension (technically such values would actually be assigned to an empty string array key)?
Since JSON can only represent two data structures - a numerically-indexed array, or an object - I would think that a numerically-indexed array would make more sense as the first dimension, meaning you might end up with JSON like:
[
{"group_id": 1, ... },
{"group_id": 2, ... },
...
]
I think numerically-indexed arrays might also be used to hold the contacts and phone number data nested underneath the contact group.
You are treating all values from the result set as strings (the way they are represented by default when retrieving from mysqli_fetch_array()
). Is this desired behavior, or does it make sense to cast primary key id's, for example, as integers?
Putting aside the problems noted above around LEFT JOINS and null. I would suggest that you consider working with each row of your results set as an object. IMO, this can make your code a lot cleaner and, in most cases, actually more truly represents what you are working with (an object representation retrieved from DB).
Taking this approach, as well as taking the approach noted on using numerically-indexed arrays for contact groups, contacts, and phone numbers, you might end up with something more like this:
$data = array();
$group = new stdClass();
$group->group_id = 0;
while ($row = mysqli_fetch_object($query)) {
// build new contact group if needed
if((int)$row->group_id !== $group->group_id) {
// break existing reference between $group and $data
// data in $data will remain after this dereferencing
unset($group);
$group = new stdClass();
$group->group_id = (int)$row->group_id;
$group->group_name = $row->group_name;
$group->contacts = array();
// build reference from group to location in $data
// this allows you to simply work with $group here in loop
// as opposed to $data[$index] which requires you to manually
// track index values
// taking similar apporach below for contacts as well eliminates
// $data[$group_index]->contacts[$contact_index]->phone_numbers[]
// types of references
$data[] =& $group;
// reset $contact
unset($contact);
$contact = new stdClass();
$contact->id = 0;
}
// build new contact if needed
if((int)$row->contact_id !== $contact->id) {
// dereference contact
unset($contact);
// create new contact
$contact = new stdClass();
$contact->id = (int)$row->contact_id;
$contact->name = $row->contact_name;
$contact->phone_numbers = array();
$group->contacts[] =& $contact;
}
// add phone number to current contact
$phone = new stdClass();
$phone->id = (int)$row->phone_id;
$phone->number = $row->phone_number;
$contact->phone_numbers[] = $phone;
}
unset($group, $contact, $phone);
You will note that taking this approach means that you are truly only building a single logical representation (a stdClass
object) for each contact group and contact and simply adding phone numbers to the current contact and adding contacts to the current group. I think thinking in terms of objects here really makes the code more meaningful in a real-world sense than building out a convoluted associative array. Considering the JSON serialization converts both objects and associative arrays to object notation anyway, there really seems to be no value to using associative array here.