West Midlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches#198
West Midlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches#198JanefrancessC wants to merge 3 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| Helper function for ways_to_make_change to avoid exposing the coins parameter to callers. | ||
| """ | ||
| if total == 0 or len(coins) == 0: | ||
| coins = (200, 100, 50, 20, 10, 5, 2, 1) |
There was a problem hiding this comment.
Declaring coin as a local variable means a different tuple (with identical items) is created in every recursive function call.
Why not pass the coin list through the parameter so that the function can be reused for different coin list?
When we pass a list to a function, we are only passing its reference -- the cost is negligible.
However, if the function is expected to accept different coin lists, it would not be appropriate to declare the cache as a parameter with a default value.
There was a problem hiding this comment.
I have refactored and made COINS a global variable
There was a problem hiding this comment.
Why not pass the coin list through the parameter?
| ways = ways_to_make_change_helper(total, coin_index + 1) | ||
| else: | ||
| ways = (ways_to_make_change_helper(total - coin, coin_index) + ways_to_make_change_helper(total, coin_index + 1)) |
There was a problem hiding this comment.
It seems the code on lines 30-32 could be shorten.
There was a problem hiding this comment.
The if/else structure, once flattened, breaks the flow for large inputs like the (ways_to_make_change(9176))
There was a problem hiding this comment.
One of the function calls will always happen. That would suggest it does not need to reside in an if-else block.
Learners, PR Template
Self checklist
Changelist
Completed improve code with caches exercise