• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Does this small piece of code seem ok?

#1
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.
Reply

#2
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.
Reply

#3
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.
Reply

#4
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 >= ) {
 
      return $this->generate_code();
 
   }
 
   return $code;

Reply

#5
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/Func...mming.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.
Reply

#6
Code:
$guest_code = bin2hex(pack('N', microtime(TRUE))).bin2hex(get_instance()->security->get_random_bytes(4));

Almost guaranteed uniqueness and better randomness.
Reply

#7
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.
IgnitedCMS Pro
IgnitedCMS
Practical guide to IgnitedCMS - Book coming soon
Reply

#8
(10-30-2015, 11: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 Smile
Reply

#9
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. Wink

http://betterexplained.com/articles/the-...-to-guids/

Interesting, but I'm sure a 16 digit number is quite safe for low volume visitors anyway.
IgnitedCMS Pro
IgnitedCMS
Practical guide to IgnitedCMS - Book coming soon
Reply

#10
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.
Reply


Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  


Users browsing this thread:
1 Guest(s)


  Theme © 2014 iAndrew  
Powered By MyBB, © 2002-2017 MyBB Group.