patternphpMinor
Code Reiview for an PHP PDO Queries? is there a better way to do it?
Viewed 0 times
reiviewphpwaybetterforcodequeriestherepdo
Problem
so this is my code for a equipping an item in my game dev't:
try {
$db = getConnection();
$db->beginTransaction();
$sql_chara_gold = $db->query("SELECT chara_gold FROM chara WHERE chara_id = $shop->chara_id");
$get_chara_gold = $sql_chara_gold->fetchColumn();
$sql_shop_price = $db->query("SELECT item_price FROM shop WHERE item_id = $shop->item_id");
$get_shop_price = $sql_shop_price->fetchColumn();
if($get_chara_gold >= $get_shop_price){
$sql_put_item = "INSERT INTO bag(chara_id, item_id, item_qty)
VALUES(:id,:item_id,1)
ON DUPLICATE KEY
UPDATE item_qty = item_qty +1";
$sql_set_gold = "UPDATE chara ch
INNER JOIN item it
ON it.item_id = :item_id
SET ch.chara_gold = ch.chara_gold - it.item_price
WHERE chara_id = :id";
$sql_set_qty = "UPDATE shop
SET item_qty = item_qty - 1
WHERE item_id = :item_id";
$stmt = $db->prepare($sql_put_item);
$stmt->bindParam("id", $shop->chara_id);
$stmt->bindParam("item_id", $shop->item_id);
$stmt->execute();
$stmt = $db->prepare($sql_set_gold);
$stmt->bindParam("id", $shop->chara_id);
$stmt->bindParam("item_id", $shop->item_id);
$stmt->execute();
$stmt = $db->prepare($sql_set_qty);
$stmt->bindParam("item_id", $shop->item_id);
$stmt->execute();
$db->commit();
$db = null;
echo json_encode($shop);
}else{
echo '{"error":{"text":'.json_encode($get_shop_price).'}}';
}Solution
I don't know if I'd use ORM for something so simple, but it may be something to look into. At the very least, you should get in the practice of escaping query parameters to avoid SQL injection. I often use Aura SQL for this, because it's dead simple: https://github.com/auraphp/Aura.Sql
An example query using Aura would look like this:
Aura will automatically escape any bound parameters, like :id, so you don't have to worry about SQL injection. If you don't use Aura, at least use PDO::quote.
Funds transfer is the textbook example of why you need transactions to meet the ACID requirements, so it's good that you're using one, but I don't see your rollback. I assume this is in your catch clause which wasn't included.
An example query using Aura would look like this:
$user = $db->fetchOne('SELECT chara_gold FROM chara WHERE chara_id = :id', [
'id' => $shop->chara_id
]);Aura will automatically escape any bound parameters, like :id, so you don't have to worry about SQL injection. If you don't use Aura, at least use PDO::quote.
Funds transfer is the textbook example of why you need transactions to meet the ACID requirements, so it's good that you're using one, but I don't see your rollback. I assume this is in your catch clause which wasn't included.
Code Snippets
$user = $db->fetchOne('SELECT chara_gold FROM chara WHERE chara_id = :id', [
'id' => $shop->chara_id
]);Context
StackExchange Code Review Q#25861, answer score: 2
Revisions (0)
No revisions yet.