View Single Post
Old 06-11-2008, 11:31 AM   #6 (permalink)
Salathe
Moderateur
RegEx Guru PHP Guru Top Contributor Advanced Programmer 
 
Salathe's Avatar
 
Join Date: Apr 2007
Posts: 710
Thanks: 2
Salathe is on a distinguished road
Default

I would advise making the function somewhat more efficient. There are a number of things which, unless you can justify their inclusion or the way they've been done, I feel are useless or could be better implemented.

First, the method of constructing the array of characters which can be used in the generated string. You make four function calls (range, array_merge) simply to construct a single array of characters a-zA-Z0-9. This array will always be the same upon every function call so why not just hard-code?

Next, the for/shuffle lines. What advantage does shuffling the array 5, 10, 50 times have over doing it once? Does it make the array more random? I'd say this whole section is unnecessary since you're later choosing array keys at random anyway (with mt_rand).

I'd also suggest returning the resulting string rather than echoing out each character as the function will be much more versatile (who says you need to output the string?).

Sketch, thanks for the note about the off-by-one error on $len however you introduce one yourself by changing the final for loop to use intval($passLen-1) (the resulting random string will always be one character less than $passLen).

I think this function could well be refactored into something maybe 10 times faster to run and equally so easy to glance at and understand what's going on.
__________________
Salathe is offline  
Reply With Quote
The Following User Says Thank You to Salathe For This Useful Post:
EyeDentify (06-11-2008)