[eluser]diego.greyrobot[/eluser]
Hello all, I've been trying to think through this subtle logic error in a pair of if blocks.
I have this section of code:
Code: function check_inventory($id, $quantity) {
$Q = $this->db->get_where('products', array('id' => $id));
$p = $Q->row();
return array($p->title, ($p->quantity - $quantity));
}
//this is the main function that is called by the controllers and determines which other model functions to call.
function update_cart($id = false, $quantity = 1, $checkout = false) {
//if adding an item from the shop
if ($id) {
$item_id = $id;
if ($quantity > 0) $cart = $this->add_to_cart($item_id, $quantity);
else $cart = $this->delete_from_cart($item_id);
}
//if updating the cart from a cart form
else { $cart = $this->update_cart_post(); }
//don't do anything if we're just checking out, just pass the cart on
if ($checkout) $cart = $_SESSION['cart'];
//updating the cart is done, now set the session variables
$_SESSION['cart'] = $cart;
$_SESSION['sub_total'] = $this->cart_sub_total(); //cart_sub_total() depends on $_SESSION['cart'] being up to date
//set optional variables
//cart_total_price() depends on $_SESSION['cart'] being up to date
if (isset($_SESSION['shipping'])) $_SESSION['total_price'] = $this->cart_total_price();
if (!count($cart)) { //if theres nothing left in the cart, user has 0'ed out the quantities, cart must be deleted
unset($_SESSION['cart']);
unset($_SESSION['total_price']);
unset($_SESSION['sub_total']);
unset($_SESSION['fl']);
unset($_SESSION['shipping']);
}
}
function update_cart_post() {
$cart = $_SESSION['cart'];
$_SESSION['ship_cost'] = $this->input->post('ship_cost', true);
$_SESSION['shipping'] = $this->input->post('shipping_options', true);
$_SESSION['fl'] = ($this->input->post('fl', true) == 'y') ? 'y' : 'n';
$ids = $this->input->post('id', true);
$quantities = $this->input->post('quantity', true);
//combine items and quantities arrays together into a cart array and get sub_total price and sum up item quantities
$i = 0; $sub_total = 0; $total_items = 0;
foreach ($ids as $id) {
//get item price from db
$item = $this->products_model->get_product($id);
$cart[$id]['price'] = $item['price'];
$cart[$id]['id'] = $id;
$cart[$id]['title'] = $item['title'];
$cart[$id]['quantity'] = $quantities[$i];
$i++;
}
//check for 0 quantities and delete that item
foreach ($ids as $id) if ($cart[$id]['quantity'] == 0) unset($cart[$id]);
return $cart;
}
in this particular function:
Code: function update_cart($id = false, $quantity = 1, $checkout = false) {
//if adding an item from the shop
if ($id) {
$item_id = $id;
if ($quantity > 0) $cart = $this->add_to_cart($item_id, $quantity);
else $cart = $this->delete_from_cart($item_id);
}
//if updating the cart from a cart form
else { $cart = $this->update_cart_post(); }
//don't do anything if we're just checking out, just pass the cart on
if ($checkout) $cart = $_SESSION['cart'];
//code .....
}
...which gets passed in an int id in this particular case from segment 3 in the URL, gives me this output when i try to echo the value of id inside the first if block...
Code: if ($id) {
die("id: ".$id);
$item_id = $id;
if ($quantity > 0) $cart = $this->add_to_cart($item_id, $quantity);
else $cart = $this->delete_from_cart($item_id);
}
//output: "id: 41";
But if take the same die statement and put it in the else block it also runs...
Code: //if updating the cart from a cart form
else ($id === false) { die("id: ".$id); $cart = $this->update_cart_post(); }
//outputs: "id: "
I don't understand why the else block runs at all, or why the value of id is a bool(false) (if i do var_dump)
One thing i do know is that this code worked perfectly fine on my own pc during development, only after i moved it to the live server did this error show up. I checked that it was the same version, php5. Any ideas? what else should i check for?
[eluser]TheFuzzy0ne[/eluser]
You have a syntax error. Should be "else if ($condition)" not "else ($condition)". Also, you're trying to run code after telling the server to die().
Hope this helps.
[eluser]diego.greyrobot[/eluser]
Oh sorry, ignore that, that was just a typo in the last code block i missed when proofreading them. the error remains. Anything else it could be? thanks for the reply btw 
[eluser]TheFuzzy0ne[/eluser]
It's not totally clear to me what's happening here either. However, I do have a few suggestions:
CodeIgniter has an excellent sessions library. Unless you have a specific reason for wanting to use PHP sessions, I'd highly recommend you use it.
Encapsulation: I'm not sure if this is the problem, but most of your if and for* statements are missing curly braces. It makes your code very hard to read, and as you've just shown, quite difficult to debug. To save you some time, I've done it for you. Please let me know if it makes a difference:
Code: function check_inventory($id, $quantity)
{
$Q = $this->db->get_where('products', array('id' => $id));
$p = $Q->row();
return array($p->title, ($p->quantity - $quantity));
}
//this is the main function that is called by the controllers and determines which other model functions to call.
function update_cart($id = false, $quantity = 1, $checkout = false)
{
if ($id) //if adding an item from the shop
{
# I've remove the item_id variable as it's unnecessary.
$cart = ($id && $quantity > 0) ? $this->add_to_cart($id, $quantity) : $this->delete_from_cart($id);
}
else //if updating the cart from a cart form
{
$cart = $this->update_cart_post();
}
if ($checkout) //don't do anything if we're just checking out, just pass the cart on
{
$cart = $_SESSION['cart'];
}
//updating the cart is done, now set the session variables
$_SESSION['cart'] = $cart;
$_SESSION['sub_total'] = $this->cart_sub_total(); //cart_sub_total() depends on $_SESSION['cart'] being up to date
//set optional variables
if (isset($_SESSION['shipping'])) //cart_total_price() depends on $_SESSION['cart'] being up to date
{
$_SESSION['total_price'] = $this->cart_total_price();
}
if (!count($cart)) //if theres nothing left in the cart, user has 0'ed out the quantities, cart must be deleted
{
unset(
$_SESSION['cart'],
$_SESSION['total_price'],
$_SESSION['sub_total'],
$_SESSION['fl'],
$_SESSION['shipping']
);
}
}
function update_cart_post()
{
$cart = $_SESSION['cart'];
$_SESSION['ship_cost'] = $this->input->post('ship_cost', true);
$_SESSION['shipping'] = $this->input->post('shipping_options', true);
$_SESSION['fl'] = ($this->input->post('fl', true) == 'y') ? 'y' : 'n';
$ids = $this->input->post('id', true);
$quantities = $this->input->post('quantity', true);
//combine items and quantities arrays together into a cart array and get sub_total price and sum up item quantities
$i = 0;
$sub_total = 0;
$total_items = 0;
foreach ($ids as $id)
{
$item = $this->products_model->get_product($id); //get item price from db
$cart[$id]['price'] = $item['price'];
$cart[$id]['id'] = $id;
$cart[$id]['title'] = $item['title'];
$cart[$id]['quantity'] = $quantities[$i];
$i++;
}
//check for 0 quantities and delete that item
foreach ($ids as $id)
{
if ($cart[$id]['quantity'] == 0)
{
unset($cart[$id]);
}
}
return $cart;
}
[eluser]diego.greyrobot[/eluser]
Hey thanks for the reply. Ok so i pasted in your code:
Code: function update_cart($id = false, $quantity = 1, $checkout = false) {
//if adding an item from the shop
if ($id) {
$cart = ($id && $quantity > 0) ? $this->add_to_cart($id, $quantity) : $this->delete_from_cart($id);
}
//if updating the cart from a cart form
else { die(var_dump($id));
$cart = $this->update_cart_post();
}
//outputs: bool(false)
this is the main problem, that $id shows up as a bool within the else block but as numeric string in the upper if block. is there anyway i'm overwriting the $id value with something else? If i understand correctly these should be local function scope vars and that shouldnt be a problem right? why is the $id variable showing both values?
[eluser]TheFuzzy0ne[/eluser]
$id is set to FALSE by default in the function argument declaration, so if the else statement is hit, it will be false, if it's not hit, then you've supplied a number.
[eluser]diego.greyrobot[/eluser]
I default $id to false so that i can call that function with or without arguments. Help me to understand this correctly, if i use a default in the arg declaration AND i pass a value in $id in that function, the if else block will use both values?
if i delete the default value for $id and later call this function without arguments, will there be a problem?
[eluser]diego.greyrobot[/eluser]
Thanks FuzzyOne. I checked everything and both the if and the else run one right after the other. It's really weird, i know $id is set to a number (i can even echo out the value in an exit statement) and the code in the if($id) block will run but also the else block will run. and if a put an exit statement in the else block and echo the value of $id it is (false).
What I did to work around the problem was place a check inside the $this->update_cart_post() method to only run if there's post data.
That's mainly where my problem was, i would get an error because $this->update_cart_post() was trying to process non existent post data, even if an $id was passed in.
So the question remains, why do both the if and the else blocks run even if $id has a value that is not 0 or false?
Thanks for your help so far. It reaffirms my questions about php.
[eluser]TheFuzzy0ne[/eluser]
The default is used only if an id is not passed over as the first parameter. It's there so that you can simply check whether it's FALSE or not. If it's FALSE, you know it's not been specified by the calling function, if it's not FALSE, then it has. By unsetting the default, PHP will issue a warning if you don't specify an argument as the first parameter. This could lead to problems in the future, such as your visitors seeing errors on the page, and the application not functioning as intended. It can also lead to other errors, or other problems such invalid data being inserted into the database.
Basically, setting it with a default means that you won't get the PHP warning, but you still need to check in your function whether or not it's set to something other than FALSE before you use it.
Code: if ($id) //if ID is not 0, "" or FALSE
{
$cart = ($id && $quantity > 0) ? $this->add_to_cart($id, $quantity) : $this->delete_from_cart($id);
}
else // Otherwise it's 0, "" or FALSE, and hasn't been set with a meaningful value.
{
$cart = $this->update_cart_post();
}
If $id was not set to FALSE by default, you'd have to check if $id was set before using it, otherwise you'd end up with another PHP warning. You're application should be able to function without issuing any warnings or errors, with logging set to E_ALL. It's quite a tall order, but should be what every developer aims for. You should code carefully, and ensure that all potential errors are coded for, and handled correctly.
|
