Page 1 of 1

Fixing a Code Smell

Posted: Wed Feb 13, 2008 12:14 pm
by seodevhead
Hey guys... I have a mysql web app that's purpose is similar to forums in that people post messages. And each registered user can belong to multiple usergroups. Some users may belong to none, some may just belong to one usergroup, while others may belong to many usergroups at one time.

When I go to output users' messages, I like to know which usergroups each member belongs to, because if they belong to 'usergroup A', then I'd like to output this graphic, and if they belong to 'usergroup B', then also add this graphic to their post, etc.

So I have a 'usergroups' table that is so:

Code: Select all

CREATE TABLE usergroups (
   id MEDIUMINT UNSIGNED NOT NULL AUTO_INCREMENT,
   user_id MEDIUMINT UNSIGNED NOT NULL,
   usergroup_id MEDIUMINT UNSIGNED NOT NULL,
   PRIMARY KEY (usergroup_id)
);
.. where 'user_id' refers to the user_id in the 'users' table (that holds all user information).

So cutting to the chase, I think I have a code smell in that when I go to output all the users messages, I first call them like so:

Code: Select all

SELECT u.user_id, u.username, u.firstLastName, m.message_id, m.message, m.post_date FROM users AS u, messages AS m WHERE u.user_id=m.user_id
.. which works.. but then I need to determine which usergroups each user that has a message belongs too...

Code: Select all

<?php
...
while($resultSet = mysql_fetch_assoc($message_query_result)))
{
   $usergroups_query = "SELECT usergroup_id FROM usergroups WHERE user_id={$resultSet['user_id']}";
   $usergroups_query_result = @mysql_query($usergroups_query);
   ....
   while ($ug_resultSet = mysql_fetch_assoc($usergroups_query_result)
   {
      if ($ug_resultSet['usergroup_id'] == '3')
         echo 'Usergroup 3 Graphic Here!';
      if ($ug_resultSet['usergroup_id'] == '5')
         echo 'Usergroup 5 Graphic Here!';
   }
}
?>
 
As you can see, I have a second mysql query that is occuring within the while loop of the first query. From what I understand, this is poor practice is it not? If so, is there a more 'optimized' way of getting all the usergroups a user belongs too in the original query? I have been experimenting with LEFT JOINING the usergroup table to the original, but that outputs duplicate posts for every usergroup a member belongs too.

Any help on this matter would GREATLY be appreciated. Thank you so much.

Re: Fixing a Code Smell

Posted: Wed Feb 13, 2008 1:38 pm
by John Cartwright
If there is a 1-1 relationship between the usergroups and the user, then use a INNER join. If the user can belong to several user groups you'll need to LEFT join the use php to sort the results properly.

Re: Fixing a Code Smell

Posted: Wed Feb 13, 2008 1:48 pm
by seodevhead
Hey JCart,

If I use a LEFT JOIN, it will return multiple records having the same message but different usergroups for each user. So you're saying to allow this "duplication" of sorts, to occur, but to use PHP to recognize it and only output each message once but still make itself aware of multiple usergroups a user can belong too?

I mean, how bad is it to have the second query in the first query's while loop? Is it really that bad? Thanks so much for your help and advice.

Re: Fixing a Code Smell

Posted: Wed Feb 13, 2008 2:26 pm
by John Cartwright
seodevhead wrote:Hey JCart,

If I use a LEFT JOIN, it will return multiple records having the same message but different usergroups for each user. So you're saying to allow this "duplication" of sorts, to occur, but to use PHP to recognize it and only output each message once but still make itself aware of multiple usergroups a user can belong too?

I mean, how bad is it to have the second query in the first query's while loop? Is it really that bad? Thanks so much for your help and advice.
By using a left join, you are not getting duplicate data, you are getting multiple records with the same id (but different attributes) however because there are multiple records in the user group table, therefore a row must be returned for each.

Therefore, what you need to do is to have php detect when the id has changed to display the content correctly.

Code: Select all

$lastid = null;
while ($row = mysql_fetch_assoc($result)) {
   if ($lastid !== $row['user_id']) {
     echo 'now on '. $row['user_id'] .'<br>';
   }
   echo 'belongs to user group: '. $row['usergroup_id'].'<br>';   
   $lastid = $row['user_id'];
}
In nearly all cases, a join will be faster than nested queries.