Skip to content

Conversation

@AFatmaa
Copy link

@AFatmaa AFatmaa commented Jan 17, 2026

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

I improved the performance of fibonacci and making_change functions using caching (memoisation).

  • Fibonacci:
    • I added a memo dictionary to store calculated results.
    • This stops the code from calculating the same numbers again.
  • Making Change:
    • I added a global cache dictionary using tuple keys.
    • I added a specific check (if len(coins) == 1) to solve the problem with math instead of looping.

Questions

I don’t have any questions, thank you!

@AFatmaa AFatmaa 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. labels Jan 17, 2026
@@ -1,22 +1,38 @@
from typing import List

cache = {}
Copy link

Choose a reason for hiding this comment

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

Better to make cache local inside ways_to_make_change(), and define
ways_to_make_change_helper() as an inner function inside ways_to_make_change().

Since that's not the purpose of this exercise, change is optional.

Helper function for ways_to_make_change to avoid exposing the coins parameter to callers.
"""
# We use tuple(coins) because lists cannot be keys.
key = (total, tuple(coins))
Copy link

@cjyuan cjyuan Jan 23, 2026

Choose a reason for hiding this comment

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

From line 45, we know coins can only be one of the following 9 lists:

[200, 100, 50, 20, 10, 5, 2, 1]
[100, 50, 20, 10, 5, 2, 1]
[50, 20, 10, 5, 2, 1]
...
[1]
[]

We could further improve the performance if we can

  • avoid repeatedly creating the same sub-lists at line 45, and
  • create key as (total, a_value_identifiying_one_of_the_sublists) instead of as (total, tuple(coins))

I don't think this exercise expects trainees to optimize the code to this level. So change is optional.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jan 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants