will this if logic work?

[beary]

I have some code to generate a string of 3 letters followed by 3 numbers. The idea is that it generates a string, then looks in a mysql db to make sure the string doesn't exist, and if it doesn't, it accepts the code. Can anyone tell me whether the logic is sound? In other words, if it finds a match of the string in the db, will it make a different string? The code is

	$success='n';
	while($success=='n')
		{
		$x=1;
		while ($x <= 3) { $random .= rand(0,9); $x++; }	
		$rtocode=strtolower(substr($rtoname,0,3).$random);
		
		// Check rtocode not already used
		$q="select rtoid from rtos where rtoid='$rtocode'";
		$r=mysql_query($q) or sqlerror($q,mysql_error());
		if(mysql_numrows($r)==0)
			{
			$success='y';
			}
		}

[moltendorf]

Based on what I see, it will work. But, you could improve it by making a method that will guarantee that it will get a result it can insert on the first try (no while loop with an unknown break point), so that if/when the database gets filled with these values, you don't pound the database while trying to find a value that has not been used. Also, instead of doing that while loop wrapped around the rand(0,9) call, you could simply do rand(0,999).

[beary]

Moltendorf,

Thanks muchly for your reply. I'll certainly change the rand part. I also like the idea of avoiding the while loop altogether.Did you have anything in mind for accomplishing that (guaranteeing it gets a unique result on the 1st try)?

Based on what I see, it will work. But, you could improve it by making a method that will guarantee that it will get a result it can insert on the first try (no while loop with an unknown break point), so that if/when the database gets filled with these values, you don't pound the database while trying to find a value that has not been used. Also, instead of doing that while loop wrapped around the rand(0,9) call, you could simply do rand(0,999).

[Markus]

It makes more sense to do:

while(true) {
    //...
    if(mysql_num_rows($query) === 0) break;
}

The way you're doing it is hard to read and thus hard to maintain.

Mark.

[moltendorf]

It makes more sense to do:

while(true) {
    //...
    if(mysql_num_rows($query) === 0) break;
}

The way you're doing it is hard to read and thus hard to maintain.

Mark.

Actually, instead of checking in the first place, why not flat out insert it, and if it fails, continue? It would be quicker, and there is no chance for a race condition of another thread somehow inserting the same key just before another thread inserted the key just after the while loop finished. You would need to make sure the column for the code is set to primary or unique index though.
[code=php]while (true)
{
// Generate random code here.
// ...

$sql = 'INSERT INTO
rtos (rtoid)
VALUES (' . $rtocode . ');';

mysql_query ($sql)
or continue;

break;
}[/code]
I will give you a tip of how to make the guaranteed unique key on first try in a little bit, I just need to think of what must be done (since I haven't done that in a while).