How to give discounts in simple Ruby shopping cart?

Question

I'm trying to make a very simple Ruby shopping cart, and I need to be able to give discounts if a user buys certain combinations of goods. These are indicated in the @costs - if bulk is true, a user gets a discount (of :bulk_price) for buying :bulk_num of goods. I've got it making basic charges, but now I need to subtract discounts in certain cases. Here's what I have so far:

    class Cart
     attr_accessor :total, :costs, :item_array, :subtotal

     def initialize
      @item_array=[]
      @subtotal=0
      @costs= [{"A"=>{:price=>2, :bulk=>true, :bulk_num=>4, :bulk_price=>7}}, {"B"=>{:price=>12, :bulk=> false}},{"C"=>{:price=>1.25,:bulk=>true, :bulk_num=>6, :bulk_price=>6}}, {"D"=>{:price=>0.15, :bulk=>false}}]
     end



    def scan(*items)
     items.each do |item|
     @item_array<<item
     @costs.each do |cost|
     if cost.has_key?(item)
      @subtotal+=cost[item][:price]
     end
   end
   @subtotal
 end
end


 def total

 end

end

Now, I've created an array to keep track of which items are purchased, and I'd ideally like to have the total function check the array and subtract from the subtotal if needed. Maybe I've just been staring at this too long, but I am having trouble figuring that out. Could anyone help?

Answer 1

Since your question involves an exercise, I decided to change it around a bit to make some points that you might find helpful. A few notes:

• I renamed scan to checkout, lest the former be confused with String#scan
• An order quantity is given for each item ordered, in the form of a hash that is passed to the checkout method;
• I changed :bulk_price to a unit price that applies if :bulk is true and the quantity ordered is at least :bulk_num.
• I changed @costs to a hash, because you need to access item names, which are now keys.
• I moved @costs outside the class, for two reasons. Firstly, that data is likely to change, so it really shouldn't be hardwired in the class definiation. Secondly, doing that provides flexibility should you want different class instances to use different @costs. You'll see I chose to pass that hash as an argument when creating a new class instance.
• I eliminated all your accessors.
• An exception is now raised if you enter an item name that is not a key in @costs.

This is the approach I took:

class Cart
  def initialize(costs)
    @costs= costs
  end
  def checkout(items)
    purchases = {}
    items.each do |(item, qty)|
      cost = @costs[item]
      raise ArgumentError, "Item '#{item}' not in @costs array" \
        if cost == nil
      if cost[:bulk] && qty >= cost[:bulk_num]
        tot_cost = qty.to_f * cost[:bulk_price]
        discount = qty.to_f * (cost[:price] - cost[:bulk_price])
      else
        tot_cost = qty.to_f * cost[:price]
        discount = 0.0
      end
      purchases[item] = {qty: qty, tot_cost: tot_cost, discount: discount}
    end
    purchases
  end
  def tot_cost(purchases)
    purchases.values.reduce(0) {|tot, h| tot + h[:tot_cost]}
  end
  def tot_discount(purchases)
    purchases.values.reduce(0) {|tot, h| tot + h[:discount]}
  end
end 

costs = {"A"=>{price:    2, bulk: true, bulk_num: 4, bulk_price: 1.75},
         "B"=>{price:   12, bulk: false                              },
         "C"=>{price: 1.25, bulk: true, bulk_num: 6, bulk_price: 1.00},
         "D"=>{price: 0.15, bulk: false                              }}
cart = Cart.new(costs)

purchases = cart.checkout({"A"=>6, "B"=>7, "C"=>4}) # item => quantity purchased
p purchases # => {"A"=>{:qty=>6, :tot_cost=>10.5, :discount=>1.5},
            # =>  "B"=>{:qty=>7, :tot_cost=>84.0, :discount=>0.0},
            # =>  "C"=>{:qty=>4, :tot_cost=>5.0,  :discount=>0.0}}

p cart.tot_cost(purchases)     # => 99.5
p cart.tot_discount(purchases) # =>  1.5

Answer 2

A few things:

• Indent your code properly, it will make it much easier for you in the long run.
• Remove :total from attr_accessor, it isn't needed and the generated total method will be overridden by the one you define later on.
• Consider making each item an object which knows its own cost, rather than looking up the cost of each item in @costs. Conceptually, it doesn't make sense for a "shopping cart" to keep track of all the prices of all the items in your store.
• Make your total method functional. Don't bother subtracting from @subtotal -- it will cause problems if total is called more than once.

• Actually, subtotal would also be better if you recalculate whenever needed:

  def subtotal
    @item_array.reduce(0) { |sum,item| sum + (@costs[item][:price] || 0) }
  end
  

It may not be obvious to you now, but writing your code "functionally", like this, makes it easier to avoid bugs. You can cache values if they are really expensive to calculate, and will be needed more than once, but in this case there's no need to.

• For total, you can do something like:

  def total
    result = self.subtotal
    # check which discounts apply and subtract from 'result'
    result
  end