Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solution #1497

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Solution #1497

wants to merge 3 commits into from

Conversation

Liliia2000
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Thank you for your hard work on this pull request! 😊 While your implementation of the cache decorator function is mostly correct and effectively caches the results of function calls, there is a critical issue that needs to be addressed before we can approve it. The dict_funcs dictionary is currently shared across all instances of the decorated function, which could lead to unexpected behavior if multiple functions are decorated with cache. To resolve this, please move dict_funcs inside the inner function to ensure each decorated function has its own cache. Once this change is made, your solution will be much more robust and ready for approval. Keep up the great work! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

app/main.py Outdated
@@ -2,5 +2,15 @@


def cache(func: Callable) -> Callable:
# Write your code here
pass
dict_funcs = {}

Choose a reason for hiding this comment

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

The dict_funcs dictionary is defined at the top level of the cache function, which means it will be shared across all instances of the decorated function. This could lead to unexpected behavior if multiple functions are decorated with cache. Consider moving dict_funcs inside the inner function to ensure each decorated function has its own cache.

@Liliia2000 Liliia2000 changed the title Solution Amended solution Jan 18, 2025
@Liliia2000 Liliia2000 changed the title Amended solution Solution Jan 18, 2025
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Thank you for your submission! 🎉 Your implementation of the cache decorator function shows great effort, but there are a few critical issues that need attention to ensure the code functions correctly:

  1. Shared Cache Across Functions: The dict_funcs dictionary is currently shared across all instances of the decorated function. This can lead to incorrect caching behavior. To fix this, move the dict_funcs initialization inside the inner function to ensure each decorated function has its own cache.

  2. Incorrect Use of Function Signature: The cache decorator function signature should not include **dict_funcs. Instead, initialize dict_funcs within the decorator to store the cache.

  3. Printing Statements: You've correctly implemented the printing statements for cache hits and misses, which is great!

Please address these issues to improve your code. This decision to approve is made to help you move forward with your learning. However, it's crucial to fix these issues in future implementations to avoid similar problems. Keep up the good work and continue learning from these experiences! 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

app/main.py Outdated
def cache(func: Callable) -> Callable:
# Write your code here
pass
def cache(func: Callable, **dict_funcs) -> Callable:

Choose a reason for hiding this comment

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

The dict_funcs parameter should not be part of the cache function signature. Instead, initialize dict_funcs as an empty dictionary inside the inner function to ensure each decorated function has its own cache.

app/main.py Outdated
# Write your code here
pass
def cache(func: Callable, **dict_funcs) -> Callable:
def inner(*args, **dict_of_element) -> Callable:

Choose a reason for hiding this comment

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

Remove dict_of_element from the parameters of the inner function as it is not needed. You can define it within the function body.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants