Users online script, where is the problem?

Discussion in 'PHP' started by RyanDoubleyou, Jul 1, 2008.

  1. #1
    I have a users online script that tells me the users that are logged in. There is only one problem. Well, its not an error. But take a look:

    Just checking to see if the user is online
    
    function chklogin($postuser) {
    $time = time(); 
    $previous = "180"; 
    $timeout = $time-$previous; 
    $online = mysql_query("SELECT * FROM online WHERE timeout > '$timeout'"); 
    $row_online = mysql_fetch_array($online); 
    if ($row_online['username'] == $postuser) { 
    return '<img src="/images/icons/user_comment.png" alt="online" height="16" width="16" />';
    } else {
    return '<img src="/images/icons/user_delete.png" alt="offline" height="16" width="16" />'; 
    }
    }
    PHP:
    This bit, shows all of the users online, not just one.
    $time = time(); 
    $previous = "180"; 
    $timeout = $time-$previous;
    
    if (isset($_SESSION['username'])) { 
    $username = $_SESSION['username']; 
    $verify = mysql_query("SELECT * FROM online WHERE username=\"$username\" AND timeout > \"$timeout\""); 
    
    $row_verify = mysql_fetch_assoc($verify); 
    if (!isset($row_verify['username'])) { 
    $insert = mysql_query("INSERT INTO online (username, timeout) VALUES (\"$username\", \"$time\")"); 
    }
    }
    
    $online = mysql_query("SELECT * FROM online WHERE timeout > \"$timeout\""); 
    $row_online = mysql_fetch_assoc($online); 
    if (isset($row_online['username'])) { 
    echo 'Logged in users: ';
    do {
    echo $row_online['username'].", "; 
    } while($row_online = mysql_fetch_assoc($online));
    } else {
    echo "There are no logged in users."; 
    }
    PHP:
    The second one works. On the first one, I am using it on my forum. For each post, it has a chklogin($username); The problem is that when there is 2 or more users online, it only says that one is online. It says all the others are offline.

    ::Notice::! When I say "it only says that one is online" I means the first bit on code. That has the function chklogin();
     
    RyanDoubleyou, Jul 1, 2008 IP
  2. AliasXNeo

    AliasXNeo Banned

    Messages:
    151
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #2
    Well from what I can tell you should be looping through all the MySQL results, not just the first one. It seems you just get the first result, check to see if it matches the current user and if it does then you say they are online.

    Basically just add a while loop to the check:

    $result= mysql_query("SELECT * FROM online WHERE timeout > '$timeout'");
    while ($row = mysql_fetch_assoc($result))
    {
        if ($row['username'] == $postuser) { 
            return '<img src="/images/icons/user_comment.png" alt="online" height="16" width="16" />';
        } else {
            return '<img src="/images/icons/user_delete.png" alt="offline" height="16" width="16" />'; 
        }
    }
    PHP:
     
    AliasXNeo, Jul 1, 2008 IP
  3. clarky_y2k3

    clarky_y2k3 Well-Known Member

    Messages:
    114
    Likes Received:
    9
    Best Answers:
    0
    Trophy Points:
    108
    #3
    Wouldn't it be a better idea to query the number of rows as opposed to retreiving and looping over the entire table?
     
    clarky_y2k3, Jul 1, 2008 IP
  4. RyanDoubleyou

    RyanDoubleyou Peon

    Messages:
    86
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    0
    #4
    I just want to grab the if(user is online) data for 1 user
     
    RyanDoubleyou, Jul 1, 2008 IP
  5. AliasXNeo

    AliasXNeo Banned

    Messages:
    151
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #5
    No idea what you're talking about. He needs to check the username for each result in which timeout has not yet expired so it's only logical to loop through each result.
     
    AliasXNeo, Jul 1, 2008 IP
  6. clarky_y2k3

    clarky_y2k3 Well-Known Member

    Messages:
    114
    Likes Received:
    9
    Best Answers:
    0
    Trophy Points:
    108
    #6
    I'd do it something like this (unless I've got the wrong end of the stick so-to-speak):
    
    <?php
    mysql_query('SELECT `id` FROM `online` WHERE `username` = "' . $postuser . '" AND `timeout` > ' . $timeout); // replace the id field name appropriately, there is no need to retreive all fields, one will do
    $iRows  = mysql_num_rows($rQuery);
    mysql_free_result($rQuery);
    
    if ($iRows == 1)
    {
    	// online
    }
    else
    {
        // offline
    }
    ?>
    
    PHP:
     
    clarky_y2k3, Jul 1, 2008 IP
  7. AliasXNeo

    AliasXNeo Banned

    Messages:
    151
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #7
    Ok, you're not understanding my point. Did you try my code? I think if you do you'll see what I'm saying.

    Think about what's happening. You're querying the database to retrieve ALL timeouts that are greater than the current timeout. Assuming there's not one row in your database, multiple rows can be returned. If you want to just find one user then just query like this:

    $online = mysql_query("SELECT * FROM online WHERE timeout > '$timeout' AND username = '$postuser'");
    PHP:
     
    AliasXNeo, Jul 1, 2008 IP
  8. RyanDoubleyou

    RyanDoubleyou Peon

    Messages:
    86
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    0
    #8
    Alright AliasXNeo, I tried that one line, and it worked. Thanks to you two for helping :D
     
    RyanDoubleyou, Jul 1, 2008 IP
  9. clarky_y2k3

    clarky_y2k3 Well-Known Member

    Messages:
    114
    Likes Received:
    9
    Best Answers:
    0
    Trophy Points:
    108
    #9
    If I understand and you only need the data from one user, the current method is overkill.

    My method is more efficient and quicker.
     
    clarky_y2k3, Jul 1, 2008 IP
  10. AliasXNeo

    AliasXNeo Banned

    Messages:
    151
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #10
    Yes your method would be cleaner and I would have suggested it if I understood the first time.

    The best way to do this:

    if (mysql_num_rows(mysql_query("SELECT `username` FROM `online` WHERE `timeout` > '{$timeout}' AND `username` = '{$postuser}'"))
    {
        // User is active //
    } else {
        // User is not active //
    }
    PHP:
    Make sure you use mysql_real_escape_string() on $postuser for security reasons.
     
    AliasXNeo, Jul 1, 2008 IP
  11. RyanDoubleyou

    RyanDoubleyou Peon

    Messages:
    86
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    0
    #11
    Hmm, I dont think so, I protected that in the registration.

    Okay, So this should work out for me
     
    RyanDoubleyou, Jul 1, 2008 IP