TeddyKiller Posted April 5, 2010 Share Posted April 5, 2010 I have a function, where $user, is the data for the current logged in user. $profile, is the data for the current profile being viewed. $id, is the current id of the profile. I'm just wondering how I can .. make this function better, maybe with less code, maybe arrays for some information if needed etc. I'm useless to functions, but maybe you can help make it better. Here is the function, currently working. function user_details($id, $profile, $user) { $query = mysql_query("select * from `users` where `id`='$id'"); $row = mysql_fetch_array($query); //Open profile header $prfhdr = '<div id="profheader" style="padding-top:10px; min-height:150px; border:1px solid black;">'; //Displaying the about me and status section (Right collum) $prfhdr .= '<div style="float:right; width:390px; margin-right:10px;"> <div style="background:#E1E3DE; padding:4px;">'; $query1 = mysql_query("select * from `user_status` where `user_id`='$id' order by 'posted' desc LIMIT 1"); if(mysql_num_rows($query1) > 0) { $row1 = mysql_fetch_array($query1); $prfhdr .= 'Status: '.$row1['status'].' - '.date("F j, Y, g:i a",$row1['posted']).''; } else { $prfhdr .= 'Status: <i>No Status</i>'; } $prfhdr .= '</div>'; if($profile->about_me){ $prfhdr .= '<strong>About me</strong><br /><i>'.$profile->about_me.'</i>'; } else { $prfhdr .= '<strong>About me</strong><br /><i>Unknown</i>'; } $prfhdr .= '</div>'; //Displaying the add as a friend link and profile photo (Left side) $prfhdr .= '<div style="text-align:center; width:130px; float:left;">'; $prfhdr .= '<img src="resize_image.php?file='.$row['avatar'].'&size=120" border=0 /><br />'; $query = mysql_query("select * from `friends` where `user_id`='$user->id' and `friend_id`='$id'"); if(mysql_num_rows($query) == 1) { $prfhdr .= 'Already Friends'; } else { $query = mysql_query("select * from `friend_requests` where `request_by`='$user->id' and `request_to`='$id' and `status`='0'"); if(mysql_num_rows($query) == 1) { $prfhdr .= "Friend request sent"; } else { if($user->id != $id) { $prfhdr .= "<a href=\"#\" onclick=\"$.facebox.settings.opacity = 0.2; jQuery.facebox({ajax: 'profile/friends/request.php?uid=$id'})\">Add as a friend</a>"; } } } $prfhdr .= '</div>'; //Displaying the user details (Center collum) $prfhdr .= '<div style="margin-left:140px; margin-right:400px;"> <strong>User Details</strong> <table> <tr><td>Username:</td><td><i>'.ucwords($row['username']).'</i></td></tr> <tr><td>Registered on:</td><td><i>'.date('F j, Y',$row['date']).'</i></td></tr> <tr><td>Last Active:</td><td><i>'.date('F j, Y',$row['last_login']).'</i></td></tr> <tr><td>Martial Status:</td><td><i>'; if($profile->rel_status){ $prfhdr .= $profile->rel_status; } else { $prfhdr .= 'Unknown'; } $prfhdr .= '</i></td></tr> <tr><td>Orientation:</td><td><i>'; if($profile->orien_status){ $prfhdr .= $profile->orien_status; } else { $prfhdr .= 'Unknown'; } $prfhdr .= '</i></td></tr> <tr><td>Looking for:</td><td><i>'; if($profile->looking_for){ $prfhdr .= $profile->looking_for; } else { $prfhdr .= 'Unknown'; } $prfhdr .= '</i></td></tr> </table> </div>'; //Close profile header $prfhdr .= '</div>'; return $prfhdr; } Thanks Quote Link to comment Share on other sites More sharing options...
ignace Posted April 6, 2010 Share Posted April 6, 2010 If you want to add flexibility to your functions you may want to try: function html_profile($id) { $user = user_get_details($id); $post = user_get_posts($id, 'DESC', 1); $post = empty($post) ? array() : $post[0]; $html_status = 'Status: ' . (empty($post) ? '<em>No status</em>' : $post['status'] . ' - ' . date('F j, Y, g:i:a', strtotime($post['posted']))); $html_about = '<strong>About me</strong><br /><em>' . (isset($user['about_me']) ? $user['about_me'] : 'Unknown') . '</em>'; $html_friend = user_is_friend($id, $_SESSION['id']) ? 'Already friends' : (user_has_invite($id, $_SESSION['id']) ? 'Friend request sent' : '<a href="#" onclick="$.facebox.settings.opacity = 0.2; jQuery.facebox({ajax: \'profile/friends/request.php?uid=$id\'})"> Add as a friend</a>'); $html_username = '<em>' . $user['username'] . '</em>'; $html_register_date = '<em>' . date('F j, Y', $user['date']) . '</em>'; $html_last_active_date = '<em>' . date('F j, Y', $user['last_login']) . '</em>'; $html_maritial_status = '<em>' . (isset($user['rel_status']) ? $user['rel_status'] : 'Unknown') . '</em>'; $html_sexual_orientation = '<em>' . (isset($user['orien_status']) ? $user['orien_status'] : 'Unknown') . '</em>'; $html_looking_for = '<em>' . (isset($user['looking_for']) ? $user['looking_for'] : 'Unknown') . '</em>'; return '<div id="profheader" style="padding-top:10px; min-height:150px; border:1px solid black;">' . '<div style="float:right; width:390px; margin-right:10px;">' . '<div style="background:#E1E3DE; padding:4px;">' . $html_status . '</div>' . $html_about . '</div>' . '<div style="text-align:center; width:130px; float:left;">' . '<img src="resize_image.php?file=' . (isset($user['avatar']) ? $user['avatar'] : 'images/avatars/default.jpg') . '&size=120" border="0" />' . $html_friend . '</div>' . '<div style="margin-left:140px; margin-right:400px;">' . '<strong>User Details</strong>' . '<table>' . '<tr><th>Username</th><td>' . $html_username . '</td></tr>' . '<tr><th>Registered on</th></td>' . $html_register_date . '</td></tr>' . '<tr><th>Last active on</th><td>' . $html_last_active_date . '</td></tr>' . '<tr><th>Maritial status</th><td>' . $html_maritial_status . '</td></tr>' . '<tr><th>Sexual orientation</th><td>' . $html_sexual_orientation . '</td></tr>' . '<tr><th>Looking for</th><td>' . $html_looking_for . '</td></tr>' . '</table>' . '</div>' . '</div>'; } function user_get_details($id) { $id = intval($id); if (0 === $id) return array(); $result = mysql_query("SELECT * FROM users WHERE id = $id"); return $result ? mysql_fetch_assoc($result) : array(); } function user_get_posts($id, $sort = 'ASC', $limit = 0) { $id = intval($id); if (0 === $id) return array(); $result = mysql_query("SELECT * FROM user_status WHERE user_id = $id ORDER BY posted $sort"); if (!$result) return array(); $posts = array(); $i = 1; while ($row = mysql_fetch_assoc($result)) { $posts[] = $row; if ($limit > 0 && $i == $limit) break; ++$i; } mysql_free_result($result); return $posts; } function user_get_friends($id, $limit = 0) { $id = intval($id); if (0 === $id) return array(); $result = mysql_query("SELECT * FROM friends WHERE user_id = $id"); if (!$result) return array(); $friends = array(); $i = 1; while ($row = mysql_fetch_assoc($result)) { $friends[] = $row; if ($limit > 0 && $i == $limit) break; ++$i; } mysql_free_result($result); return $friends; } function user_is_friend($user_id, $friend_id, $table_rows = array()) { $user_id = intval($user_id); $friend_id = intval($friend_id); if ($user_id < 1 || $friend_id < 1) return false; if (!empty($table_rows)) { $key = key($table_rows); if (!is_integer($key)) { return isset($row['user_id']) && isset($row['friend_id']) && $user_id == $row['user_id'] && $friend_id == $row['friend_id']; } else foreach ($table_rows as $row) { if (isset($row['user_id']) && isset($row['friend_id'])) { if ($user_id == $row['user_id'] && $friend_id == $row['friend_id']) return true; } } return false; } else { $result = mysql_query("SELECT user_id FROM friends WHERE user_id = $user_id AND friend_id = $friend_id"); return $result ? true : false; } } function user_has_invite($user_id, $friend_id, $table_rows = array()) { $user_id = intval($user_id); $friend_id = intval($friend_id); if ($user_id < 1 || $friend_id < 1) return false; if (!empty($table_rows)) { $key = key($table_rows); if (!is_integer($key)) { return isset($row['user_id']) && isset($row['friend_id']) && $user_id == $row['user_id'] && $friend_id == $row['friend_id'] && $row['status'] == 0; } else foreach ($table_rows as $row) { if (isset($row['user_id']) && isset($row['friend_id'])) { if ($user_id == $row['user_id'] && $friend_id == $row['friend_id'] && $row['status'] == 0) return true; } } return false; } else { $result = mysql_query("SELECT status FROM friend_requests WHERE user_id = $user_id AND friend_id = $friend_id AND status = 0"); return $result ? true : false; } } Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 and if I'm correct with saying, I'll only need to call html_profile and that handles it all? Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 For some reason, the registered date ends up by "user details" Theres no reason for it to? Also, if $id == $_SESSION['id'], is it possible for you to fit something like that in so I could perhaps add stuff in if the current user is viewing their own profile. Quote Link to comment Share on other sites More sharing options...
ignace Posted April 6, 2010 Share Posted April 6, 2010 Your question was How can I make this function better I showed you how you can split your code into multiple functions for added flexibility and better maintainability. My code was not intended to be just copy-pasted to your editor I'm quite frankly amazed that it works I used $_SESSION to show you that the added parameters $profile and $user had no value and could be replaced with something more easier to maintain the same functionality Also, if $id == $_SESSION['id'], is it possible for you to fit something like that in so I could perhaps add stuff in if the current user is viewing their own profile. Just add anything you want added to html_profile() Like for example: return '<div style="display:' . ($id == $_SESSION['id'] ? 'block' : 'none') . '; color:#0C0;">THIS IS YOU!</div>' . '.. PS The code I wrote is really powerful like: user_is_friend($user_id, $friend_id, $table_rows = array()) Can be used like: user_is_friend(1, 1); user_is_friend(1, 1, array('user_id' => 1, 'friend_id' => 1)); user_is_friend(1, 1, array(array('user_id' => 1, 'friend_id' => 1), array('user_id' => ..))); This may not be obvious if you just look at the code PPS you also may want to review you DB design/scheme as I found that you used unnecessary tables like friend_requests for example Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 Yeah. I was reading through it. I tweaked a bit as some values weren't being gotten, so I passed the $profile variable through. I just don't understand how the registered user bit is ending up in the completely wrong place? I mean.. this seems pretty straight forward.. '<div style="margin-left:140px; margin-right:400px;">' . '<strong>User Details</strong>' . '<table>' . '<tr><td>Username</td><td>' . $html_username . '</td></tr>' . '<tr><td>Registered on</td></td>' . $html_register_date . '</td></tr>' . '<tr><td>Last active on</td><td>' . $html_last_active_date . '</td></tr>' . '<tr><td>Maritial status</td><td>' . $html_maritial_status . '</td></tr>' . '<tr><td>Sexual orientation</td><td>' . $html_sexual_orientation . '</td></tr>' . '<tr><td>Looking for</td><td>' . $html_looking_for . '</td></tr>' . '</table>' . '</div>' . It just doesn't make sense. I changed this line '<tr><td>Registered on</td></td>' . $html_register_date . '</td></tr>' . To '<tr><td>Registered on</td></td>' . '</td></tr>' . This doesn't display the date at all.. so for some reason it's getting the contents of the second <td></td> where it's not suppose to. It's highly confusing. The table is also more spaced out than what it use to be. but still, it just doesn't make sense. I'm absolutely puzzled. Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 PPS you also may want to review you DB design/scheme as I found that you used unnecessary tables like friend_requests for example It took me ages to figure out how to do friend requests and accept them. You click "add as a friend" or whatever, it inserts a row into "friend requests",with a status of 0. If this status is 1, (so invitation was declined by other user) it doesn't add them as a friend in to the friends table. If the status is 2, so the invitation was accepted.. it adds two rows into the friends database. eg: user_id friend_id 11 16 16 11 To make it easier to grab the friends. I'd basically do.. get friend_id where user_id == $user_id (logged in user) and for displaying friends on profiles, it'll be the same query, but replace $user_id with perhaps $_GET['id'] I'm not good at organising.. I'm useless at it. I'm sure I'm annoying people on here too.. but I am trying. Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 It seems to only be the following line '<tr><td>Registered on</td></td>' . $html_register_date . '</td></tr>' . I can move it anywhere else on that field, and it displays it in the wrong place. Yet the source code view in firefox shows it displaying where it should. I'm really confused? Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 The difference between.. $html_register_date = '<em>' . date('F j, Y', $user['date']) . '</em>'; return '<tr><td>Member since:</td></td>' . $html_register_date . '</td></tr>' ; and return '<tr><td>Member since:</td><td><em>' . date('F j, Y', $user['date']) . '</em></td></tr>'; Is what exactly? The top method brings up the dodgey aligning, the bottom method displays it perfectly. IT DOESN'T MAKE SENSE. It's driving me rather insane, when two same lines.. don't work?! Edit: OMGOMGOMGOMGOGOMGMG A simple / Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 Right ok. Ignace, I understand your if statements.. I've been changing another function of mine to a similar one as you gave. Though, I'm not sure how I'd do this, as it involves some whiles. etc - I've changed it slightly though. function friend_display($id) { $query = mysql_query("SELECT * FROM `friends` WHERE `user_id`='$id' ORDER BY RAND() LIMIT 12"); if(mysql_num_rows($query) == 0) { $html_friends = 'This user has no friends'; } else { while($row = mysql_fetch_array($query)) { $sql = mysql_query("SELECT `avatar` FROM `users` WHERE `id`='".$row['friend_id']."'"); if (mysql_num_rows($sql) > 0) { $html_amount_friends = ((mysql_num_rows($query)) == 1 ? '<a href="#" style="float:right; margin-right:5px;">1 Friend</a><br />' : '<a href="#" style="float:right; margin-right:5px;">'.mysql_num_rows($query).' Friends</a><br />'); while ($item = mysql_fetch_array($sql)) { $html_friends = '<a href="profile.php?uid='.$row['friend_id'].'"> <img src="resize_image.php?file='.$item['avatar'].'&size=60" title="" border="0" /> </a>'; } } } } return '<div style="float:left; width:400px; margin-top:10px;">' . '<div style="background:#e11919; text-align:center; color:#fff; font-size:14px; font-weight:bold; padding:5px;">' . 'Friends' . '</div>' . '<div style="border:1px solid #000; text-align:center; padding-bottom:10px;">' . $html_amount_friends . $html_friends . '</div>' . '</div>'; } Not too sure much can be done with it to be honest. Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 Further basing on Ignace's code, I've put myself into a problem and I can't find a way out. I'm wanting to return 2 values. $avatar and $friend_id. I tried putting them into an array, and returning the array. Though for some reason, this isn't doing it. Hopefully someone can help. Here is my 3 functions. function html_friends($id) { $query = mysql_query("SELECT * FROM `friends` WHERE `user_id`='$id' ORDER BY RAND() LIMIT 12"); $html_amount_friends = ((mysql_num_rows($query) == 1) ? '<a href="#" style="float:right; margin-right:5px;">1 Friend</a><br />' : '<a href="#" style="float:right; margin-right:5px;">'.mysql_num_rows($query).' Friends</a><br />'); $html_friends = (mysql_num_rows($query) == 0) ? 'This user has no friends' : (user_retrieve_friends($id, $query) ? '<a href="profile.php?uid='.$array[1].'"> <img src="resize_image.php?file='.$array[0].'&size=60" title="" border="0" /> </a>' : ''); return '<div style="float:left; width:400px; margin-top:10px;">' . '<div style="background:#e11919; text-align:center; color:#fff; font-size:14px; font-weight:bold; padding:5px;">' . 'Friends' . '</div>' . '<div style="border:1px solid #000; text-align:center; padding-bottom:10px;">' . $html_amount_friends . $html_friends . '</div>' . '</div>'; } function user_retrieve_friends($id, $query) { $id = intval($id); while($row = mysql_fetch_array($query)) { $friend_id = $row['friend_id']; $avatar = user_fetch_avatars($id); $avatar = $avatar['avatar']; return $array = array($avatar, $friend_id); } } function user_fetch_avatars($id) { $id = intval($id); $result = mysql_query("SELECT avatar FROM users WHERE id='$id'"); return $result ? mysql_fetch_assoc($result) : array(); } Quote Link to comment Share on other sites More sharing options...
TeddyKiller Posted April 6, 2010 Author Share Posted April 6, 2010 Changed it to function html_friends($id) { $query = mysql_query("SELECT * FROM `friends` WHERE `user_id`='$id' ORDER BY RAND() LIMIT 12"); $html_amount_friends = ((mysql_num_rows($query) == 1) ? '<a href="#" style="float:right; margin-right:5px;">1 Friend</a><br />' : '<a href="#" style="float:right; margin-right:5px;">'.mysql_num_rows($query).' Friends</a><br />'); while($row = mysql_fetch_array($query)) { $avat = user_fetch_avatars($row['friend_id']); $html_friends = ((mysql_num_rows($query) == 0) ? 'This user has no friends' : '<a href="profile.php?uid='.$row['friend_id'].'"> <img src="resize_image.php?file='.$avat['avatar'].'&size=60" title="" border="0" /> </a>'); } return '<div style="float:left; width:400px; margin-top:10px;">' . '<div style="background:#e11919; text-align:center; color:#fff; font-size:14px; font-weight:bold; padding:5px;">' . 'Friends' . '</div>' . '<div style="border:1px solid #000; text-align:center; padding-bottom:10px;">' . $html_amount_friends . $html_friends . '</div>' . '</div>'; } function user_fetch_avatars($id) { $id = intval($id); $result = mysql_query("SELECT avatar FROM users WHERE id='$id'"); return $result ? mysql_fetch_assoc($result) : array(); } And it works. Was hoping to do it differently, but looking back at it.. it wouldn't of made sense anyway. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.