Recursive tree terminates function prematurely

Question

I am trying to define a function that returns a list of all combinations of legal coins that amount to a given sum. Let's say the given sum were 10 and the legal coins were 5, 2, 3. In that case the function should return:

[[2, 2, 2, 2, 2], [3, 3, 2, 2], [5, 3, 2], [5, 5]]

I've managed to create a recursive function that gives me the correct result, but by printing the correct answers on separate lines and mixing in a bunch of None's.

How can I return the correct answer to a list of legal solutions without terminating the function prematurely. I know that my function is ineffective. I can think of some improvements myself, and will implement them later.

Here's my code:

def count_change(tot_amount, coins, used_coins=[]):

    # Sort coins and get rid of coins that are too big
    if len(used_coins) == 0:
        coins = [x for x in sorted(coins) if x <= tot_amount]

    for coin in coins:
        # If the first condition holds I want to add the printed
        # statement to a list of solution instead of printing
        if tot_amount - sum(used_coins) == coin:
            print(used_coins + [coin])
        elif tot_amount - sum(used_coins) > coin:
            print(count_change(tot_amount,
                               [x for x in coins if x <= coin],
                               used_coins + [coin]))

print(count_change(10, [5, 2, 3]))

This is the output:

[2, 2, 2, 2, 2]
None
None
None
None
None
None
None
[3, 3, 2, 2]
None
None
None
None
None
None
[5, 3, 2]
None
[5, 5]
None
None

PM 2Ring · Accepted Answer · 2017-12-03T13:17:46.263

You just need to turn your function into a generator, by replacing those print calls with yield.

I've also changed the default of used_coins to None, since you don't really want a default mutable argument here. See “Least Astonishment” and the Mutable Default Argument for details.

def count_change(tot_amount, coins, used_coins=None):
    # Sort coins and get rid of coins that are too big
    if used_coins is None:
        used_coins = []
        coins = [x for x in sorted(coins) if x <= tot_amount]

    for coin in coins:
        # If the first condition holds we have a valid combination 
        if tot_amount - sum(used_coins) == coin:
            yield used_coins + [coin]
        # Otherwise, if this coin is small enough, recurse to find combinations
        # that use this coin in addition to the existing used_coins
        elif tot_amount - sum(used_coins) > coin:
            yield from count_change(tot_amount,
                               [x for x in coins if x <= coin],
                               used_coins + [coin])

for t in count_change(10, [5, 2, 3]):
    print(t)

output

[2, 2, 2, 2, 2]
[3, 3, 2, 2]
[5, 3, 2]
[5, 5]

If you do actually need a list of the solutions, then simply run the generator into a list, like this:

seq = list(count_change(10, [5, 2, 3]))
print(seq)

output

[[2, 2, 2, 2, 2], [3, 3, 2, 2], [5, 3, 2], [5, 5]]

Recursive tree terminates function prematurely

1 Answers1

Linked