PDO insert from check boxes

I've seen so many different ways of doing this all seem very complex and all seem to be done in MySQL. I'm using PDO in my project so I've had a go myself to see what I could come up with.

• Is this safe?

• Is there something I've missed a better way of doing it?

//build query to insert users_contracts
$sth = "INSERT INTO `users_contracts` (users_id,contracts_id) VALUES (:user,:contract)";
$q = $conn->prepare($sth);

//take each item in the contract array from _POST and bind to the above INSERT query
foreach($contract as $contract => $contract_id)
{
    try {
       $q->execute(array(':user'=>$last_id,':contract'=>$contract_id));
    } catch (PDOException $e) {
       $errors[] = array(
          'message' => "Could not insert \"$contract\", \"$contract_id\".",
          'error' => $e
          );
    }    
}

Its intended purpose is to allow the admin user to assign contracts to users via a checkbox list.

There are a couple of minor things I'd suggest you look into:

• Names matter: you're assigning the query string to a variable called $sth, and assinging the prepared statement to a variable called $q. To me, that looks wrong, and it should be the other way around. A variable called $q or $query should be a query string, and a variable called $sth or $stmt should be a prepared statement.

• Transactions: if, somewhere down the line, one of the INSERT queries failed, I wouldn't carry on. To me, that smells of bad/invalid/malicious data, and I'd want to stop inserting and even undo whatever inserts that were already executed. By starting a transaction before the loop, and committing it at the end, you can do this easily

• Because you are re-using a statement several times (which is good), you might want to add a closeCursor call. The MySQL drivers don't really care about this, but some other drivers do. Depending on what DB you're using, your code might behave differently if you leave it out.

Basically, using bindParam as Pinoniq suggested is a good idea. However, make sure $last_id stays valid. It's not being altered in your loop, so make sure the code between bindParam and the execute call doesn't do anything untoward with that variable.

All things aside, your code could be made to look something like this:

$query = 'INSERT INTO users_contracts (users_id,contracts_id)
    VALUES (:user,:contract)';
try
{//begin try-block here
    $conn->beginTransaction();//start the transaction
    $stmt = $conn->prepare($query);
    $stmt->bindParam('user', $lastId);
    $stmt->bindParam('contract', $contractId);
    foreach($contract as $contract => $contractId)
    {
        $stmt->execute();
        $stmt->closeCursor();//commit();//everything went smoothly, save inserts
}
catch (PDOException $e)
{
    $conn->rollBack();//undo changes!
    //handle error
    printf(
        '%d - %s insert failed: (%d) %s',
        $lastId,
        $contractId,
        $e->getCode(),
        $e->getMessage()
    );
}

StackExchange Code Review Q#64373, answer score: 5