+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: General (https://forum.codeigniter.com/forumdisplay.php?fid=1)
+--- Forum: Lounge (https://forum.codeigniter.com/forumdisplay.php?fid=3)
+--- Thread: Does this small piece of code seem ok? (/showthread.php?tid=63439)
Does this small piece of code seem ok? - PaulD - 10-30-2015
Hi,
I am not sure why but I am uncomfortable with this piece of code.
All I am doing after this loop is inserting the generated $code value (not for crypotgraphic purposes) into a unique column, in a later query. So to make sure the code is unique I am doing this:
PHP Code:
$loop = TRUE;
while($loop)
{
$code = random_string('alnum', 16);
$this->db->where('guest_code', $code);
$check = $this->db->count_all_results('guests');
if ($check == 0)
{
$loop = FALSE;
}
}
But I am uncomfortable with the loop though. Is this a sensible way to do this?
It is working and I have moved on for now but it is bugging me because of the potential for an infinite loop. Is this a bad thing to do or am I being silly.
Any advice would be welcomed,
Best wishes,
Paul.
RE: Does this small piece of code seem ok? - Martin7483 - 10-30-2015
The only potential for an infinite loop is if every possible combination that random_string can return is already stored in your table.
You could add a counter and have the loop exit when it hits 1000 for example.
RE: Does this small piece of code seem ok? - PaulD - 10-30-2015
Thanks for the reply.
I will do that and then add an error message particular for that failure. "You could not add that guest as we were unable to generate a unique code for them" or similar.
I still don't like it though (the loop above that is)
Best wishes,
Paul.
RE: Does this small piece of code seem ok? - Martin7483 - 10-30-2015
For generating a unique code you will need to check if it already exists, and if it does regenerate.
If you don't want the loop you could create this as an iterative method.
PHP Code:
public function generate_code() {
$code = random_string('alnum', 16);
$this->db->where('guest_code', $code);
$check = $this->db->count_all_results('guests');
if ( $check >= 1 ) {
return $this->generate_code();
}
return $code;
}
RE: Does this small piece of code seem ok? - PaulD - 10-30-2015
That is an awesome solution!!!!
At first I was concerned that it was worse as it might hit the memory limit as php has to remember all the deeper and deeper levels of functions within functions, but that is not how PHP works in this case (I think). It seems that it assigns the function to the variable, returns that, 'level' closed, then re-evaluates it.
That lead me to this: http://www.phptherightway.com/pages/Functional-Programming.html which was interesting too but I really must get back to what I was doing :-)
I am definitely going to use that function - thank you!
Paul.
RE: Does this small piece of code seem ok? - Narf - 10-30-2015
Code:
$guest_code = bin2hex(pack('N', microtime(TRUE))).bin2hex(get_instance()->security->get_random_bytes(4));Almost guaranteed uniqueness and better randomness.
RE: Does this small piece of code seem ok? - ignitedcms - 10-30-2015
With a random 16 alphanumeric string, I don't even check if it is duplicated. The chances are too low. If you're still worried increase the string length.
RE: Does this small piece of code seem ok? - Martin7483 - 10-30-2015
(10-30-2015, 10:10 AM)iamthwee Wrote: With a random 16 alphanumeric string, I don't even check if it is duplicated. The chances are too low. If you're still worried increase the string length.
Just because the changes are low does not mean you should ignore the fact it CAN and at some point WILL happen. You can't cover every single scenario in programming, but to ignore something like this that is quite easy to avoid is just bad practice. But that is my opinion
RE: Does this small piece of code seem ok? - ignitedcms - 10-30-2015
Quote:Just because the changes are low does not mean you should ignore the fact it CAN and at some point WILL happen
When you say will happen, can you determine the probability of a random 16 alphanumeric collision.
http://betterexplained.com/articles/the-quick-guide-to-guids/
Interesting, but I'm sure a 16 digit number is quite safe for low volume visitors anyway.
RE: Does this small piece of code seem ok? - Martin7483 - 10-30-2015
I said at SOME POINT. No way of knowing when or where, but fact remains that it CAN happen. If something can happen that could cause an error that is avoidable, I choose to avoid it.