views:

193

answers:

1

I'm writing a script to let users place items in their baskets. It is very complex so far, and I would like to talk it through with someone to see if they can suggest better design or tidy the current design up. Here is the code I have, which doesn't work very well (i.e. has errors that I haven't yet resolved), with comments to show my intentions:

<?php
session_start();
include_once("db_include.php5");
doDB();


if(!$_GET["productid"] || !$_GET["qty"]) {
//the user has entered the address directly into their address bar, send them away (if=1 to let me know where the script branched)
header("Location:index.php5?if=1");
exit();
}

**//do select query to verify item id is valid, in case they entered data into the query string or the item has been removed from db**
$check_sql = "SELECT * FROM aromaProducts1 WHERE id='".$_GET["productid"]."'";
$check_res = mysqli_query($mysqli, $check_sql) or die(mysqli_error($mysqli));

if(mysqli_num_rows($check_res) == 0) {
**//item doesn't exist, redirect user**
header("Location:index.php5?if=2");
exit();
} else if(mysqli_num_rows($check_res) != 0) {
**//item exists
//do select query to check for item id already in basket - if this item is already in the table associated with the user's session id (which will be added every time an item is), then we want to change the quantity only**
$duplicate_sql = "SELECT qty FROM sessionBasket WHERE product_id='".$_GET["productid"]."' AND usersessid='".$_SESSION["PHPSESSID"]."'";
$duplicate_res = mysqli_query($mysqli, $duplicate_sql) or die(mysqli_error($mysqli));

if(mysqli_num_rows($duplicate_res) != 0) {
**//item in basket - add another - fetch current quantity and add new quantity**
$basketInfo = mysqli_fetch_array($duplicate_res);
$currQty = $basket_info["qty"];
$add_sql = "UPDATE sessionBasket SET qty='".($_GET["qty"]+$currQty)."' WHERE usersessid='".$_SESSION["PHPSESSID"]."'AND product_id='".$_GET["productid"]."'";
$add_res = mysqli_query($mysqli, $add_sql) or die(mysqli_error($mysqli));

if($add_res !== TRUE) {
**//wasn't updated for some reason - this is where my script currently breaks**
header("Location:basketfailredirect.php5?error=add");
exit();
} else if($add_res === TRUE) {
**//was updated - send them away**
header("basket.php5?res=add");
exit();
}


} else if(mysqli_num_rows($duplicate_res) == 0) {
**//no existing items in basket, so we want to add the current item info associated with the user's id/session id**

**//fetch product id, passed in query string from the product info page**
$productid = $_GET["productid"];

**//sanitize possible inputs, if set - notes is a field added to the product info page for custom products, and we want to sanitize it if it's set - note that check_chars_mailto() is a function I have in the db_include file**
$notes = isset($_GET["notes"])?trim(mysqli_real_escape_string(check_chars_mailto($_GET["notes"]))):"";
**//if the user is logged in, their userid is stored in the session variable**
$userid = $_SESSION["userid"]?$_SESSION["userid"]:"";
**//not sure about the keep alive option - i.e. store basket contents even if the user doesnt register/sign in, but keeping the option there**
$alive = $_SESSION["alive"]?$_SESSION["alive"]:"no";


**//insert query**
$insert_sql = "INSERT INTO sessionBasket (userid, usersessid, date_added, keep_alive, product_id, qty, notes) VALUES (
'".$userid."',
'".$_SESSION["PHPSESSID"]."',
now(),
'".$alive."',
'".$productid."',
'".$_GET["qty"]."',
'".htmlspecialchars($notes)."')";
$insert_res = mysqli_query($mysqli, $insert_sql) or die(mysqli_error($mysqli));

if($insert_res === TRUE) {
**//success**
header("Location:basket.php5?res=add");
exit();
} else if($insert_res !== TRUE) {
**//fail**
header("Location:basketfailredirect.php5?error=add2");
exit();
}
}
}
?>

It's pretty complicated for me - I want to allow empty fields, add the userid if it's available (which is missing from my UPDATE query)... Is this a million miles from a good design, or what?

Also, when I try to add items to the basket, I now get an error: internal server error 500. I suspect this is due to bad coding because my search results and product view pages work and they use the same server and the same database as this script.

+1  A: 

You should consider using PHP's in-built pseudo object orientated style.

You should also perhaps look in to using a PHP Framework such as Zend or CakePHP. Even if you don't go with a PHP framework, you should still be able to create your code in an object-orientated fashion via php's class and interface objects.

By seperating out this code in to classes and functions you will make your (and our) debugging much much easier, both now and when you come to edit the code at some point in the future.

I am very new to programming, so the step up to object oriented is a delay that I would rather avoid on this project. I get on well with the procedural style for the moment, but I will bear the suggestion in mind. After this project, I do want to advance my knowledge and application of all the languages I'm working with, and OOP seems to be the next logical move.