Recursive function to generate a unique code

@MichalOravec Oh sorry yes it was obvious that the code wasn't executed.

So I have to pass the parameter to the generate() function.

Hmmm ... another question : is there any other way to check if a code already exists between those two ways ?

• retrieve all codes in a collection and check if the new generated code exists or not inside the collection

• check each time in the database for each generated code

What would be the best way ?

And is it a good idea to send multiple queries to the database inside a recursive function ?

@vincent15000 I would most likely do something like this:

class UniqueGuestCodeGeneration
{
    public function generate(int $length)
    {
        do {
            $code = Str::random($length);
        } while (Event::where('guest_code', $code)->exists());

        return $code;
    }
}

or

class UniqueGuestCodeGeneration
{
    public function generate(int $length)
    {
        $codes = Event::pluck('guest_code');

        do {
            $code = Str::random($length);
        } while ($codes->contains($code));

        return $code;
    }
}

If I had to choose, I'll stick with the first option.

@MichalOravec I don't use Str::random to be able to avoid some unwanted letters.

Why the first option ? Somebody in my team just told me that it wasn't a good idea to send queries to the database inside a recursive function. I really don't understand why.

But for me it's also not a good idea to load all codes in a collection. Or no matter ?

@vincent15000 It depends on the length, but it will probably be more likely that it will generate a unique code on the first try than to keep an ever-growing list of unique codes.

@MichalOravec Ok thank you ;).

@vincent15000 The reason it’s generally not a good idea to put database calls inside recursive functions is that they are likely to get called with each recursion. If it’s the type of function that ends up recursing 150 times before returning, you get 150 database calls, which is going to slow everything down an awful lot. Some recursive functions are meant to work this way, to call themselves many many times.

In this case, though, as Michal says, there will almost certainly only ever be one level of recursion and thus one database call, since generating two already existing codes in a row is extremely unlikely unless your codes are only two or three characters long. Even with just three characters, there are 32,768 possible code combinations, and with four characters, you have over a million already. If your codes are a reasonable length, like 16 characters, there are 1,208,925,800,000,000,000,000,000 possible combinations, and hitting even just one existing combination out of that is humongously unlikely – hitting two in a row is about as likely as picking up a cucumber and having it turn into the Sun.

(Don’t forget that your original code also has the database call inside the recursion.)

@wafto That's effectively another good idea ;).

One function generates a randomized code and the other one checks if the code exists or not.

@vincent15000 Yeap and you can make it better putting the chars on a constant and even the length of the chars, I think this code is easy to read.

@wafto But the code suggested by @michaloravec is much shorter and also easy to read ;).

@Snapey It's an existing application and for now the code has a 5 characters length, that's not the case for a uuid. Furthermore it's a code that users have to type to access to a form without having to log in.