0

I have a following question. I store a shopping cart in an array session like below

session_start();
$id= $_GET['id'];
if(isset($_SESSION['cart']))
{
array_push($_SESSION['cart'], $id);
}
else
    $_SESSION['cart']= array($id);

header("location:cart.php");

And when I try to retrieve the cart. I get the same product id as many as I put to the cart.

<?php
if(!isset($_SESSION['cart'])) {
    echo "Your cart is empty.<br /><br /><a href='products.php'>Show products</a>";
} else {
    echo '<table border="0.2">';

    $total_price = 0;

    foreach($_SESSION['cart'] as $id) {
        $the_query = "select * from products where id='$id' GROUP BY id";

        $result = mysql_query($the_query) or die('Query failed: ' . mysql_error());

        $the_product = mysql_fetch_array($result, MYSQL_ASSOC);

        $total_price = $total_price + $the_product['price'];

        $href = "show_products.php?id=".$the_product['id'];
        //echo "<tr>";
        echo "<tr><td><a href='$href'>";
        echo "<img src='".$the_product['image_url_small']."' /></a></td>";
        echo "<td><strong>".$the_product['name']."</strong></td><td><em>$".$the_product['price']."</em>";
        echo "</td>";
        echo "<td> <a href='do_deletecart.php?id=". $the_product['id'] ."'>Delete item </a></td></tr>";
    }
    echo "<tr><td colspan='2'></td></tr>";
    echo "<tr><td style='text-align:center;font-size:40px;'>$</td><td><strong>Total</strong><br /><em>$".$total_price."</em></td></tr>";
    echo "</table>";
    echo "<br /><a href='empty_cart.php'>Empty Cart</a> <a href='showallproducts.php'>Show phones</a><br /><br />";
}

how can I make it show only one product id or name. Thank in advance

2
  • 1
    This code is highly vulnerable to SQL Injection. Please, sanitize your inputs and use a recent API Commented Apr 29, 2013 at 10:20
  • I'm actually new to PHP, thanks for your advice. Commented Apr 30, 2013 at 8:33

2 Answers 2

1

If I understand your question correctly, you are getting many results for the same product id. This is because you are storing same id values many time in the $_SESSION variable.

You could do the following to not repeat the same ids in the $_SESSION variable.

EDIT

For sake of completeness I have updated the code. Hope that helps.

index.php

<?php

session_start();

$id= isset($_GET['id']) ? $_GET['id'] : null;

if(!is_null($id)){
    if(isset($_SESSION['cart']) && count($_SESSION['cart']) > 0){

        // increment product quantity if already exists
        // or create a new one
        add_or_increment_product_to_cart($id, $_SESSION['cart']);

    } else {
        // initialize cart
        // add the first product
        $_SESSION['cart'] = array();
        array_push($_SESSION['cart'], (object) array('id' => $id, 'quantity' => 1));
    }
}

function add_or_increment_product_to_cart($id, $cart){

    foreach ($cart as $key => $product) {
        if($id == $product->id){
            $product->quantity++;
            return;
        }
    }

    array_push($_SESSION['cart'], (object) array('id' => $id, 'quantity' => 1));
}

header("location:cart.php");

Cart.php

<?php

session_start();

$cart = isset($_SESSION['cart']) ? $_SESSION['cart'] : null;

if($cart) {
    foreach ($cart as $key => $product) {
        $the_query = "SELECT * FROM products WHERE id=" . $product->id . " LIMIT 1";

        // your code to fetch the products from the database
        // what you have done is fine but vulnerable
        // PDO recommended 
    }
} else {
    echo "Your cart is empty.<br /><br /><a href='products.php'>Show products</a>";
}

Also please note that mysql_connect is deprecated and PDO class is the recommended and safe way to connect to the database. Your code is vulnerable to SQL Injection like @Touki said in his comment.

Sign up to request clarification or add additional context in comments.

3 Comments

Using an array() would work but is still a bit messy. I'd suggest instead of fixing symptoms, to fix the root of the problem (multiple instances of the same product ids).
Yes. I agree. The in_array() check makes sure that there is no repeated instances of the same id.
My mistake, I didn't read your example correct. This sample is used to inject any new ids instead of looping to print them.
0

I would recommend performing only one query to retrieve all of the products, and then iterate the result of the query to populate the HTML. For example;

$the_query = "select * from products where id in (". implode(',', $_SESSION['cart']) .")";
$result = mysql_query($the_query);
while (($the_product = mysql_fetch_array($result, MYSQL_ASSOC))) {
    ...
}

This has the added bonus that you only perform one query, and would also only select one row per product.

It's worth noting, however, that the mysql_* methods are deprecated, and it would be advisable to start using another library such as mysqli or PDO.

On a related note, this code currently is very liable to SQL injection, and the input should ideally be sanitised before being put into a query string.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.