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' #1399

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

'Solution' #1399

wants to merge 5 commits into from

Conversation

Dizatvarska
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.


Hello! 😊 It looks like there are a few critical issues that need to be addressed before we can approve this pull request. Let's go through them:

  1. Decorator Functionality: The cache decorator is not functioning as expected. It should store the results of function calls with different arguments and return cached results for repeated arguments. Currently, it simply calls the function and returns its result without caching, which is a core requirement of the task.

  2. Output Messages: The decorator should print specific messages ("Getting from cache" or "Calculating new result") based on whether it is returning a cached result or computing a new one. This functionality is missing, which is important for understanding the behavior of the decorator.

  3. Handling Multiple Functions: The decorator should be able to handle multiple functions simultaneously, ensuring that cached results are managed separately for each function. This aspect is not addressed in the current implementation.

  4. Return Type of wrapper Function: The return type of the wrapper function should match the return type of the decorated function. Consider changing -> None to -> Any or the specific return type of func.

These issues significantly impact the functionality and core requirements of the task. Once these are resolved, your code will be much closer to meeting the task requirements. Keep up the great work, and don't hesitate to reach out if you need any help! 🚀

✨ 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,8 @@


def cache(func: Callable) -> Callable:
# Write your code here
pass
def wrapper(*args, **kwargs) -> None:

Choose a reason for hiding this comment

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

The return type of the wrapper function is currently None, but it should match the return type of the function it decorates. Consider changing -> None to -> Any or the specific return type of func.

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! 🎉 While your current implementation of the cache decorator in app/main.py is a great start, there are a few critical areas that need improvement to fully meet the task requirements:

  1. Caching Functionality: The core requirement of caching the results of function calls with different arguments is missing. Ensure that the decorator stores results and returns cached results for repeated arguments.

  2. Output Messages: The decorator should print specific messages: "Getting from cache" when returning a cached result and "Calculating new result" when performing a new computation. This will help in understanding the behavior of the caching mechanism.

  3. Handling Multiple Functions: The decorator should be capable of managing cached results separately for each function it decorates. This aspect is crucial for the correct functionality of the cache.

  4. Return Type of wrapper Function: Consider aligning the return type of the wrapper function with the return type of the decorated function for better type consistency.

These improvements are essential for the functionality of your code. Please review the comments and address these issues to enhance your solution. Remember, learning from these points will be beneficial for your growth as a developer. Keep up the good work and continue to improve! 🚀

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

app/main.py Outdated
Comment on lines 5 to 7
def wrapper(*args, **kwargs) -> Any:
result = func(*args, **kwargs)
return result

Choose a reason for hiding this comment

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

The wrapper function should implement caching logic. It should store results of function calls and return cached results for repeated arguments. Additionally, it should print 'Getting from cache' if a cached result is used, or 'Calculating new result' if a new computation is performed. This functionality is currently missing.

app/main.py Outdated
def wrapper(*args, **kwargs) -> Any:
result = func(*args, **kwargs)
return result
return wrapper

Choose a reason for hiding this comment

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

The cache decorator should handle multiple functions simultaneously, ensuring that cached results are managed separately for each function. This aspect is not addressed in the current implementation.

Copy link
Contributor

@vsmutok vsmutok left a comment

Choose a reason for hiding this comment

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

Tests Failed!

app/main.py Outdated
Comment on lines 8 to 14
for _ in range(numero):
results.append(func(*args, **kwargs))
for result in results:
if results.count(result) > 1:
print(f"Getting from cache {result}")
else:
print(f"Calculating new result {result}")
Copy link
Contributor

Choose a reason for hiding this comment

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

try to solve this task without using loops

Copy link
Contributor

@vsmutok vsmutok left a comment

Choose a reason for hiding this comment

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

Tests Failed!
Before submitting the task for review, ensure that your code passes all tests.

def cache(func: Callable) -> Any:
results = {}

def wrapper(*args) -> Any:
Copy link
Contributor

Choose a reason for hiding this comment

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

what if the function will also accept key arguments?
you should also consider this point

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.

3 participants