Marketplace checkout system

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_ru

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 = 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.

StackExchange Code Review Q#146029, answer score: 5