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

• Mast: For TDD, I'd expect a lot more tests.
• user3496065: Agreed, I only tested the test cases they gave, which was foolish, do you have any other advice?

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.

• user3496065: Thank you for the advice, I have another code test to do that this will help with!