patternphpMinor
Auction function for a game
Viewed 0 times
auctiongamefunctionfor
Problem
This is an auction function for a game where you can bid on houses on the website.
You set a limit, and if posts limit below current bet or below your limit, it will raise the bid from current plus 1, until the limit is met. If your bid is over a player's limit, it updates it with your limit.
Let's say the current limit for house #55 is 500 and was submitted by player CykaBlyat and then Andrej comes and submit a limit of 800 it will update latest limit plus one (so bid will be 501) and keep raising with 1 until someone submit a limit higher than 800. And so on.
Is it safe? Is there something I have missed? Something I can do better? A cleaner way?
```
$id = isset($_GET['id']) ? intval($_GET['id']) : 0;
// Fetch some info about the house
$house_query = "SELECT h.id AS houseID, h.name AS houseName, h.rent, h.size, h.numofdoors, h.beds, h.owner, h.paid, p.name
FROM houses h
LEFT JOIN players p ON p.id = h.owner
WHERE h.id = ?";
$stmt = $db->prepare($house_query);
$stmt->execute(array($id));
$row = $stmt->fetch();
// Is house on auction
$auctions = "SELECT *
FROM house_auctions ha
LEFT JOIN players p ON p.id = ha.player_id
WHERE ha.house_id = $id";
$stmt = $db->prepare($auctions);
$stmt->execute(array($id));
$auction = $stmt->fetch();
if (count($_POST) > 0) {
$limit = trim($_POST['limit']);
$char = trim($_POST['character']);
// Get balance
$balance = $db->prepare("SELECT balance FROM players WHERE id = ?");
$balance->execute(array($char));
if ($limit $balance->fetchColumn())
$errors[] = "You do not have enough gold.";
else if ($limit $auction['limit']) {
$bid = $auction['limit'] + 1;
$limit2 = $limit;
$pid = $char;
} else {
$bid = $auction['limit'] + $limit + 1;
$limit2 = $limit;
$pid = $char;
}
// Let's update!
if (empty($errors)) {
echo "NO ERRORS =)";
$stmt = $db->prepare("UPDATE house_auctions SET house_id = :house_id, player_id = :player_id,
You set a limit, and if posts limit below current bet or below your limit, it will raise the bid from current plus 1, until the limit is met. If your bid is over a player's limit, it updates it with your limit.
Let's say the current limit for house #55 is 500 and was submitted by player CykaBlyat and then Andrej comes and submit a limit of 800 it will update latest limit plus one (so bid will be 501) and keep raising with 1 until someone submit a limit higher than 800. And so on.
Is it safe? Is there something I have missed? Something I can do better? A cleaner way?
```
$id = isset($_GET['id']) ? intval($_GET['id']) : 0;
// Fetch some info about the house
$house_query = "SELECT h.id AS houseID, h.name AS houseName, h.rent, h.size, h.numofdoors, h.beds, h.owner, h.paid, p.name
FROM houses h
LEFT JOIN players p ON p.id = h.owner
WHERE h.id = ?";
$stmt = $db->prepare($house_query);
$stmt->execute(array($id));
$row = $stmt->fetch();
// Is house on auction
$auctions = "SELECT *
FROM house_auctions ha
LEFT JOIN players p ON p.id = ha.player_id
WHERE ha.house_id = $id";
$stmt = $db->prepare($auctions);
$stmt->execute(array($id));
$auction = $stmt->fetch();
if (count($_POST) > 0) {
$limit = trim($_POST['limit']);
$char = trim($_POST['character']);
// Get balance
$balance = $db->prepare("SELECT balance FROM players WHERE id = ?");
$balance->execute(array($char));
if ($limit $balance->fetchColumn())
$errors[] = "You do not have enough gold.";
else if ($limit $auction['limit']) {
$bid = $auction['limit'] + 1;
$limit2 = $limit;
$pid = $char;
} else {
$bid = $auction['limit'] + $limit + 1;
$limit2 = $limit;
$pid = $char;
}
// Let's update!
if (empty($errors)) {
echo "NO ERRORS =)";
$stmt = $db->prepare("UPDATE house_auctions SET house_id = :house_id, player_id = :player_id,
Solution
Use placeholder there:
You should separate presentation from your logic. It will be better: read all $_POST outside the function and pass in arguments. Maybe you wanna later to play your game via AJAX, or something else, and you won't need $_POST anymore, but you still need the logic.
Due the same approach, returning status and errors from function is a better way, then just prints them out. You can return 2 values from function -- status and errors array.
Then do something like this:
Or you can just return an array, and check length of it. If array contain some errors, print them out.
Rename your variables.
In your code it isn't clear enough. Especially
WHERE ha.house_id = $id"; Placeholder is better due to security approach.if (count($_POST) > 0) {
$limit = trim($_POST['limit']);
$char = trim($_POST['character']);You should separate presentation from your logic. It will be better: read all $_POST outside the function and pass in arguments. Maybe you wanna later to play your game via AJAX, or something else, and you won't need $_POST anymore, but you still need the logic.
Due the same approach, returning status and errors from function is a better way, then just prints them out. You can return 2 values from function -- status and errors array.
Then do something like this:
list($status, $errors) = your_function(...);
if (!$status) {
//printing errors
}Or you can just return an array, and check length of it. If array contain some errors, print them out.
Rename your variables.
char to characterpid to playerIdlimit2 to newLimit? Or what it means?In your code it isn't clear enough. Especially
limit2.Code Snippets
if (count($_POST) > 0) {
$limit = trim($_POST['limit']);
$char = trim($_POST['character']);list($status, $errors) = your_function(...);
if (!$status) {
//printing errors
}Context
StackExchange Code Review Q#68319, answer score: 2
Revisions (0)
No revisions yet.