HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Cash Register Kata

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
kataregistercash

Problem

The Setup

This kata is a spin on the old backpack problem. Please give feedback on both the kata itself as well as my solution. I'm not sure if the kata needs to lead the student more or if this is a good open-ended problem. My original intent with this kata was to practice using Hashes. This is why the output is required to be in the form of a hash. That detail led me to create the Change class which inherits from Hash. I think of this as an implementation detail of my solution but I'm considering making it part of the kata. Thanks for your input/reading.

The Kata

```
CashRegister Kata
Create a CashRegister class
- detail: It that is initialized with an Array of Integers
- detail: Each integer in the array represents the value of a type of coin in the cash register
- detail: You can assume that there are an infinite number of coins in the cash register
- detail: The cash register should default to 25, 10, 5 and 1 cent pieces, if no argument is provided.
- detail: The cash register should throw an error if it initialized with an argument that is not an array of integers
- example: CashRegister.new([25,10,5,1])

completed (Y|n):

Create a make_change() method
- detail: It takes an positive integer as an argument.
- detail: It returns a hash showing the smallest combination of coins which sum to that number.
- detail: If a non integer argument is passed in, make_change() should throw an error.
- detail: The hash should have an entry for every coin in the cash register.
- detail: If a coin is not used, it's value should be zero.
- example: CashRegister.new().make_change(123) evaluates to: {'25' => 4, '10' => 2, '5' => 0, '1' => 3}
- example: CashRegister.new([10,7,1]).make_change(14) evaluates to: {'10' => 0, '7' => 2, '1' => 0}

completed (Y|n):

Congratulations!
  • Create a CashRegister class 00:21:08
  • Create a make_change() method

Solution

Duck typing - the ruby language is duck-typed, and should be written that way. Checks on an object's class are frowned upon. If something wants to be an integer, and is prepared to go the distance - don't discourage it! I understand that the requirement says 'should throw an error if it initialized with an argument that is not an array of integers', so I guess for the sake of argument asking coins.is_a?(Array) is OK, but for the numbers themselves, a more rubyish way of asking would be:

raise Exception unless coins.is_a?(Array) && coins.all?(&:integer?)


I like the way you fill the @optimal_change hash, though I wish you had named your parameters better - key is an obfuscated choice. Also, you could have refactored it into a method

Having is not inheriting - Your Change class is curious - you opted for it to inherit from Hash instead of having a Hash, and then you override count to do something not very Hash-y... The names you chose for this class are very generic and obfuscated... I would have written that class like this:

class Change
  def initialize(coins = Hash.new(0))
    @coins = coins
    @coins.default = 0
  end

  # changes the value of this instance
  def add!(coin)
    @coins[coin] = @coins[coin] + 1
    self
  end

  # does not change the value of this instance
  def add(coin)
    Change.new(@coins.dup).add!(coin)
  end

  def total_value
    @coins.map { |coin, number| coin * number }.reduce(:+)
  end

  def coin_count
    @coins.values.reduce(:+)
  end
end

Code Snippets

raise Exception unless coins.is_a?(Array) && coins.all?(&:integer?)
class Change
  def initialize(coins = Hash.new(0))
    @coins = coins
    @coins.default = 0
  end

  # changes the value of this instance
  def add!(coin)
    @coins[coin] = @coins[coin] + 1
    self
  end

  # does not change the value of this instance
  def add(coin)
    Change.new(@coins.dup).add!(coin)
  end

  def total_value
    @coins.map { |coin, number| coin * number }.reduce(:+)
  end

  def coin_count
    @coins.values.reduce(:+)
  end
end

Context

StackExchange Code Review Q#42779, answer score: 3

Revisions (0)

No revisions yet.