[RESOLVED] random() function, Looking for improvements!

spiritssight

Hello All,

I am looking for any feed back on improving the code below, I am trying to design this that its good for a few different uses, one being to create user_ids, record_ids, and activation codes / verifie codes, I have not come up with any other reasons yet but if I do I want to be able to use this same function and just add any speical part like I did for the record Id and user id part as you will see, is there a better way to write this and make it have a high change of being uniqe for the first try. I notice alot of the output has letters and numbers repeat in the same string like one had four f in it, is this good or a bad thing?

any how I would like any ideas to make this a better function.

Thanks for your insight into this.

Sincerely,
Christopher

<?php
function random( $uses = NULL, $type = NULL,  $length = 5 )
{
	switch( $type )
	{
	default:
		$pattern = "1234567890abcdefghijklmnopqrstuvwxyz";
	break;
	case 'u_letters':
		$pattern = "ABCDEFGHIJKLMONPQRSTUVWXYZABCDEFGHIJKLMONPQRSTUVWXYZ";
	break;
	case 'l_letters':
		$pattern = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz";
	break;
	case 'm_letters':
		$pattern = "aBcDeFgHiJkLmNoPqRsTuVwXyZAbCdEfGhIjKlMnOpQrStUvWxYz";
	break;
	case 'numbers':
		$pattern = "12345678901234567890123456789012345678901234567890";
	break;
	}


	switch($uses)
	{
	default:
		$default = '';
		for($i=0; $i < $length; $i++)
		{
		$default .= $pattern{mt_rand(0,35)};
		}
		return( $default );

	break;
	case 'user_id':
		include '/home/dev/www/lib/db_config_ro-dev.php';
		include '/home/dev/www/lib/db_conn-select.php';
		$user_id = 'U-';
			do
			{
		//	$user_id = 'U-' . sprintf("%'09d", mt_rand(1, 999999999));
			for($i=0; $i < $length; $i++)
			{
			$user_id .= $pattern{mt_rand(1, 35)};
			}
			$query_s = "SELECT user_id FROM access_credentials WHERE user_id = '$user_id'";
			$result_s = mysql_query($query_s) OR die("Sorry, unable to select record: " . mysql_error());
			}
			while(mysql_num_rows($result_s) != 0); // don't do again if = 0
		return( $user_id );
	break;
	case 'record_id':	
		include '/home/dev/www/lib/db_config_ro-dev.php';
		include '/home/dev/www/lib/db_conn-select.php';
		$record_id = 'R-';
			do
			{
		//	$record_id = 'R-' . sprintf("%'09d", mt_rand(1, 999999999));

			for($i=0; $i < $length; $i++)
			{
			$record_id .= $pattern{mt_rand(1,35)};
			}

			$query_s = "SELECT record_id FROM record_status WHERE record_id = '$record_id'";
			$result_s = mysql_query($query_s) OR die("Sorry, unable to select record: " . mysql_error());
			}
			while(mysql_num_rows($result_s) != 0); // don't do again if = 0
		return( $record_id );
	break;
	}
//	}
}
?>

Weedpacket

Two problems: one technical, one conceptual.

The technical problem is that you're only selecting characters from $pattern between the 0th and the 35th inclusive, without
considering how many characters $pattern actually has. So the pattern strings in your switch statement might as well be

1234567890abcdefghijklmnopqrstuvwxyz
AABBCCDDEEFFGGHHIIJJKLMONPQRSTUVWXYZ
aabbccddeeffgghhiijjklmnopqrstuvwxyz
ABCDEFGHIJLNPRTVXZabcdefghijkmoqsuwy
000111122223333444455556666777888999

Where I've sorted the characters so that you can see how some are more likely to appear than others (in lower case strings, 'f' is twice as likely to appear as 'o', for example).

That part of the problem can be fixed by replacing "35" with the length of the pattern (minus 1). Note that some of the calls to mt_rand() have '1' as the lower bound, meaning the 0th character would have no chance of being selected.

The conceptual problem is that you're trying to get this function to do everything. Not only is it generating a random string AND using it (itself a dodgy-sounding idea), but tries to anticipate every possible use of it as well: ASSUMING you have a specific database schema; ASSUMING you're only ever going to use this with MySQL; ASSUMING you want the function to kill the script if the query fails; and ASSUMING that you're only interested in user or record IDs (why not simply use sequential integers?), then any function behaviour other than the default "return the string" may be useful. That's a lot of assumptions. Any time any of those things changes you'll either have to change this function as well, or write it again elsewhere, or use its default behaviour. The last case can be used anywhere: you can leave the user ID and record ID "uses" where they should be - in your code for managing users and records.

Functions shouldn't have to be told what they're being used for. In Soviet Russia, functions use YOU!

So I just figured I'd whip up a cheap-n-nasty random string generator just as an illustration of a function that just generates random strings. To make it a bit more interesting I threw in some switches
to allow different mixes of characters. This code has never been run, let alone tested.

function random_string($length=5, $use_lower=true, $use_upper=true, $use_digit=true, $use_punct=false)
{
	if($length<=0)
	{
		trigger_error('Invalid length for random string', E_USER_WARNING);
		return false;
	}

$chars = '';
if($use_lower) $chars .= 'qwertyuioplkjhgfdsazxcvbnm';
if($use_upper) $chars .= 'QAZXSWEDCVFRTGBNHYUJMKIOLP';
if($use_digit) $chars .= '0147896325';
if($use_punct) $chars .= '!@#$%^&*=+?';
if($chars == '')
{
	trigger_error('No character ranges to generate string from', E_USER_WARNING);
	return false;
}

$string = '';
$maxchar = strlen($chars)-1;
for($i=0; $i<$length; $i++)
{
	$string .= $chars[mt_rand(0,$maxchar)];
}
return $string;
}

spiritssight

I am trying to keep that function only do one thing and only one thing each!!!

I do however forget 🙁

Thanks for the reminder and by the way I like your code better then mine even when it comes to making mine only do the one thing

Thanks,
Christopher

PS: I going to post another function I am trying to make and I am sure I am going over board with that, if you don't mind could you take a look Weedpacket

spiritssight

Weedpacket,

is there a way to make the numbers be more random, this is what I got when I hit refresh 5 times:
218143722
553321074
015021611
631642522
430675664
see how most of them have at less one number that is in it 3, is this a good or bad thing?

Thanks again!
Christopher

laserlight

see how most of them have at less one number that is in it 3, is this a good or bad thing?

That's fine, if I understand what you are trying to say.

Weedpacket

I'd expect a 9-digit number generated from a uniform distribution drawn from the digits [0..9] to contain a '3' about 61% (1-(9/10)⁹⁾ of the time in the long run (or three out of five samples; and no, five samples is not "in the long run").

I'd also point out that most of those numbers contain a '6', a '7', a '0', a '5', a '1', a '2', and a '4' as well. But then, there are 45 digits in that table, and only ten of them can be distinct.

spiritssight

Trying to understand all of this, I think I understand it somewhat :-)

Thanks for the fed back!

Sincerely,
Chrisopher

Horizon88

Weedpacket wrote:
I'd expect a 9-digit number generated from a uniform distribution drawn from the digits [0..9] to contain a '3' about 61% (1-(9/10)⁹⁾ of the time in the long run (or three out of five samples; and no, five samples is not "in the long run").

I'd also point out that most of those numbers contain a '6', a '7', a '0', a '5', a '1', a '2', and a '4' as well. But then, there are 45 digits in that table, and only ten of them can be distinct.

I love you so much Weedpacket. <3

NogDog

It's the old "random coin-flip" thing: the coin never remembers what the previous flip was, so it's always a 50/50 chance it will come up heads, even if the last 3 tosses where all heads. Therefore, "real" random sequences tend to have clumps that do not "look" random; and over the long run, the bigger your sample, the more likely you are to find bigger "clumps" while at the same time the totals are more likely to approach the expected statistical averages. But there's still a possibility that, say, 10 truly 50/50 random coin flips could all come up heads (approximately 0.09% if you're thinking about placing any bets on it happening)