Ruby [Ruby Quiz] 针对代码片段，提点改进意见吧

race · 2012年03月04日 · 最后由 Rei 回复于 2012年03月04日 · 3809 次阅读

最近面试，要求解决如下小问题模拟超市收银台的工作流程，商品价格可以单价和 package 价格例如：

Product Code | Price

A | $2.00 each or 4 for $7.00 B | $12.00 C | $1.25 or $6 for a six pack D | $0.15 实现工作流如下： terminal.setPricing(...) terminal.scan("A") terminal.scan("C") ... etc. result = terminal.total 显示结果： Scan these items in this order: ABCDABAA; Verify the total price is $32.40. Scan these items in this order: CCCCCCC; Verify the total price is $7.25. Scan these items in this order: ABCD; Verify the total price is $15.40. 我的代码如下：

class Terminal
  attr_accessor :item_types, :item_sequence

  def initialize
    self.item_types = []
    self.item_sequence = []
  end

  #input product name and price.
  #can be only unit price or both unit price and volume price
  #
  #@terminal.set_price 'A', 3.00
  #@terminal.set_price 'A', 3.00, 5, 13.00
  def set_price p_code, *price
    p_class = item_exists?(p_code) ? eval("#{p_code}") : create_item(p_code) 
    p_class.send :unit_price=, price[0]
    p_class.send :volume_price=, {amount: price[1], v_price: price[2]} unless price[1].nil? || price[2].nil? 
  end

  def scan p_code
    eval "#{p_code}.new"
    self.item_sequence << p_code
  end

  def total_cost
    self.item_types.inject(0.0){|sum,item| sum += Item.cost(item)}
  end

  private
  def create_item p_code
    Object.const_set p_code, Class.new( Item )
    p_class = eval "#{p_code}"
    p_class.amount = 0
    p_class.scaned_items = []
    self.item_types << p_class
    p_class
  end
  def item_exists? p_name
    eval "defined?(#{p_name}) && #{p_name}.is_a?(Class)"
  end
end

class Item
  class << self; attr_accessor :unit_price, :volume_price, :amount, :scaned_items; end
  attr_accessor :item_name

  def initialize
    self.class.amount += 1
    self.class.scaned_items << self
    self.item_name = "#{self.class}_#{self.class.amount}" 
  end

  protected
  def self.cost item
    return 0 if item.amount == 0
    if item.volume_price.nil?
      item.amount * item.unit_price
    else
      item.unit_price * (item.amount % item.volume_price[:amount]) + 
      item.volume_price[:v_price] * (item.amount / item.volume_price[:amount]).to_i
    end
  end
end

Rei #1 2012年03月04日

eval 得不必要，Hash 就能解决

vkill #2 2012年03月04日

看着有点别扭

def initialize
  @item_types = []
  @item_sequence = []
end

def set_price(p_code, *price)
  #something
end

eval 楼上说了

# 这个 Terminal 里 set_price 感觉怪，这个是 Item 里的事情，和终端没关系吧？ 
p_class.send :unit_price=

race #3 2012年03月04日

@vkill 前两个改过来看着是要顺眼 eval 的要改成什么样呢？我的本意只是希望，A，B，C，这些商品能够，独立成 Item 的子类。然后，在 set_price 的时候，动态定义一个产品。不 set_price 就不定义产品然后，在 scan 的时候，生成对应产品一个实例。就是说，可以用 A.amount A.cost 呵呵，看着有道理，再指点指点吧

kenshin54 #4 2012年03月04日

如果是我的话，快速解决问题，不会动用那么多类，毕竟这是面试。

kenshin54 #5 2012年03月04日

class Terminal
  def initialize
    @total, @sequence, @products = 0, "", Hash.new(0)
  end

  def set_pricing(pricings)
    @pricings = pricings
  end

  def scan(product)
    return if @pricings[product] == nil
    @products[product] += 1
    @sequence << product
    if reach_packing?(product)
      @total += @pricings[product][:pack][:money]
      @products[product] = 0
    end
  end

  def result
    @products.each {|k, v| @total += @pricings[k][:each] * v}
    p "Scan these items in this order: #@sequence; Verify the total price is $#@total."
  end

  private
  def reach_packing?(product)
    @pricings[product][:pack] && @pricings[product][:pack][:count] == @products[product]
  end
end

terminal = Terminal.new
terminal.set_pricing({
  "A" => {:each => 2.0, :pack => {:count => 4, :money => 7.0}},
  "B" => {:each => 12.0},
  "C" => {:each => 1.25, :pack => {:count => 6, :money => 6.0}},
  "D" => {:each => 0.15}
})

terminal.scan("A")
terminal.scan("B")
terminal.scan("C")
terminal.scan("D")
terminal.scan("A")
terminal.scan("B")
terminal.scan("A")
terminal.scan("A")
terminal.result

贴个代码，虚心求教

race #6 2012年03月04日

@kenshin54 哦，我没说清楚，不是 on site 面试，是面试完了，给个 puzzle 回去不限制时间，（当然，太长时间人家也会考虑）主要看什么代码风格，代码结构，思路是否清晰，有没有面向对象思路，有没有，BDD（我写 minitest 啦）之类。

require 'minitest/spec'
require 'minitest/autorun'
require 'terminal_source'

describe Terminal do
  before do
    @terminal = Terminal.new
  end

  it "should accept to set only unit price" do
    @terminal.set_price 'M', 9.0
    M.unit_price.must_equal 9.0
    @terminal.total_cost.must_be_close_to 0
  end

  it "should also accept to set unit and pack price" do
    @terminal.set_price 'N', 9.0, 3, 25.0
    N.unit_price.must_equal 9.0
    N.volume_price[:v_price].must_equal 25.0
    N.volume_price[:amount].must_equal 3
  end

  it "should respond to scan after set_price" do
    @terminal.must_respond_to :scan
  end

  it "cost and item sequence should be 0 and empty" do
    @terminal.total_cost.must_be_close_to 0
    @terminal.item_sequence.must_be_empty
  end
end

describe Item do
  before do
    @terminal = Terminal.new
    @terminal.set_price 'P', 9.0
    @terminal.scan 'P'
  end

  it "should create Item class" do
    P.scaned_items.first.must_be_kind_of Item
  end

  it "should create certain sequence after scan" do
    @terminal.set_price 'Q', 8.0, 5, 35.0
    @terminal.scan 'Q'
    @terminal.scan 'P'
    @terminal.scan 'P'
    @terminal.item_types.must_include Q
    @terminal.item_sequence.must_equal %w(P Q P P)
    @terminal.item_sequence.join().must_equal "PQPP"
  end
end

describe "When terminal and price have been set" do
  it "should have right item scan sequence" do
    @terminal = Terminal.new
    @terminal.set_price 'A', 2.00, 4, 7.00
    @terminal.set_price 'B', 12.00
    @terminal.set_price 'C', 1.25, 6, 6.00
    @terminal.set_price 'D', 0.15
    %w(A B C D A B A A).each{|x| @terminal.scan x}
    @terminal.item_sequence.join.must_equal "ABCDABAA"
    @terminal.total_cost.must_equal 32.40
  end

end

kenshin54 #7 2012年03月04日

#6 楼 @race 哈哈我写完的时候还在想，要不要写个测试，最后还是算了

Rei #8 2012年03月04日

class Terminal
  # Example:
  # Terminal.set_pricings({
  #   "A" => {1 => 2.0, 4 => 7.0},
  #   "B" => {1 => 12.0},
  #   "C" => {1 => 1.25, 6 => 6.0},
  #   "D" => {1 => 0.15}
  # })
  def self.set_pricings(pricings)
    @pricings = pricings
  end

  def self.pricings
    @pricings
  end

  def initialize
    @products = {}
  end

  def scan(name)
    @products[name] = @products[name].to_i + 1
  end

  def total
    result = 0
    @products.each do |name, number|
      result += self.class.item_cost(name, number)
    end
    result
  end

  def self.item_cost(name, number)
    pricings[name].keys.sort.reverse.inject(0) do |sum, package|
      if number / package > 0
        sum += ((number / package) * pricings[name][package])
        number = number % package
      end
      sum
    end
  end
end

require 'test/unit'
require 'terminal'

class TerminalTest < Test::Unit::TestCase
  def setup
    Terminal.set_pricings({
      "A" => {1 => 2.0, 4 => 7.0},
      "B" => {1 => 12.0},
      "C" => {1 => 1.25, 6 => 6.0},
      "D" => {1 => 0.15}
    })
  end

  def test_should_get_cost
    terminal = Terminal.new
    terminal.scan 'A'
    assert_equal 2.0, terminal.total
    terminal.scan 'A'
    assert_equal 4.0, terminal.total
    2.times{ terminal.scan 'A' }
    assert_equal 7.0, terminal.total
  end

  def test_multi_item_cost
    assert_mult_scan_cost_equal 32.40, 'ABCDABAA'
    assert_mult_scan_cost_equal 7.25, 'CCCCCCC'
    assert_mult_scan_cost_equal 15.40, 'ABCD'
  end

  def assert_mult_scan_cost_equal(expected, names)
    terminal = Terminal.new
    names.each_char do |name|
      terminal.scan name
    end
    assert_equal expected, terminal.total
  end
end

xzgyb #9 2012年03月04日

来个不一样的，仿照 Agile Web Development with Rails

product.rb

class Product
    attr_accessor  :name, :unit_price, :pack_count, :pack_price

    def initialize(name, unit_price, pack_count, pack_price)
        @name, @unit_price, @pack_count, @pack_price = 
            name, unit_price, pack_count, pack_price
    end

    def ==(other)
        @name == other.name
    end

    def self.products_data
        @@products_data ||= {}
    end

    def self.add(name, unit_price, pack_count = nil, pack_price = nil)
        self.products_data[name] = Product.new(name, unit_price, pack_count, pack_price)
    end

    def self.find_by_name(name)     
      self.products_data[name]
    end
end

cart_item.rb

require './product'

class CartItem
  attr_reader :product, :quantity

  def initialize(product)   
     @product  = product
     @quantity = 1
   end

    def increment_quantity
        @quantity += 1
    end

    def price
        if @product.pack_price.nil?
            @product.unit_price * @quantity
        else
            pack_items_count   = @quantity / @product.pack_count
            rest_quantity = @quantity - pack_items_count * @product.pack_count

            @product.pack_price * pack_items_count + 
              @product.unit_price * rest_quantity
        end     
    end 
end

cart.rb

require './cart_item'

class Cart
    attr_reader :items

    def initialize
        @items = []
    end

    def add_product(product)
        current_item = @items.find { |item| item.product == product }
        if current_item
            current_item.increment_quantity
        else
            @items << CartItem.new(product)
        end
    end

    def total_cost
        @items.inject(0) { |sum, item| sum += item.price}
    end

end

terminal.rb

require './product'
require './cart'

class Terminal
    def initialize
        @cart = Cart.new
    end

    def set_price(params)
        Product.add(params[:name],
                    params[:unit_price],
                    params[:pack_count],
                    params[:pack_price])
    end

    def scan(p_code)
        product = Product.find_by_name(p_code)

        raise "Product #{p_code} not found!" if product.nil?

        @cart.add_product(product)
    end

    def total_cost
        @cart.total_cost
    end
end

if $0 == __FILE__
    terminal = Terminal.new

    terminal.set_price(name:'A', unit_price: 2, pack_count: 4, pack_price: 7)
    terminal.set_price(name:'B', unit_price: 12)
    terminal.set_price(name:'C', unit_price: 1.25, pack_count: 6, pack_price: 6)
    terminal.set_price(name:'D', unit_price: 0.15)

    terminal.scan('A')
    terminal.scan('A')
    terminal.scan('A')
    terminal.scan('A')
    terminal.scan('B')
    terminal.scan('C')

    puts "Total cost #{terminal.total_cost}"
end

race #10 2012年03月04日

@Rei 调了一会儿，明白你说的 HASH 方案了，确实会简洁。算 total cost 也有一套，容易扩展，先大 package 再小 package BTW: 我跑测试要 require 改 require_relative 你 ruby1.8？

race #11 2012年03月04日

@xzgyb 你这边看起来更侧重，代码结构设计，那个重写产品判断，改产品名判断挺有意思。结构是要清晰一些创建 terminal 的时候，初始化 Cart add_product 的时候，判断是否有该 Item，否的话创建

Rei #12 2012年03月04日

#10 楼 @race 我是这样跑的

ruyb -I. terminal_test.rb

需要登录后方可回复, 如果你还没有账号请注册新账号