Seperate the concerns
The first problem I see with the current implementation, is that it "magically" acts on the $_SESSION variable. By doing so, you limit your ability to modify the logic in the future since the implentation of the logic is so tightly soupled with how the data is being stored. You also run the risk of forgetting that you had a function somewhere else in the code that also altered the value of $_SESSION, thus making the behavior of removeProductFromBasket unpredictable and vulnerable to bugs.
To fix this issue, you need to decouple your function from the $_SESSION. You can achieve this by adding a parameter to the function signature that takes in the shopping cart as an array.
function removeProductFromBasket($cart, $itemID)
{
// Loop shopping cart
foreach($cart as $key => $productArr) {
// Check if product is already included
if ($productArr['productid'] == $itemID) {
echo $itemID;
$cart[$key]['quantity'] = 0;
}
}
return cart; // return the updated cart
}
By modifying the code in this way allows you to handle the $_SESSION elsewhere, maybe in a wrapper object or something of the sort without affecting the logic of the item removal of the shopping cart. It also allows you to more easily change how the information is persisted. Say you wanted to use Redis or some other data store, you would have less lines to modify to make the change.
Calling the function
The other problem I see with your current implementation is the way the function is called. From your sample code, I'm guessing that the full line ressembles something like this:
echo '<a href="'.removeProductFromBasket($productID).'">Remove this</a>
Since PHP is executed on the server side, what this is doing is calling the function removeProductFromBasket when the page gets loaded. This means that by the time the page is rendered and loaded on the client side, the item has already been removed, and the href will look like href="12345" du to the echo in your function.
Instead, you should be echoing a valid url to the item removal logic, with Item ID proerly concatenated as a parameter.
form.php
foreach($_SESSION['shoppingCart'] as $key => $item) {
echo '<a href="http://www.example.com/path/to/file.php?itemId='.$item['productid'].'">Remove this</a>
}
item_removal.php
<?php
// Make sure the session is started, $_GET is not empty and any other validation
function removeProductFromBasket($cart, $itemID)
{
// Loop shopping cart
foreach($cart as $key => $productArr) {
// Check if product is already included
if ($productArr['productid'] == $itemID) {
// the echo is no longer needed
$cart[$key]['quantity'] = 0;
}
}
return cart; // return the updated cart
}
$_SESSION['shoppingcart'] = removeProductFromBasket($_SESSION['shoppingcart'], $_GET['itemId']);
?>
Of course there is still room for improvement, for example by creating an object ShoppingCart and a wrapper Session around the $_SESSION variable, but this should be a good place for you start and build upon.
