Shopping Cart Problem

[paul_mellon36]

on my shopping cart i create a new session "basket" for every user.

when they select an product to add to there basket, i add that products "id" & "quantity" to the session like below:

Code:

<?php
session_start();

if(!isset($_SESSION['basket']))
{
$_SESSION['basket'] = array();
}

if(isset($_POST['selected_item_id']))
{
$u_item_id = $_POST['selected_item_id'];
$u_quantity = $_POST['sel_item_qty'];
$u_details = $u_item_id."|".$u_quantity;
$_SESSION['basket'][] = $u_details;
header("Location: showcart.php");
}
?>

in showcart.php I get the "id" and "quantity" from the session by:

Code:

$product = $_SESSION['basket'][$i];
$myarray = explode('|', $product);

this code is inside a for loop as for the reason of the "$i" been used....

my problem lies in selecting the products colour from tblstore_colour where the tblstore_colour FK "item_id" is equals to the tblstore_items PK "$myarray[0]"

i get the same colour for each of my products when only some products in my basket have a colour.

this is how i query tblstore_colour for each product colour:

Code:

$get_color = "SELECT * FROM tblstore_colour, tblstore_items WHERE tblstore_colour.item_id = tblstore_items.id AND tblstore_colour.id = $myarray[0]";

querying the tblstore_items for products details where tblstore_items.id = $myarray[0] works fine, displaying each products details in a table but i have colours in a separate table and this is where my problem lies...

here is how i have my table created/populated:

Code:

mysql_query("insert into tblstore_items values ('1', '1', 'Easter Egg Selection',5.99, 'Which chocolate most tickles your fancy this easter?','images/easter/newsletter_easter.jpg')");
mysql_query("insert into tblstore_items values ('2', '1', 'Chocolate Bunny',4.99, 'Give this gorgeous bunny to a loved one this easter.','images/easter/EasterBunny.jpg')");
mysql_query("insert into tblstore_items values ('3', '1', 'Easter Candle Pack',10.99, 'Light up your life this easter with some scented candles.','images/easter/eastdisp.jpg')");

Code:

mysql_query("insert into tblstore_colour values ('1', '2', 'Gold')");
mysql_query("insert into tblstore_colour values ('2', '2', 'Silver')");
mysql_query("insert into tblstore_colour values ('3', '3', 'Yellow')");
mysql_query("insert into tblstore_colour values ('4', '3', 'White')");
mysql_query("insert into tblstore_colour values ('5', '3', 'Mixed')");

again tblstore_colour has PK and FK, FK been tblstore_item's PK...

so again my code to loop through each product in the basket and display its details ina table is:

Code:

for($i = 0; $i < count($_SESSION['basket']); $i++)
{
$product = $_SESSION['basket'][$i];
$myarray = explode('|', $product);

$get_cart = "SELECT c.id AS cat_id, c.cat_title, si.id, si.item_title, si.item_price, si.item_desc, si.item_image FROM tblstore_items AS si LEFT JOIN tblstore_categories AS c ON c.id = si.cat_id WHERE si.id = $myarray[0]";
$get_cart_res = mysql_query($get_cart,$conn);

while($row = mysql_fetch_array($get_cart_res))
{
$item_id = $row['id'];
$item_title = $row['item_title'];
$item_price = $row['item_price'];
}
$unit_price = sprintf("%.02f", $item_price * $myarray[1]);
$total_price += $unit_price;



$get_color = "SELECT * FROM tblstore_colour, tblstore_items WHERE tblstore_colour.item_id = tblstore_items.id AND tblstore_colour.id = $myarray[0]";



$get_color_results = mysql_query($get_color,$conn);

while($row_colour = mysql_fetch_array($get_color_results))
{
$colour = $row_colour['item_colour'];
}
$colours = (empty($colour)) ? "&nbsp;" : $colour;

$display_block2 .= "
<tr>
<td class=border align=center>$item_title</td>
<td class=border align = center>$size</td>
<td class=border align = center>$colours</td>
<td class=border align = center>£ $item_price</td>
<td class=border align = center>$myarray[1]</td>
<td class=border align = center>£ $unit_price</td>
<td class=border align = center><a href=removefromcart.php?id=$item_id>Remove</a></td>
</tr>";
}

thanks for help in advance...

if you require more information please ask!

paul_mellon36  :D

[VGR]

hello, I'm not sure I understood if your SELECT with implicit (cartesian) JOIN will return anything if the item has no corresponding row in the colours table, but anyway you can't do this :

$get_color = "SELECT * FROM tblstore_colour, tblstore_items WHERE tblstore_colour.item_id = tblstore_items.id AND tblstore_colour.id = $myarray[0]";

you've to do :

$get_color = "SELECT * FROM tblstore_colour, tblstore_items WHERE tblstore_colour.item_id = tblstore_items.id AND tblstore_colour.id = {$myarray[0]}";

[VGR]

well, for a start here's a static analysis of your code :

1) you're in a loop and you don't re-initialize $coulour. No wonder it's not tested correctly as "empty($colour)" afterwards.

2) you don't have any error code in case a query fails : you try the mysql_fetch_array() nonetheless

3) I suggest you transform also the $myarray[1] in the last string into {$myarray[1]}

4) I definitely think you should do a LEFT JOIN and test whether the colour returned is NULL or not before trying to read and display it ;-)

5) I suggest you run the same script with NOTICEs displayed ;-)
just do at first : error_reporting(E_ALL);

[seandelaney]

Hi Paul & VGR

4) I definitely think you should do a LEFT JOIN

You don't need a LEFT JOIN, because that will give you rows that don't match.  You want an INNER JOIN.

Try this:

Code:

SELECT tblstore_colour.* FROM tblstore_colour JOIN tblstore_items USING (id) WHERE tblstore_colour.item_id = '$myarray[0]';

VGR im not disagreeing with you or syaing your solution(s) to improve pauls code is wrong, its just I think INNER JOIN is the way to go...  :wink:

Sean

[VGR]

may-be you're right. It depends what he wants exactly.

given he selects from tblstore_items JOIN tblstore_colours, and some items don't have a colour, I thought it was better to return the tblstore_items columns anyway. May-be I misunderstood the purpose of the code ? ;-)

[seandelaney]

If it's just that one column that is blank, it's likely that your code isn't retrieving the column name that you want it to.  Perhaps an alias will help - try this:

Code:

$get_color = "SELECT tblstore_colour.item_colour FROM tblstore_colour, tblstore_items WHERE tblstore_colour.item_id = tblstore_items.id AND tblstore_colour.id = {$myArray[0]}";

Sean  :D

[VGR]

no, as a result from the LEFT JOIN, the columns from the 2nd table are blank is the join condition was not met, but the column names are there in the result set.

that's pretty standard. I don't get your point :D

[seandelaney]

just noticed an error in my last comment.... :scratch:

Code:

$get_item_colour = "SELECT tblStore_colour.Colour FROM tblStore_colour, tblStore_items WHERE tblStore_colour.item_id = tblStore_items.id AND tblStore_colour.item_id = {$myArray[0]}

sorry for confusion   :-$

[paul_mellon36]

i have tried all of these things and no success so far???

Code:

$get_color = "SELECT * FROM tblstore_colour, tblstore_items WHERE tblstore_colour.item_id = tblstore_items.id AND tblstore_colour.item_id = {$myarray[0]}";



$get_color_results = mysql_query($get_color,$conn);

while($row_colour = mysql_fetch_array($get_color_results))
{
$colour = $row_colour['item_colour'];
}
$colours = (empty($colour)) ? "&nbsp;" : $colour;


$display_block2 .= "
<tr>
<td class=border align=center>$item_title</td>
<td class=border align = center>$size</td>
<td class=border align = center>$colours</td>

[VGR]

as I told you, $colour should be reinitialized to array() on each loop's iteration

[seandelaney]

VGR

You have lost me now?

[VGR]

let's imageine that a colour was found for a given item_id.
As you test $colours = (empty($colour))?someth:elsewh; this evaluates to true, and also to true forever, because you didn't re-empty $colour at the start of the loop's iteration.

no ?

[seandelaney]

yes! - had my "Java head" screwed on earlier...  :cheers:

[seandelaney]

Right OK paul - do you undertand?

at the beginning of the for loop create your variable

Code:

$colour = "";

then inside the loop after you have queried the DB, have:

Code:

$get_color_results = mysql_query($get_color,$conn);

      while($row_colour = mysql_fetch_array($get_color_results))
      {
         $colour = $row_colour['item_colour'];
      }
      $colours = (empty($colour)) ? "&nbsp;" : $colour;

then right before the last } of the for loop after your $display_block .= "...";

reset your $colour variable empty again like so:

Code:

$colour = "";

Im sure VGR is assist you if your still confused... speak to you tomz...

sean  :D

p.s Rangers are crap!

[VGR]

in fact, I don't agree with both of you ;-)

to not confuse things between isempty(), isset(), <>"", count()==0 etc depending on the type of the variable tested, and given PHP has a polymorphism which can be sometimes dangerous, I suggest this :

As you clearly have a string (or integer or other scalar value) eventually - in the French sense - in $row_colour['item_colour'], I suggest to set initially the variable $colour to the empty string and "stupidly" test is it's still the empty string or not afterwards.

Let's imagine something like :

Code:

foreach($_SESSION['basket'] as $product) {
  $myarray = explode('|', $product);

  $get_cart = "SELECT c.id AS cat_id, c.cat_title, si.id, si.item_title, si.item_price, si.item_desc, si.item_image FROM tblstore_items AS si LEFT JOIN tblstore_categories AS c ON c.id = si.cat_id WHERE si.id = {$myarray[0]}";
  $get_cart_res = mysql_query($get_cart,$conn);

  $row=mysql_fetch_array($get_cart_res);
  $item_id = $row['id'];
  $item_title = $row['item_title'];
  $item_price = $row['item_price'];
  $unit_price = sprintf("%.02f", $item_price * $myarray[1]);
  $total_price += $unit_price;

  $get_color = "SELECT * FROM tblstore_colour, tblstore_items WHERE tblstore_colour.item_id = tblstore_items.id AND tblstore_colour.id = {$myarray[0]}";

  $colour="";
  $get_color_results = mysql_query($get_color,$conn);

  if ($get_color_results!==FALSE) { // or else, add an " or die("failed to execute '$get_color' : ".mysql_error());" to the mysql_query() call above...
    if ($row_colour=mysql_fetch_array($get_color_results))) $colour = $row_colour['item_colour'];
    $colours = ($colour=="") ? "&nbsp;" : $colour;
  } else $colours='$nbsp;'; // if failed to execute query on colours
  $display_block2 .=<<<EOS
<tr>
  <!--VGR REM the align should be a CSS attribute in the class border. Beware, border is a reserved keyword and can lead to confusion-->
  <td class="border" align="center">$item_title</td>
  <td class="border" align="center">$size</td>
  <td class="border" align="center">$colours</td>
  <td class="border" align="center">£ $item_price</td>
  <td class="border" align="center">{$myarray[1]}</td>
  <td class="border" align="center">£ $unit_price</td>
  <td class="border" align="center"><a href="removefromcart.php?id=$item_id">Remove</a></td>
</tr>
EOS;
} // foreach

I think it's better written. You had two loops which seemed useless to me (unless you REALLY want to parse through an entire results set keeping ONLY the last row's values ;-)
Let's assume you've only one row that interests you : the first one. Else modify your code to get all the colours correctly.

I also thought you'd want to display something whatever the case.