Not sure why this isn't working

This is partially from the Agile Web Dev with Rails book. It is the
section where there was a clear cart button add to the AddToCart page.
I thought I would make things a little more interesting and instead of
clearing the whole cart that I would allow the the quantities to be
reduced until there was none left, then the item would be fully
removed.

Unfortunately, when the “remove from cart” button is clicked nothing
happens.

Here is the code for the Cart.rb

class Cart
include Reloadable

attr_reader :items

def initialize
#create an empty array for the cart_items
@items = []
end

#add the product to the items array
def add_product(product)

existing_product = find_product_in_cart(product.id)

if existing_product
existing_product.increment_quantity
else
@items << CartItem.new(product)
end

end

def remove_product(id)

existing_product = find_product_in_cart(id)

if existing_product
if existing_product.quantity > 1
existing_product.decrement_quantity
else
existing_product = nil
end
end

end

def find_product_in_cart(id)
@items.find {|item| item.product.id == id}
end

end

The find_product_in_cart refactored out what could be in the finding of
an item in the cart. The method works when adding or incrementing the
quantity, but it is where the remove_product method fails. in the
remove_product method the “existing_product” value is nil and never
makes it into the if statement.

Is there something silly that I am missing in the @items.find… I
don’t understand why this doesn’t work.

Thanks for any help

Chris O. wrote:

The find_product_in_cart refactored out what could be in the finding of
an item in the cart. The method works when adding or incrementing the
quantity, but it is where the remove_product method fails. in the
remove_product method the “existing_product” value is nil and never
makes it into the if statement.

Is there something silly that I am missing in the @items.find… I
don’t understand why this doesn’t work.

Thanks for any help

Random guess: You are passing params[:id] (or similar) into
remove_product
However params[:id] is a string, and so is not == to the id attribute of
any Product items (since those are integers.

Fred

Frederick C. wrote:

Random guess: You are passing params[:id] (or similar) into
remove_product
However params[:id] is a string, and so is not == to the id attribute of
any Product items (since those are integers.

Fred

Thanks for the help, but I don’t think Ruby is a language that is
sensitive to datatypes. I am new to the language, but I have not been
able to find anything on the subject.

Thanks

Chris O. wrote:

Thanks for the help, but I don’t think Ruby is a language that is
sensitive to datatypes. I am new to the language, but I have not been
able to find anything on the subject.

Ruby may not be statically typed, but that does not mean that 1 == “1”
(try it in irb) (and thus your code will not work if the id parameter
passed in is a string containing a number rather than the number itself.
It’s ok with ActiveRecord::Base#find, but that’s a different story.

Fred

Chris,

In your remove_product method existing_product is a local variable, a
copy of an element in the @items array. When you manipulate
existing_product it has no effect on @items array. Here is a basic
example:

a = [1,2,3] => [1, 2, 3]
b = a[1] => 2
b += 1 => 3
b => 3
a => [1, 2, 3]

Aaron

“Chris O.” wrote:

[snip]

Here you are just setting the local var to nil. To correct this you
should
do something along the line of:

@items.remove(existing_product)

hth,

Long

Frederick C. <rails-mailing-list@…> writes:

Chris O. wrote:

Thanks for the help, but I don’t think Ruby is a language that is
sensitive to datatypes. I am new to the language, but I have not been
able to find anything on the subject.

Ruby may not be statically typed, but that does not mean that 1 == “1”
(try it in irb) (and thus your code will not work if the id parameter
passed in is a string containing a number rather than the number itself.
It’s ok with ActiveRecord::Base#find, but that’s a different story.

Yeah, you’re confusing dynamic typing (where you don’t have to declare
variable
types in the source code) with loose typing (where variable types are
automatically converted where needed).

Ruby can sort of support loose typing, but it relies on you doing the
conversion
inside your own functions. Unfortunately, since that’s not a universal
convention, you’re unlikely to be able to guarantee other code works
like that.

Gareth

It is all working now.

I now ensure that both values in the comparison statement block use the
to_i method to get them in the integer format. (Thanks Fred)

And as I figured Long’s comment about the remove function helped me out
once I got past the first little problem.

Thanks for the help guys I actually learned quite a bit from this
example :slight_smile:

Thanks for the help

Fred:
After testing it with irb I can see how it will cause a problem.

Aaron:
I am a still little unclear with Ruby since everything is an object.
If what you say is correct, how is it that it works when incrementing
the product quantity?

Long:
Thanks for the tip. Since it wasn’t making it up to the enclosing
condition I wasn’t worrying about it yet, but I thought there was going
to be an issue with that statement. You saved me from later confusion.

Hi –

On Sat, 23 Sep 2006, Aaron B. wrote:

Chris,

In your remove_product method existing_product is a local variable, a
copy of an element in the @items array.

It’s not a copy; it’s a new reference to the same object.

When you manipulate existing_product it has no effect on @items
array. Here is a basic example:

a = [1,2,3] => [1, 2, 3]
b = a[1] => 2
b += 1 => 3

That’s the same as:

b = b + 1

You’re re-initializing the local variable b; it no longer has any
connection with what it was assigned to previously. So you’re not
really putting to the test the question of what happens when you
manipulate the object you got from the array. (And you can’t change
integers anyway :slight_smile:

b => 3
a => [1, 2, 3]

Note the following, however:

a = %w{a b c} => [“a”, “b”, “c”]
b = a[1] => “b”
b.upcase! => “B”
b => “B”
a => [“a”, “B”, “c”]

Here, b is a reference to the second element of a. I do an in-place
upcasing of b, and since b and a[1] are the same object, a[1] is now
uppercase.

David


David A. Black | [email protected]
Author of “Ruby for Rails” [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB’s Weblog) [2] | Co-director, Ruby Central, Inc. [4]
[1] Ruby for Rails | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com | [4] http://www.rubycentral.org