Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion exercises/change/.meta/generator/change_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,25 @@

class ChangeCase < Generator::ExerciseCase
def workload
assert_equal(expected, "Change.generate(#{coins}, #{target})")
if error_expected?
handle_errors
else
assert_equal(expected, subject_of_test)
end
end

private

def handle_errors
case test_name
when 'test_cannot_find_negative_change_values'
assert_raises('Change::NegativeTargetError', subject_of_test)
else
assert_raises('Change::ImpossibleCombinationError', subject_of_test)
end
end

def subject_of_test
"Change.generate(#{coins}, #{target})"
end
end
7 changes: 6 additions & 1 deletion exercises/change/.meta/solutions/change.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
class Change
attr_reader :coins, :target

class NegativeTargetError < ArgumentError; end
class ImpossibleCombinationError < StandardError; end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like these two 👍


def initialize(coins, target)
@coins = coins.sort.reverse
@target = target
@total_change = []
end

def generate
raise NegativeTargetError if target < 0
return [] if target.zero?

calculate_change(coins, [], target)
raise ImpossibleCombinationError if total_change.none?

total_change.any? ? total_change.sort : -1
total_change.sort
end

def self.generate(coins, target)
Expand Down
14 changes: 10 additions & 4 deletions exercises/change/change_test.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'minitest/autorun'
require_relative 'change'

# Common test data version: 1.2.0 044d09a
# Common test data version: 1.3.0 258c807
class ChangeTest < Minitest::Test
def test_single_coin_change
# skip
Expand Down Expand Up @@ -45,16 +45,22 @@ def test_no_coins_make_0_change

def test_error_testing_for_change_smaller_than_the_smallest_of_coins
skip
assert_equal -1, Change.generate([5, 10], 3)
assert_raises(Change::ImpossibleCombinationError) do
Change.generate([5, 10], 3)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArgumentError is probably OK here, but what about returning a subclass which is more descriptive?

end
end

def test_error_if_no_combination_can_add_up_to_target
skip
assert_equal -1, Change.generate([5, 10], 94)
assert_raises(Change::ImpossibleCombinationError) do
Change.generate([5, 10], 94)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArgumentError is probably not appropriate here.
Maybe nil or a custom error?

end
end

def test_cannot_find_negative_change_values
skip
assert_equal -1, Change.generate([1, 2, 5], -5)
assert_raises(Change::NegativeTargetError) do
Change.generate([1, 2, 5], -5)
Copy link
Copy Markdown
Contributor

@Insti Insti Oct 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArgumentError is reasonable here. 👍

(I don't think invalid input like this is something we should be testing for, but that's an unrelated issue. (and very much a matter of opinion.))

end
end
end