Generating code and checking against DB

[crowden]

Hi all,

I'm working on a db entry script that generates a 6-character code, checks to see if that code is in the db already, and re-generates the code if it's found.

This is probably pretty simple, and I think I have a working solution but I'd like to have some other eyes look at it.

//$code = strtoupper(genCode(6));
        $code = 'ABCDEF';
        $query = "SELECT clientcode ";
        $query .= "FROM clients ";
        $rows = $dbh->query($query);
        foreach($rows as $row)
        {
            while(in_array($code, $row))
            {
                $code = strtoupper(genCode(6));
            }
        } 

As you can see, I hard coded a value into the $code var that I know is in the DB. Does this seem like the most efficient way, or am I missing out on a better way?

Thanks for any advice.

[AnthonySterling]

Try...

<?php
$iCode = 67344;
$rResult = mysql_query(
    sprintf(
        'SELECT code FROM table WHERE code = %d LIMIT 1',
        $iCode
    )
);
if(0 !== mysql_num_rows($rResult))
{
    #regenerate
}
else
{
    #accept?
}
?>

[crmalibu]

One thing to keep in mind is the possibility of a race condition. You might do the search, and find the code is available. But in the tiny window of time between the search, and the actual insert, it's possible another script already inserted the same code. Now you might have a duplicate.

If this might be an issue, you can:
Let the database enforce uniqueness. Declare a unique constraint on the column. It will not fail you due to a race condition.

But, you still need to detect the collision so you can regenerate.
The easiest way imo is to just insert it, and check for the duplicate key error code. If it happens, regenerate and try again.

Another way is to lock the table. Once you have the lock, you can safely do a select to check, and follow up with the insert, then unlock. I think this is better if you have more than one unique constraint on the table, because you can easily see which column was the conflict. With the other method, you need to resort to parsing the error message to find out which column, which I think is real ugly.

[crowden]

Thanks for the replies guys.

@SBK - What happens if when you regenerate, the code regenerated is already in the DB? You don't have a check on the newly generated code.

@crmalibu - I do have a unique constraint on that column, but I'm not inserting it at that point. I'm wanting to generate it so that it's available to view before any info is input in a form to db entry. (that was difficult to word )

But you have me thinking maybe I should change how I'm doing that, especially if inserting the code at that point would be faster than querying and running it through a loop.

[AnthonySterling]

@SBK - What happens if when you regenerate, the code regenerated is already in the DB? You don't have a check on the newly generated code.

Just repeat I guess, or pop it into a while loop. Although the race condition crmalibu pointed out is worrying.

What's the background to this 'code', I'd like to mull over the problem a little.

[crowden]

Background?

Well, I need a six character, alphanumeric code that's unique to each client.
When I go to add a client, I want that code generated upon page load so it can be seen as the client info is being filled in to the form.

[AnthonySterling]

Is there a reason you cannot use an auto-incremental field in the database for this? It would eliminate the race condition and the logic would fit a little better I would say.

Maybe something similar to...

<?php
$oDatabase->startTransaction();
$iId = $oDatabase->execute("INSERT INTO clients (status)VALUES('pending')")->getLastId();
if(formCompleted())
{
    $oDatabase->execute(sprintf("UPDATE clients SET ..... WHERE id = %d", $iId));
    $oDatabase->commit();
}
else
{
    $oDatabase->rollBack();
}
?>

Thoughts?

[crowden]

I like that, but how does the 6 character code get generated? I like the random generator as I usually get something with more fluff ('M36YW7' or '03DRCP4' as opposed to '000000' or '000001').

And yes, that matters in my case.

If the auto-increment can be a randomly generated, 6-char code, then I'm all for it.

[Jake Arkinstall]

@crmalibu - I do have a unique constraint on that column, but I'm not inserting it at that point. I'm wanting to generate it so that it's available to view before any info is input in a form to db entry. (that was difficult to word )

In that case, would it not be simpler to insert only that 6-letter code, and a flag for 'new'?

If it returns an error complaining of duplication, loop it. That way you'll always have 1 less query than your current solution.

Pass that 6-letter code onto the form, so when you insert it it's just a case of an update query - making sure that the update clause is that the code is equal AND 'new' is flagged - unflagging it in the query.

That 'new' flag should be enough to stop someone from (whether on purpose or by accident) overwriting a current record.

[crowden]

If it returns an error complaining of duplication, loop it. That way you'll always have 1 less query than your current solution.

Doesn't my solution only have 1 query? 2, actually if you count the form insert query.

If I loop it, wouldn't it be sending a query every time it happens to match (duplication)?

Following your suggestion, I'm not understanding the purpose of a 'new' flag. If the code isn't a duplicate, the insert is successful and can return the newly inserted ID. On $_POST submit, wouldn't you just update that ID? Where does the threat of overwriting a record come from?

[AnthonySterling]

Bah, I hate Monday mornings!

Well the only thing we can guarantee that would be unique at the time of record creation would be the id value returned by the database.

Bearing this in mind, I guess we could use that, what about...

<?php
function generateCode($iId)
{
    $aKey = range('A', 'J');
    $sCode = '';
    preg_match_all('~[0-9]~', sprintf('%06d', $iId), $aMatches);
    foreach(array_shift($aMatches) as $iValue)
    {
        $sCode .= $aKey[(int)$iValue];
    }
    return $sCode;
}

echo generateCode(1);        #AAAAAB
echo generateCode(000001);    #AAAAAB
echo generateCode(435355);    #EDFDFF
echo generateCode(756356);    #HFGDFG
echo generateCode(867453);    #IGHEFD
echo generateCode(9444);    #AAJEEE
echo generateCode(5389);    #AAFDIJ
?>

It's early, so you'll have to forgive the sloppy code.

[crowden]

I'm just sticking with the original plan for right now. It works well enough as I'm the only one using it

This is in an admin area/control panel to add clients so it's not going to be bogging anything down any time soon. I'll revisit it as needed.