Skip to content

West Midlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches#198

Open
JanefrancessC wants to merge 3 commits into
CodeYourFuture:mainfrom
JanefrancessC:caches
Open

West Midlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches#198
JanefrancessC wants to merge 3 commits into
CodeYourFuture:mainfrom
JanefrancessC:caches

Conversation

@JanefrancessC

Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

Completed improve code with caches exercise

@github-actions

This comment has been minimized.

@JanefrancessC JanefrancessC changed the title WestMidlands | 26-SDC-Mar | Chioma Okeke | Sprint 2| Improve code with caches WestMidlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches Jul 1, 2026
@github-actions

This comment has been minimized.

@JanefrancessC JanefrancessC changed the title WestMidlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches West Midlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches Jul 1, 2026
@github-actions

This comment has been minimized.

@JanefrancessC JanefrancessC changed the title West Midlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches West Midlands | 26-SDC-Mar | Chioma Okeke | Sprint 2 | Improve code with caches Jul 1, 2026
@JanefrancessC JanefrancessC added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Complexity The name of the module. labels Jul 1, 2026
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have refactored and made COINS a global variable

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why not pass the coin list through the parameter?

Comment on lines +30 to +32
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))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It seems the code on lines 30-32 could be shorten.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The if/else structure, once flattened, breaks the flow for large inputs like the (ways_to_make_change(9176))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

One of the function calls will always happen. That would suggest it does not need to reside in an if-else block.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 1, 2026
@JanefrancessC JanefrancessC added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 1, 2026
@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Complexity The name of the module. Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants