I created a shopping cart, and whenever there is an item, I will increase the quantity. And if the item is not in the cart, I am pulling the information of the item from the database. However, the while loop keeps overwriting the SESSION variable instead of appending it. This is my code:
if (isset($_GET['bookId'])) {
$bookId = $_GET['bookId'];
if (isset($_SESSION['cartItems'])) {
echo "cart NOT empty";
$cartItems = $_SESSION['cartItems'];
// Loop through the cartItems array.
$found = 0;
foreach ($cartItems as &$cartItem) {
// If the book is already in the cart items
if ($cartItem['bookId'] == $_GET['bookId']) {
$cartItem['quantityInput'] += 1;
$found = 1;}
// If the book is not in the cart item, then append the book.
elseif ($found == 0) {
//add the book
echo "new book!";
// Connect to Database to get data. Output is $result
****THIS IS THE PART GOING WRONG!*******
while ($book = $result -> fetch_assoc()) {
echo "This part is not working well, IT OVERWRITES THE ARRAY!";
$cartItem = array('bookId'=>$book['bookId'], 'title'=>$book['title'], 'price'=>$book['price'], 'quantityInput'=>1);
$cartItems[$book['bookId']] = $cartItem;
echo $cartItems[$book['bookId']]['title'];
}
}
}
} else {
echo "cart empty";
$cartItems = array();
// Add the book into cart item
// This part I connect with DB to get data, output is $result
while ($book = $result -> fetch_assoc()) {
$cartItem = array('bookId'=>$book['bookId'], 'title'=>$book['title'], 'price'=>$book['price'], 'quantityInput'=>1);
$cartItems[$book['bookId']] = $cartItem;
}
}
$_SESSION['cartItems'] = $cartItems;
print_r($cartItems);
}
I'm confused because I have the key which is bookId
, and all of item has different id numbers. So it should not overwrite... What am I missing here? How can I fix this problem? Below is the output of the array when I print out the cart item at the end with print_r($cartItems);
( [2] => Array ( [bookId] => 1 [title] => The Royal Easter [price] => 14.99 [quantityInput] => 1 ) [1] => Array ( [bookId] => 1 [title] => The Royal Easter [price] => 14.99 [quantityInput] => 2 )
You have two big culprits in your code. The first one is a flawed control structure:
foreach ($cartItems as &$cartItem) {
if ($cartItem['bookId'] == $_GET['bookId']) {
$cartItem['quantityInput'] += 1;
$found = 1;
}
// If the book is not in the cart item, then append the book.
elseif ($found == 0) {
// add a whole new book
}
}
Since this is inside a loop, the elseif
portion will execute each time the id doesn't match, until the book is found. If the book was not present, it would execute for every element of the cart.
That's where the second culprit comes in. You pass the element as a reference in your loop, meaning any operation over that element would change it. Since you were doing exactly that in the elseif
branch by assigning it a new value ($cartItem = ...
), it would ultimately result in it being overwritten.
What you actually need to do is extract the contents of that branch outside the loop, because you only want it to run once - if the book was not found:
foreach ($cartItems as &$cartItem) {
// If the book is already in the cart items
if ($cartItem['bookId'] == $_GET['bookId']) {
$cartItem['quantityInput'] += 1;
$found = 1;
/* Adding a break here would stop the loop once the book was found.
It would be a logical addition if no special cases exist where
an item can be added twice independently (seems like that's what
you're going for, no duplicates in the cart). */
}
}
// If the book is not in the cart item, then append the book.
if ($found === 0) {
// everything that was in your elseif branch
}
This should solve your issue. But please consider the following improvements to your code...
This code block is repeated twice (both when the book is not found and when the cart is empty):
while ($book = $result -> fetch_assoc()) {
$cartItem = array('bookId'=>$book['bookId'], 'title'=>$book['title'], 'price'=>$book['price'], 'quantityInput'=>1);
$cartItems[$book['bookId']] = $cartItem;
}
The sensible thing would be to extract any duplicating code into a function and just call it multiple times. But in your case this duplication can be eliminated. You need the exact same operation in those two cases and you can have the same code execute it:
if (isset($_GET['bookId'])) {
$bookId = $_GET['bookId'];
$found = 0;
foreach ($_SESSION['cartItems'] ?? [] as $cartItem) {
if ($cartItem['bookId'] == $bookId) {
$_SESSION['cartItems'][$bookId]['quantityInput']++;
$found = 1;
break;
}
}
if ($found === 0) {
// your query code here
while ($book = $result->fetch_assoc()) {
$newCartItem = array_merge($book, ['quantityInput' => 1]);
$_SESSION['cartItems'][$book['bookId']] = $newCartItem;
}
}
}
Note several things about the refactored solution:
$_SESSION['cartItems'] ?? []
is the equivalent of isset($_SESSION['cartItems']) ? $_SESSION['cartItems'] : []
and will tell the loop to use an empty array if there is no cart, which will in turn just skip the loop and go straight to adding the new element.$cartItem
to $newCartItem
. I see it as a good practice not to reuse variable names, it can lead to unexpected results due to accidental reassignments.$newCartItem = array_merge($book, ['quantityInput' => 1]);
does the same thing as your code did with array('bookId'=>$book['bookId'], 'title'=>$book['title'], 'price'=>$book['price'], 'quantityInput'=>1)
- it uses the same keys and values of the book, just adding quantity to it.この記事はインターネットから収集されたものであり、転載の際にはソースを示してください。
侵害の場合は、連絡してください[email protected]
コメントを追加