View Single Post
Old 06-11-2008, 12:15 PM   #8 (permalink)
EyeDentify
The Contributor
 
EyeDentify's Avatar
 
Join Date: Nov 2007
Location: Sweden
Posts: 91
Thanks: 11
EyeDentify is on a distinguished road
Default

Quote:
Originally Posted by Salathe View Post
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.
Your right. There are plenty of things that could be done in another more effective way.

This how ever were just a short bare bone code snippet that anyone could base there random string functions upon if they wish. I wasn't out to publish a complete solution, just another way to do things. Maybe not optimal but it works.

The reason i used the approach with the range() to get chars into arrays and then merge them was to be able to add other ranges() or manually constructed arrays with chars to mix with.

And maybe the shuffle() isīnt necessary.
But advantage you say ?, it makes the array more random.
And when you later down the code pick random chars.
Well that's just more random candy for ya :)

I little sharing if you like.

i admit that i should of course have made the function return the string and not echo().

This optimal performance hysteria goes to far some times.
Hardware these days can do a lot more then we tend to give it credit for. And of course itīs also a question of how often one use the said function.

To conclude. I appreciate your insight and shall remember it in future adventures.

/Eye
__________________
Of course the whole point of a doomsday machine, would have been lost if you keep it a secret.
EyeDentify is offline  
Reply With Quote