patternrubyMinor
Cash Register Kata
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!
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
I like the way you fill the
Having is not inheriting - Your
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 methodHaving 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
endCode 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
endContext
StackExchange Code Review Q#42779, answer score: 3
Revisions (0)
No revisions yet.