Monday, September 6, 2010

Code Kata Nine: Supermarket Checkout, Part A (Just Make It Work!)

   Wow, I really managed to milk Conflicting Objectives for quite a run!  Now let's move on to Dave Thomas's ninth code kata: Supermarket Checkout.  Follow the link to go read up on it.  I'll wait.

   . . .

   Ah, you're back.  Good.  Now, I want to make a little twist on the basic kata, as I believe this will be more instructive for the readers.  First I am writing as though I am a naive but competent programmer, who actually fell for it when the customer swore up and down that the input format would never ever change.  Of course, said programmer is also under pressure from The Boss (no I don't mean Bruce Springsteen) to just get the thing working and out the door.

   The initial data format was not chosen with much thought in mind.  That's not really part of the exercise, only abstracting it later.  But that, as I said, is for later.  Next time, I'll abstract it... as though the customer is, as a seasoned programmer would have predicted, changing it for the Nth time.

#! /usr/bin/ruby

# Dave Thomas Code Kata #9: Supermarket Checkout
# See http://codekata.pragprog.com/2007/01/kata_nine_back_.html
# Solution by Dave Aronson
# VERSION A: Just get it working

# This one is a bit tricky requirements-wise.  He doesn't say what
# format the rules are in!  My first cut at this will use YAML-ish
# input:
#
# sku
# quantitytotal
# quantitytotal
# sku
# quantitytotal
#
# See the definition of the RULES array for an example.
#
# For this one I am JUST going to get it working.  It will be as
# though this format was dictated by the customer and they swore up
# and down it would never ever change.  Of course you realize that in
# the real world, that means that the format is pretty much GUARANTEED
# to change, but this version will demonstrate how a naive (but
# otherwise competent) programmer would design it while holding that
# belief.
#
# Next time, in Version B, I will pretend as though the customer has
# changed his mind (as a seasoned programmer would predict), TWICE, so
# I'll be trying to make it easier for the NEXT time he does so.


class CheckOut

  def initialize rules
    cur_sku = ''
    @prices = {}
    rules.each do |line|
      line.chomp!  # just in case; not needed w/ my test, but....
      if line[0..0] == "\t"
        qty, tot = line[1..-1].split "\t"
        @prices[cur_sku] = {} if @prices[cur_sku].nil?
        @prices[cur_sku][qty.to_i] = tot.to_i
      else cur_sku = line
      end
    end
    @items = {}
  end

  def scan sku
    @items[sku] = 0 if @items[sku].nil?
    @items[sku] += 1
  end

  def total
    tot = 0
    @items.each_pair do |sku, qty|
      while qty > 0 do
        deal_qty, deal_price = find_best_deal sku, qty
        num_deals = qty / deal_qty
        tot += num_deals * deal_price
        qty -= num_deals * deal_qty
      end
    end
    tot
  end

  private

  # Find the best deal on a given quantity of a given SKU.
  # API design point: make args "sku, qty" or "qty, sku"?  The latter
  # is more like the above description, but the programmer writing a
  # call to this is likely to already be dealing with it the former
  # way (as a key and value).
  def find_best_deal sku, qty
    deal = nil
    @prices[sku].each_pair do |rule_qty, rule_tot|
      if rule_qty > qty then
        break
      else
        # Inefficient to keep overwriting this,
        # but shouldn't happen too much....
        deal = [rule_qty, rule_tot]
      end
    end
    puts "ERROR: NO PRICE FOUND for #{qty} of #{sku}!" if deal.nil?
    deal
  end

end


RULES = [
  "A",
  "\t1\t50",
  "\t3\t130",
  "B",
  "\t1\t30",
  "\t2\t45",
  "C",
  "\t1\t20",
  "D",
  "\t1\t15",
]


# TESTS, supplied by Dave Thomas

require 'test/unit'

class TestPrice < Test::Unit::TestCase

  def price(goods)
    co = CheckOut.new(RULES)
    goods.split(//).each { |item| co.scan(item) }
    co.total
  end

  def test_totals
    assert_equal(  0, price(""))
    assert_equal( 50, price("A"))
    assert_equal( 80, price("AB"))
    assert_equal(115, price("CDBA"))

    assert_equal(100, price("AA"))
    assert_equal(130, price("AAA"))
    assert_equal(180, price("AAAA"))
    assert_equal(230, price("AAAAA"))
    assert_equal(260, price("AAAAAA"))

    assert_equal(160, price("AAAB"))
    assert_equal(175, price("AAABB"))
    assert_equal(190, price("AAABBD"))
    assert_equal(190, price("DABABA"))
  end

  def test_incremental
    co = CheckOut.new(RULES)
    assert_equal(  0, co.total)
    co.scan("A");  assert_equal( 50, co.total)
    co.scan("B");  assert_equal( 80, co.total)
    co.scan("A");  assert_equal(130, co.total)
    co.scan("A");  assert_equal(160, co.total)
    co.scan("B");  assert_equal(175, co.total)
  end
end

   So now it's your turn.  Please hold off until next time on the topic of abstracting the rules.  The rest of it may well be besides the main point of the kata, but I'd still be interested to hear how you would have done the rest of it differently, and why.