Update README.md

Checkout now stores its items using a quantity and a link back to the
original product instance. It has a reference to the store to make sure
that product IDs added are valid

README.md

@@ -4,44 +4,22 @@
 
 * Ruby 4.0.6
 * Minitest 6.0.6
+* Rake 13.4.2
 
 ## Instructions
 
-Checkout class is in `checkout.rb`
+Run tests with `rake test`
 
-Run tests with `ruby -Ilib:test tests.rb`
 
-`main.rb` is a scratchpad
+## New since last review!
 
-## Initial Thoughts
+As instructed, the functionality has been expanded.
 
-* Cart can be stored in an array of strings
-* Cart items should be validated against a known list of products
-* We need a hash of products and their prices
+Here is a summary before you dive in:
 
-## Implementation Details
-
-Cart items are added via symbol, and their prices are simply stored in a hash constant on the `Checkout` class iteself.
-
-Item validation is a simple lookup in the `AVAILABLE_ITEMS` hash, and a custom exception is raised if the item is not found.
-
-Discounts are calculated separately from the running total of the checkout and applied afterwards.
-
-Discounts are simple calculations done in-line, again for simplicity
-
-## Future Work
-
-Some basic stuff is missing.
-* Checkout items are not grouped when listed
-* Subtotals are not listed anywhere
-* Discount totals are not assoiated with the items they apply to (or available to the user at all, they're just applied to the total silently)
-
-The first thing I'd want to do is make the items real objects. You'd instantiate them from templates and apply discounts directly to them (e.g. -0.50 per Pico when there are enough in the cart)
-
-We'd need a new discount method to go along with that. Each discount type would be its own class that you'd pass the checkout items to and it would mark the discounts on them as appropriate. Maybe even leave a reference back to the discount used.
-
-Then we could do a lot more with displaying the checkout to the user, including the features listed in the bullets above.
-
-Beyond that there are a lot of directions we could go. Happy to explore this later.
-
-Thanks for reading!
+* Tests are now run via `rake test`, and the `main.rb` scratchpad has been removed.
+* Product information is now stored in a `Product` class that can be instantiated with an ID, price, and discount if applicable. There is a `Shop` class that keeps track of these.
+* The `Checkout` class still exists, but adding and removing items can now be done in bulk. We still use product IDs when doing this.
+* Discount logic is encapsulated in a class. Each product can have one kind of discount at the moment, chosen from "Buy X get one free" or "Bulk discount when X are purchased" in order to satisfy requirements.
+* New test classes have been created as appropriate, although the bulk of the tests are still against the `Checkout` class.
+* See the `setup` method in `tests/checkout_test.rb` for a full example of how a shop might be configured.

Rakefile

@@ -0,0 +1,3 @@
+require "minitest/test_task"
+
+Minitest::TestTask.create

bulk_discount.rb

@@ -0,0 +1,10 @@
+class BulkDiscount
+  def initialize(unit_discount:, trigger_quantity: 2)
+    @unit_discount = unit_discount
+    @trigger_quantity = trigger_quantity.to_f
+  end
+
+  def discount_for_quantity(quantity)
+    quantity >= @trigger_quantity ? @unit_discount * quantity : 0.00
+  end
+end

buy_x_get_one_free_discount.rb

@@ -0,0 +1,10 @@
+class BuyXGetOneFreeDiscount
+  def initialize(unit_price:, trigger_quantity: 2)
+    @unit_price = unit_price
+    @trigger_quantity = trigger_quantity.to_f
+  end
+
+  def discount_for_quantity(quantity)
+    (quantity / @trigger_quantity).floor * @unit_price
+  end
+end

checkout.rb

@@ -1,62 +1,68 @@
+require 'product_in_checkout.rb'
+
 class Checkout
   class InvalidItem < StandardError; end
 
-  AVAILABLE_ITEMS = {
-    PI5: 60.00,
-    ZERO2W: 15.00,
-    PICO2: 4.00
-  }
-
-  def initialize
+  def initialize(shop:)
+    @shop = shop
     @items = []
   end
 
-  def add(item)
-    raise InvalidItem unless valid_item?(item)
-    @items.push item
+  def add(product_id:, quantity: 1)
+    product = @shop.product_by_id(product_id)
+
+    raise InvalidItem unless product
+
+    new_or_existing_product_in_checkout(product).add_quantity(quantity)
   end
 
-  def remove(item)
-    if index = @items.index(item)
-      @items.delete_at(index)
-    end
+  def remove(product_id:, quantity: 1)
+    product = @shop.product_by_id(product_id)
 
-    @items
+    raise InvalidItem unless product
+
+    product_in_checkout = existing_product_in_checkout(product)
+
+    return false unless product_in_checkout
+
+    product_in_checkout.remove_quantity(quantity)
+
+    trim_empty_items
   end
 
   def items
     @items
   end
 
-  def total
-    pre_discount_total - current_discount
+  def item_quantities
+    @items.map { |item| [item.product_id, item.quantity] }.to_h
   end
 
   def clear
     @items = []
   end
 
+  def total
+    @items.reduce(0.00) { |running_total, item| running_total + item.total }
+  end
+
   private
 
-  def valid_item?(item)
-    return AVAILABLE_ITEMS.keys.include?(item)
+  def existing_product_in_checkout(product)
+    @items.find { |item| item.product_id == product.id }
   end
 
-  def current_discount
-    # BOGOF ZERO2W
-    discount = (@items.count(:ZERO2W) / 2.0).floor * AVAILABLE_ITEMS[:ZERO2W]
-
-    # Bulk PICO2
-    if(items.count(:PICO2) >= 3)
-      discount += items.count(:PICO2) * 0.50
+  def new_or_existing_product_in_checkout(product)
+    product_in_checkout = existing_product_in_checkout(product)
+    unless product_in_checkout
+      product_in_checkout = ProductInCheckout.new(product: product)
+      @items.push(product_in_checkout)
     end
 
-    return discount
+    product_in_checkout
   end
 
-  def pre_discount_total
-    @items.reduce(0.00) do |running_total, item|
-      running_total + AVAILABLE_ITEMS[item]
-    end
+  def trim_empty_items
+    @items = @items.filter { |item| item.quantity > 0 }
   end
 end

no_discount.rb

@@ -0,0 +1,5 @@
+class NoDiscount
+  def discount_for_quantity(quantity)
+    return 0.00
+  end
+end

product.rb

@@ -0,0 +1,25 @@
+require 'no_discount.rb'
+class Product
+  def initialize(price: 0.00, id:, discount: nil, description: '')
+    @price = price
+    @id = id
+    @description = description
+    @discount = discount || NoDiscount.new
+  end
+
+  def price
+    @price
+  end
+
+  def id
+    @id
+  end
+
+  def discount
+    @discount
+  end
+
+  def description
+    @description
+  end
+end

product_in_checkout.rb

@@ -0,0 +1,26 @@
+class ProductInCheckout
+  def initialize(product:, quantity: 0)
+    @product = product
+    @quantity = quantity
+  end
+
+  def product_id
+    @product.id
+  end
+
+  def quantity
+    @quantity
+  end
+
+  def add_quantity(quantity)
+    @quantity += quantity
+  end
+
+  def remove_quantity(quantity)
+    @quantity = [@quantity - quantity, 0].max
+  end
+
+  def total
+    (@product.price * quantity) - @product.discount.discount_for_quantity(@quantity)
+  end
+end

shop.rb

@@ -0,0 +1,20 @@
+class Shop
+  class DuplicateProductIdError < StandardError; end
+  def initialize
+    @products = []
+  end
+
+  def products
+    @products
+  end
+
+  def add_product(product)
+    raise DuplicateProductIdError if product_by_id(product.id)
+
+    @products.push(product)
+  end
+
+  def product_by_id(id)
+    @products.find { |product| product.id == id }
+  end
+end

test/bulk_discount_test.rb

@@ -0,0 +1,15 @@
+require "minitest/autorun"
+
+require 'bulk_discount.rb'
+
+class BulkDiscountTest < Minitest::Test
+  def test_appropriate_discount_is_applied
+    unit_discount = 0.75
+
+    discount = BulkDiscount.new(unit_discount: unit_discount, trigger_quantity: 4)
+
+    assert_equal 0.00, discount.discount_for_quantity(3)
+    assert_equal unit_discount * 4, discount.discount_for_quantity(4)
+    assert_equal unit_discount * 5, discount.discount_for_quantity(5)
+  end
+end

test/buy_x_get_one_free_discount_test.rb

@@ -0,0 +1,25 @@
+require "minitest/autorun"
+
+require 'buy_x_get_one_free_discount.rb'
+
+class BuyXGetOneFreeDiscountTest < Minitest::Test
+  def test_appropriate_discount_is_applied
+    unit_price = 4.00
+    (2..4).each do |trigger_quantity|
+      discount = BuyXGetOneFreeDiscount.new(unit_price: unit_price, trigger_quantity: trigger_quantity)
+
+      # Single discount at and above trigger quantity
+      assert_equal unit_price, discount.discount_for_quantity(trigger_quantity)
+      assert_equal unit_price, discount.discount_for_quantity(trigger_quantity + 1)
+      assert_equal unit_price, discount.discount_for_quantity(trigger_quantity + trigger_quantity - 1)
+
+
+      # No discount if less than trigger quantity
+      assert_equal 0.00, discount.discount_for_quantity(trigger_quantity - 1)
+
+      # Double discount if double trigger quantity
+      assert_equal unit_price * 2, discount.discount_for_quantity(trigger_quantity * 2)
+      assert_equal unit_price * 2, discount.discount_for_quantity((trigger_quantity * 2) + 1)
+    end
+  end
+end

test/checkout_test.rb

@@ -0,0 +1,181 @@
+require "minitest/autorun"
+
+require "checkout.rb"
+require "buy_x_get_one_free_discount.rb"
+require "bulk_discount.rb"
+
+class CheckoutTest < Minitest::Test
+  def setup
+    @shop = Shop.new
+    @shop.add_product(
+      Product.new(
+        id: 'PI5',
+        price: 60.00
+      )
+    )
+    @shop.add_product(
+      Product.new(
+        id: 'ZERO2W',
+        price: 15.00,
+        discount: BuyXGetOneFreeDiscount.new(unit_price: 15.00, trigger_quantity: 2)
+      )
+    )
+    @shop.add_product(
+      Product.new(
+        id: 'PICO2',
+        price: 4.00,
+        discount: BulkDiscount.new(unit_discount: 0.50, trigger_quantity: 3)
+      )
+    )
+    @checkout = Checkout.new(shop: @shop)
+  end
+
+  def test_that_unknown_items_cannot_be_added
+    assert_raises Checkout::InvalidItem do
+      @checkout.add(product_id: 'PI5000')
+    end
+  end
+
+  def test_that_nil_is_not_allowed_to_be_added
+    assert_raises Checkout::InvalidItem do
+      @checkout.add(product_id: nil)
+    end
+  end
+
+  def test_that_allowed_items_are_added
+    @checkout.add(product_id: 'PI5')
+    @checkout.add(product_id: 'ZERO2W')
+    @checkout.add(product_id: 'PICO2')
+
+    assert_equal({ 'PI5' => 1, 'ZERO2W' => 1, 'PICO2' => 1 }.sort, @checkout.item_quantities.sort)
+  end
+
+  def test_that_multiples_can_be_added_at_once
+    @checkout.add(product_id: 'PI5', quantity: 5)
+    assert_equal({ 'PI5' => 5 }.sort, @checkout.item_quantities.sort)
+
+    @checkout.add(product_id: 'PI5', quantity: 5)
+    assert_equal({ 'PI5' => 10 }.sort, @checkout.item_quantities.sort)
+  end
+
+  def test_that_total_is_the_sum_of_all_items_when_no_discounts_apply
+    @checkout.add(product_id: 'PI5')    # 60.00
+    @checkout.add(product_id: 'ZERO2W') # 15.00
+    @checkout.add(product_id: 'PICO2')  #  4.00
+                                        # -----
+                                        # 79.00 total
+
+    assert_equal 79.00, @checkout.total
+  end
+
+  def test_that_an_empty_checkout_total_is_zero
+    assert_equal 0.00, @checkout.total
+  end
+
+  def test_that_we_can_empty_the_checkout
+    @checkout.add(product_id: 'PI5')
+    @checkout.add(product_id: 'ZERO2W')
+    @checkout.add(product_id: 'PICO2')
+
+    refute_empty @checkout.items
+
+    @checkout.clear
+
+    assert_empty @checkout.items
+
+  end
+
+  # Discounts
+
+  def test_that_bogof_applied
+    @checkout.add(product_id: 'ZERO2W', quantity: 4) # Should remove 2 Zeros
+
+    assert_equal 30.00, @checkout.total
+  end
+
+  def test_that_bogof_applied_with_odd_number
+    @checkout.add(product_id: 'ZERO2W', quantity: 5) # Should still remove 2 Zeros, 5th is full price
+
+    assert_equal 45.00, @checkout.total
+  end
+
+  def test_that_price_is_reduced_for_3_picos
+    @checkout.add(product_id: 'PICO2', quantity: 3) # 3.50 * 3 = 10.50
+
+    assert_equal 10.50, @checkout.total
+  end
+
+  def test_that_price_is_reduced_for_more_than_3_picos
+    @checkout.add(product_id: 'PICO2', quantity: 4) # 3.50 * 4 = 14
+
+    assert_equal 14.00, @checkout.total
+  end
+
+  def test_that_price_is_not_reduced_for_less_than_3_picos
+    @checkout.add(product_id: 'PICO2', quantity: 2) # 4.00 * 2 = 8
+
+    assert_equal 8.00, @checkout.total
+  end
+
+  def test_that_both_discounts_apply_at_the_same_time
+    @checkout.add(product_id: 'ZERO2W', quantity: 4) # 30.00 with BOGOF
+    @checkout.add(product_id: 'PICO2', quantity: 3) # 10.50 with bulk discount
+
+    assert_equal (30.00 + 10.50), @checkout.total
+  end
+
+  # Item removal
+
+  def test_that_removing_an_item_not_in_the_checkout_removes_nothing
+    @checkout.add(product_id: 'PICO2')
+    @checkout.remove(product_id: 'PI5')
+
+    assert_equal({ 'PICO2' => 1 }, @checkout.item_quantities)
+  end
+
+  def test_that_items_are_removed_one_by_one
+    @checkout.add(product_id: 'ZERO2W', quantity: 4)
+    @checkout.add(product_id: 'PICO2', quantity: 3)
+
+    assert_equal({ 'ZERO2W' => 4, 'PICO2' => 3 }.sort, @checkout.item_quantities.sort)
+
+    @checkout.remove(product_id: 'ZERO2W')
+
+    assert_equal({ 'ZERO2W' => 3, 'PICO2' => 3 }.sort, @checkout.item_quantities.sort)
+
+    @checkout.remove(product_id: 'PICO2')
+
+    assert_equal({ 'ZERO2W' => 3, 'PICO2' => 2 }.sort, @checkout.item_quantities.sort)
+
+
+    @checkout.remove(product_id: 'PICO2', quantity: 2)
+
+    assert_equal({ 'ZERO2W' => 3 }.sort, @checkout.item_quantities.sort)
+  end
+
+  def test_that_total_is_correctly_recalculated_when_items_are_added
+
+    @checkout.add(product_id: 'PICO2', quantity: 2)
+
+    assert_equal 8.00, @checkout.total
+
+    @checkout.add(product_id: 'PICO2', quantity: 1) # 3 in total now, so discount applies
+
+    assert_equal 10.50, @checkout.total
+
+    @checkout.add(product_id: 'ZERO2W', quantity: 4)
+
+    assert_equal 40.50, @checkout.total
+  end
+
+  def test_that_total_is_correctly_recalculated_when_items_are_removed
+
+    @checkout.add(product_id: 'PICO2', quantity: 3)
+
+    assert_equal 10.50, @checkout.total
+
+    @checkout.remove(product_id: 'PICO2', quantity: 1) # Bulk discount no longer applies
+
+    assert_equal 8.00, @checkout.total
+  end
+end

test/product_test.rb

@@ -0,0 +1,18 @@
+require "minitest/autorun"
+
+require "product.rb"
+
+class ProductTest < Minitest::Test
+  def test_that_missing_id_raises_an_error
+    assert_raises ArgumentError do
+      Product.new()
+    end
+  end
+
+  def test_that_initializing_a_product_sets_instance_variables
+    product = Product.new(id: 'PICO2', price: 4.00)
+
+    assert_equal 'PICO2', product.id
+    assert_equal 4.00, product.price
+  end
+end

test/shop_test.rb

@@ -0,0 +1,25 @@
+require "minitest/autorun"
+
+require "shop.rb"
+require "product.rb"
+
+class ShopTest < Minitest::Test
+  def setup
+    @shop = Shop.new
+  end
+
+  def test_adding_a_product
+    pico = Product.new(id: 'PICO2', price: 4.00)
+    @shop.add_product(pico)
+
+    assert_equal [pico], @shop.products
+  end
+
+  def test_duplicate_product_id_raises_an_error
+    @shop.add_product(Product.new(id: 'PICO2'))
+
+    assert_raises Shop::DuplicateProductIdError do
+     @shop.add_product(Product.new(id: 'PICO2'))
+    end
+  end
+end

tests.rb

@@ -1,175 +0,0 @@
-require "minitest/autorun"
-
-require "./checkout.rb"
-
-class TestCheckout < Minitest::Test
-  def setup
-    @checkout = Checkout.new
-  end
-
-  def test_that_unknown_items_cannot_be_added
-    assert_raises Checkout::InvalidItem do
-      @checkout.add(:foo)
-    end
-  end
-
-  def test_that_nil_is_not_allowed_to_be_added
-    assert_raises Checkout::InvalidItem do
-      @checkout.add(nil)
-    end
-  end
-
-  def test_that_allowed_items_are_added
-    @checkout.add(:PI5)
-    @checkout.add(:ZERO2W)
-    @checkout.add(:PICO2)
-
-    assert_equal [:PI5, :ZERO2W, :PICO2].sort, @checkout.items.sort
-  end
-
-  def test_that_total_is_the_sum_of_all_items_when_no_discounts_apply
-    @checkout.add(:PI5)     # 60.00
-    @checkout.add(:ZERO2W)  # 15.00
-    @checkout.add(:PICO2)   #  4.00
-                            # -----
-                            # 79.00 total
-
-    assert_equal 79.00, @checkout.total
-  end
-
-  def test_that_an_empty_checkout_total_is_zero
-    assert_equal 0.00, @checkout.total
-  end
-
-  def test_that_we_can_empty_the_checkout
-    @checkout.add(:PI5)
-    @checkout.add(:ZERO2W)
-    @checkout.add(:PICO2)
-
-    refute_empty @checkout.items
-
-    @checkout.clear
-
-    assert_empty @checkout.items
-
-  end
-
-  # Discounts
-
-  def test_that_bogof_applied
-    4.times do
-      @checkout.add(:ZERO2W) # Should remove 2 Zeros
-    end
-
-    assert_equal 30.00, @checkout.total
-  end
-
-  def test_that_bogof_applied_with_odd_number
-    5.times do
-      @checkout.add(:ZERO2W) # Should still remove 2 Zeros, 5th is full price
-    end
-
-    assert_equal 45.00, @checkout.total
-  end
-
-  def test_that_price_is_reduced_for_3_picos
-    3.times do
-      @checkout.add(:PICO2) # 3.50 * 3 = 10.50
-    end
-
-    assert_equal 10.50, @checkout.total
-  end
-
-  def test_that_price_is_reduced_for_more_than_3_picos
-    4.times do
-      @checkout.add(:PICO2) # 3.50 * 4 = 14
-    end
-
-    assert_equal 14.00, @checkout.total
-  end
-
-  def test_that_price_is_not_reduced_for_less_than_3_picos
-    2.times do
-      @checkout.add(:PICO2) # 4.00 * 2 = 8
-    end
-
-    assert_equal 8.00, @checkout.total
-  end
-
-  def test_that_both_discounts_apply_at_the_same_time
-    4.times do
-      @checkout.add(:ZERO2W) # 30.00 with BOGOF
-    end
-
-    3.times do
-      @checkout.add(:PICO2) # 10.50 with bulk discount
-    end
-
-    assert_equal (30.00 + 10.50), @checkout.total
-  end
-
-  # Item removal
-
-  def test_that_removing_an_item_not_in_the_checkout_removes_nothing
-    @checkout.add(:PICO2)
-    @checkout.remove(:PI5)
-
-    assert_equal [:PICO2], @checkout.items
-  end
-
-  def test_that_items_are_removed_one_by_one
-    4.times do
-      @checkout.add(:ZERO2W)
-    end
-
-    3.times do
-      @checkout.add(:PICO2)
-    end
-
-    assert_equal [:ZERO2W, :ZERO2W, :ZERO2W, :ZERO2W, :PICO2, :PICO2, :PICO2].sort, @checkout.items.sort
-
-    @checkout.remove(:ZERO2W)
-
-    assert_equal [:ZERO2W, :ZERO2W, :ZERO2W, :PICO2, :PICO2, :PICO2].sort, @checkout.items.sort
-
-    @checkout.remove(:PICO2)
-
-    assert_equal [:ZERO2W, :ZERO2W, :ZERO2W, :PICO2, :PICO2].sort, @checkout.items.sort
-
-    2.times do
-      @checkout.remove(:PICO2)
-    end
-
-    assert_equal [:ZERO2W, :ZERO2W, :ZERO2W].sort, @checkout.items.sort
-  end
-
-  def test_that_total_is_correctly_recalculated_when_items_are_added
-    2.times do
-      @checkout.add(:PICO2)
-    end
-
-    assert_equal 8.00, @checkout.total
-
-    @checkout.add(:PICO2) # 3 in total now, so discount applies
-
-    assert_equal 10.50, @checkout.total
-
-    4.times do
-      @checkout.add(:ZERO2W) # 30.00 with BOGOF
-    end
-
-    assert_equal 40.50, @checkout.total
-  end
-
-  def test_that_total_is_correctly_recalculated_when_items_are_removed
-    3.times do
-      @checkout.add(:PICO2) # Bulk discount applies
-    end
-
-    assert_equal 10.50, @checkout.total
-
-    @checkout.remove(:PICO2) # Bulk discount no longer applies
-
-    assert_equal 8.00, @checkout.total
-  end
-end