MyBB Community Forums

Full Version: More robust random_str function
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Pages: 1 2
Hi i would suggest a more robust random_str function,

i saw your implementation and sincerely i don't like the fact that you insert the character into an array.

I wrote one that i think it is better as it takes char directly from ASCII table instead of an array.

function random_str($len = 8)
{
	$retn;
	for($i = 0; $i < $len; $i++)
	{
		$type = mt_rand(1,3);
		//1: Upper Char (A-Z); 2: Lower Char(a-z); 3: numeric char(0-9)
		switch($type)
		{
			case 1:
				$retn .= chr(mt_rand(0x41, 0x5A));
				break;
			case 2:
				$retn .= chr(mt_rand(0x61, 0x7A));
				break;
			case 3:
				$retn .= chr(mt_rand(0x30, 0x39));
				break;
		}
	}
	return $retn;
}

Also you should use mt_rand instead of rand, as it is faster (i tested Wink)
100% agree, what with the source of 1.4? something has change in that code?
flash.tato Wrote:Hi i would suggest a more robust random_str function,

i saw your implementation and sincerely i don't like the fact that you insert the character into an array.

Give me a better reason other than you don't like it, because that is not good enough.

Also, we already use mt_rand in the 1.4 code.
Because PHP has to store that array in memory, and also if it isn't a great amount i think we should optimize at the max. Smile

And also why reinventing the wheel? If we have a known standard (ASCII) we could make use of it without re-declaring the characters to be used and also we consume less memory. Wink

But the main reason is that from my test i find my function more faster than the one built in MyBB. (But i didn't test with mt_rand)

[Image: testsnm4.png]

P.S. Sorry if my english sounds bad
Your function makes it hard for the average programmer to allocate other characters or remove characters we don't want in the future.

Besides, running it 10 times is not a good comparison. You should think along the lines of 10,000.

And last but not least, the time comparison difference is too small to warrant it necessary at this stage. Might I also mention that this function is called on certain pages which are run very little compared to something like the index page
I didn't keep in mind the readability of the code and the fact that it is hard to edit the set of characters to use.

I compared them in very little pages, here are the codes:
<?PHP
function random_str($length="8")
{
	$set = array("a","A","b","B","c","C","d","D","e","E","f","F","g","G","h","H","i","I","j","J","k","K","l","L","m","M","n","N","o","O","p","P","q","Q","r","R","s","S","t","T","u","U","v","V","w","W","x","X","y","Y","z","Z","1","2","3","4","5","6","7","8","9");
	$str;
	for($i = 1; $i <= $length; $i++)
	{
		$ch = rand(0, count($set)-1);
		$str .= $set[$ch];
	}
	return $str;
}
$start = microtime();
echo random_str(10);
$end = microtime();
echo "\n";
echo ($end - $start);
<?PHP
function random_str($len = 8)
{
    $retn;
    for($i = 0; $i < $len; $i++)
    {
        $type = mt_rand(1,3);
        switch($type)
        {
            case 1:
                $retn .= chr(mt_rand(0x41, 0x5A));
                break;
            case 2:
                $retn .= chr(mt_rand(0x61, 0x7A));
                break;
            case 3:
                $retn .= chr(mt_rand(0x30, 0x39));
                break;
        }
    }
    return $retn;
}
$start = microtime();
echo random_str(10);
$end = microtime();
echo "\n";
echo ($end - $start);

Well, i tried to give my contribute to MyBB Wink, mine is only a suggestion so i will not care if it will be implemented or not. Wink

Best Regards
@flash.tato,
thanks for your try, some of the suggestions accepted and implemented into mybb by the mybb team and some other not,
there are probably some considerations that the they take into account.
since mybb is an open source webware, the readability of the source is one of the consideration, as mentioned by tiki'.

Personally, I'm going to use your solution in my copy of mybb but I think the main mybb releases should stay more readable.

anyway, thanks for sharing your idea with others ^_^
And if you want an even nicer looking one:

function createRandomPassword() { 

    $chars = "abcdefghijkmnopqrstuvwxyz023456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; 
    srand((double)microtime()*1000000); 
    $i = 0; 
    $pass = '' ; 

    while ($i <= 7) { 
        $num = rand() % 33; 
        $tmp = substr($chars, $num, 1); 
        $pass = $pass . $tmp; 
        $i++; 
    } 

    return $pass; 

} 


There is ALWAYS different ways to do code...if the wheel ain't broke though...why fix it? I think mybb staff has better things to do.
Flash.tato's way is allot easier then having to deal with A a B b C c and so on. Of course it's the developers idea, but Tato may have a point.

labrocca Wrote:And if you want an even nicer looking one:

function createRandomPassword() { 

    $chars = "abcdefghijkmnopqrstuvwxyz023456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; 
    srand((double)microtime()*1000000); 

You might want to separate those letters, it might not work the way you have it. (to my knowledge.)
function createRandomPassword() { 

    $chars = "abcdefghijkmnopqrstuvwxyz023456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; 
    srand((double)microtime()*1000000); 
    $i = 0; 
    $pass = '' ; 

    while ($i <= 7) { 
        $num = rand() % 33; 
        $pass .= $chars{$i}; 
        $i++; 
    } 

    return $pass; 

} 

yeh, there could be 10 or 20 different ways I could do it. All of them have their cons and pluses, but what we have works fine
Pages: 1 2