Is there a more efficient way to do this username check?

[Merdok]

I've written a bit of code to automatically check for the existence of a username and add information to the end of it until a free username is found, however it makes quite a few calls to the database and I'm sure there will be a better solution, could you guys please have a look and tell me what you think?

 

// Check for existing username and automatically append things on to the end until a free name is found.
$UNToCheck = $usr_firstName .'_'.$usr_lastName; 
$UNCheck = mysql_query("SELECT COUNT (*) as usercount FROM core_users WHERE usr_username =" $UNToCheck ) or die ('UNCheck failed'.mysql_error());
if ($usercount != 0) { // If username exists
$UNToCheck = $usr_firstName .'_'.$usr_lastName.'2'; //Add a '2' on the end
mysql_query("SELECT COUNT (*) as usercount2 FROM core_users WHERE usr_username =" $UNToCheck ) or die ('UNCheck failed'.mysql_error());
}
if ($usercount2 != 0) { // If username exists
$UNToCheck = $usr_firstName .'_'.$usr_lastName.date(Y); // Add the 4 digit current year on the end
mysql_query("SELECT COUNT (*) as usercount3 FROM core_users WHERE usr_username =" $UNToCheck ) or die ('UNCheck failed'.mysql_error());
}
if ($usercount3 != 0) { // If username exists
$UNToCheck = $usr_firstName .'_'.$usr_lastName.date(U); //add the UNIX time code on the end (always will be unique)
mysql_query("SELECT COUNT (*) as usercount4 FROM core_users WHERE usr_username =" $UNToCheck ) or die ('UNCheck failed'.mysql_error());
}
$usr_username = $UNToCheck

 

Thank you.

 

[gamblor01]

I don't think it's going to be terribly inefficient just because it makes a few calls to the database, especially because it's all in the same PHP page.  If you had multiple pages that kept calling each other then that would mean that you are constantly creating and destroying connections to the database -- which would make it considerably less efficient.

 

However, the queries are all using the same connection and you only have 3 queries in a row.  Databases index tables and optimize queries and so forth...I don't think you have anything to worry about.

 

Why do you always append the number 2 though?  Why not pick a pseudorandom number to append to the username -- that would be more likely to work without having to add a year or the Unix timestamp.  Just do something like:

 

if ($usercount != 0) { // If username exists
$rand = rand(1, 4096);  // append a 12-bit random number
$UNToCheck = $usr_firstName .'_'.$usr_lastName.$rand;

 

Have you tested this code however?  I don't have any experience using an AS clause like you did.  Instead, I call the mysql_query() function and then I use a call to mysql_num_rows().  For example:

 

// Check for existing username and automatically append things on to the end until a free name is found.
$UNToCheck = $usr_firstName .'_'.$usr_lastName; 
$UNCheck = mysql_query("SELECT COUNT (*) as usercount FROM core_users WHERE usr_username =" $UNToCheck ) or die ('UNCheck failed'.mysql_error());
$mycount = mysql_num_rows($UNCheck);
if ($usercount != 0) { // If username exists

 

Your implementation might work...I have just never tried your code so I didn't know it works.  Pretty neat if it does though.

 

One final thing you may want to consider is to call stripslashes() and mysql_real_escape_string() if you haven't already.  Call that before your SELECT statements against the DB.

[ignace]

function my_generate_username($prefix) {
    return $prefix . implode('', array_rand(range(0, 9), 3));
}

$usernames = array_map('my_generate_username', array_fill(0, 12, $usr_username)); // generate 12 usernames (~$usr_username[0-9]{3})

// return all taken usernames
$query = 'SELECT usr_username FROM core_users WHERE usr_username IN (' . implode(', ', $usernames) . ')';
$result = mysql_query($query);
if ($result) {
    $row_count = mysql_num_rows($result);
    if (0 === $row_count) { // none of the generated usernames were taken
        $free_usernames = array_rand($usernames, 3); // select 3 usernames from the collection
    } else {
        while (list($username) = mysql_fetch_array($result, MYSQL_NUM)) {
            $key = array_search($username, $usernames);
            if (false !== $key) {
                unset($usernames[$key]); // already taken
            }
        }
        $free_usernames = array_rand($usernames, 3);
    }
    print_r($free_usernames);
}

[Axeia]

Why not send all suggestions at once?

 

<?php
  //Change the usr_username to reflect you database structure.
  $query = sprintf( 
    'SELECT usr_username as usercount 
    FROM core_users 
    WHERE usr_username = "%s" OR usr_username = "%s" OR usr_username "%s"
    ORDER BY LENGTH( username ) DESC',
    mysql_real_escape_string( $usr_firstName.'_'.$usr_lastName ),
    mysql_real_escape_string( $usr_firstName.'_'.$usr_lastName.'2' ),     //Add a '2' on the end
    mysql_real_escape_string( $usr_firstName.'_'.$usr_lastName.date(Y) ), // Add the 4 digit current year on the end
    mysql_real_escape_string( $usr_firstName.'_'.$usr_lastName.date(U) )  //add the UNIX time code on the end (always will be unique)
  );

$result = mysql_query($query);
if (!$result)
    echo 'Sorry the username and all generated suggestions are taken.';

$usr_username = mysql_result($result, 0);

 

1 Query to execute.

 

Next time don't forget the document for mysql_query. This bit in particular is of importance:

 

    A SQL query

 

    The query string should not end with a semicolon. Data inside the query should be properly escaped.