Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I find my self often doing things like this in Ruby on Rails:

@count = 0

LineItem.all.each do |item|
     if (item.cart_id == @cart.id)
       @count += item.quantity
     end
end

So many things look wrong with this, but I want to initialize my variable (@count) before the loop, and then be able to use it after I'm out of the loop.

Is there a better way to do this?

share|improve this question
add comment

2 Answers

up vote 1 down vote accepted

From what I can see, you are just collecting the item.quantity of all matching items. This can be accomplished by

@count = LineItem.all.find_all{|i| i.cart_id == @cart.id}.map{|i| i.quantity}.inject(0,:+)

Do you mean that you might be doing something else with in the cart_id check?

share|improve this answer
    
Both this and the other answer work. This one seems to complete the queries faster. I'm using Postgres. Thank you both! –  David West Jun 28 '12 at 17:41
add comment

If you work with database you can do this more efficiently with sql request:

LineItem.where(:card_id => @card.id).sum('quantity')

This is equivalent sql query

SELECT SUM(items.identity) FROM items WHERE items.card_id = ?;
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.