Marketplace checkout system

Question

I was given the following code challenge as part of an interview process, unfortunately I did not get through to the next stage. Any advice on how I could improve would be very much appreciated.

Here is the spec:

Our client is an online marketplace, here is a sample of some of the products available on our site:

Product code | Name | Price

001 | Lavender heart | £9.25

002 | Personalised cufflinks | £45.00

003 | Kids T-shirt | £19.95

If you spend over £60, then you get 10% off of your purchase. If you buy 2 or more lavender hearts then the price drops to £8.50. Our check-out can scan items in any order, and because our promotions will change, it needs to be flexible regarding our promotional rules.

The interface to our checkout looks like this (shown in Ruby):

co = Checkout.new(promotional_rules)

co.scan(item) co.scan(item) price = co.total

Implement a checkout system that fulfills these requirements. Do this outside of any frameworks. We’re looking for candidates to demonstrate their knowledge of TDD.

Test data

Basket: 001,002,003

Total price expected: £66.78

Basket: 001,003,001

Total price expected: £36.95

Basket: 001,002,001,003

Total price expected: £73.76

Here is my code:

Product:

#/lib/product.rb    

class Product
  class << self
    attr_reader :price
  end
end

Kids T-Shirt:

#/lib/products/kids_t_shirt.rb

require './lib/product'

class KidsTShirt < Product
  @price = 19.95
end

Lavender Heart:

#/lib/products/lavender_heart.rb

require './lib/product'

class LavenderHeart < Product
  @price = 9.25
end

Personalised Cufflinks:

#/lib/products/personalised_cufflinks.rb

require './lib/product'

class PersonalisedCufflinks < Product
  @price = 45
end

Checkout

#/lib/checkout.rb

class Checkout
  def initialize(promotional_rules)
    @items = {}
    @promotional_rules = promotional_rules
  end

  def scan(item)
    quantity = @items[item.class]
    @items[item.class] = quantity ? quantity + 1 : 1
  end

  def total
    @promotional_rules.total(@items).round(2)
  end
end

Promotional Rules

#/lib/promotional_rules.rb

require './lib/products/kids_t_shirt'
require './lib/products/lavender_heart'
require './lib/products/personalised_cufflinks'

class PromotionalRules
  def total(items)
    current_total = 0

    items.each do |item, quantity|
      current_total += item_pricing(item, quantity)
    end

    total_pricing(current_total)
  end

  private

  def item_pricing(item, quantity)
    if item.eql?(LavenderHeart)
      lavender_heart_pricing(item, quantity)
    else
      general_item_pricing(item, quantity)
    end
  end

  def total_pricing(total)
    total > 60 ? total * 0.9 : total
  end

  def lavender_heart_pricing(item, quantity)
    quantity >= 2 ? 8.5 * quantity : item.price * quantity
  end

  def general_item_pricing(item, quantity)
    item.price * quantity
  end
end

Checkout Spec

#/spec/checkout_spec.rb

require 'rspec'
require './lib/products/kids_t_shirt'
require './lib/products/lavender_heart'
require './lib/products/personalised_cufflinks'
require './lib/checkout'
require './lib/promotional_rules'

describe 'Checkout' do
  describe '#total' do
    context 'when lavender hearts/over £60 promotional rules are present' do
      before do
        @lavender_heart = LavenderHeart.new
        @personalised_cufflinks = PersonalisedCufflinks.new
        @kids_t_shirt = KidsTShirt.new
        @promotional_rules = PromotionalRules.new
      end

      it 'provides the total cost of the items checked out' do
        co = Checkout.new(@promotional_rules)
        co.scan(@lavender_heart)
        co.scan(@personalised_cufflinks)
        co.scan(@kids_t_shirt)

        expect(co.total).to eq(66.78)
      end

      it 'activates a discount when two or more lavender hearts are bought' do
        co = Checkout.new(@promotional_rules)
        co.scan(@lavender_heart)
        co.scan(@kids_t_shirt)
        co.scan(@lavender_heart)

        expect(co.total).to eq(36.95)
      end

      it 'activates a discount when over £60 is spent' do
        co = Checkout.new(@promotional_rules)
        co.scan(@lavender_heart)
        co.scan(@personalised_cufflinks)
        co.scan(@lavender_heart)
        co.scan(@kids_t_shirt)

        expect(co.total).to eq(73.76)
      end
    end
  end
end

Answer 1

Product

Your Product class and it's descendants are coded in a really weird way. What would be wrong with:

class Product
  attr_reader :price

  def initialize(price)
    @price = price
  end
end

You don't really need a class for every product - those classes don't do anything. All they differ in is an instance variable, so a product could very well be an instance of a class.

If having separate class for each product would prove beneficial in any way, you should still set price in initialize. Absolutely no need for any singleton class shenanigans.

Checkout

Ruby's Hashes allow you to specify default value. No need for scan to be that unreadable.

class Checkout
  def initialize(promotional_rules)
    @items = Hash.new(0)
    @promotional_rules = promotional_rules
  end

  def scan(item)
    @items[item.class] += 1
  end

  def total
    @promotional_rules.total(@items).round(2)
  end
end

PromotionalRules

They specifically asked you to make the system flexible regarding promotional rules. You didn't - anyone who would wan't to add a new promotion, would need to edit PromotionalRules class. What your potential employee would expect here, is a system to easily define new promotions. Example interface:

class MarchPromotions < PromotionalRules
  discount_for(kids_t_shirt) do |quantity|
    quantity >= 2 ? 0.85 : 1.0
  end
end

I won't go into the details here, as it could very well look much different. The main point is, someone should be able to define new promotion rules without hacking your code, but by specifying them outside of it - be it in another .rb file like in my example, or by listing rules in YAML or XML - that's up to you.