Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are two issues that affect both correctness and explicit checklist requirements. In app/main.py, inside your decorator (around where you define cash = {} and build the lookup key), the cache key currently uses only args and ignores kwargs, so calls like func(1, a=2) and func(1, a=3) will incorrectly share or miss cache entries; the task explicitly requires caching results for different arguments, including when the number and form of arguments differ. To fix this, build a composite, hashable key that includes both positional and keyword arguments, for example key = (args, frozenset(kwargs.items())), and then use key for storing and retrieving from the cache dictionary. Also in app/main.py (same decorator block, roughly lines 5–15), the variable name cash is a typo and violates the checklist item "Use descriptive and correct variable names" [CHECKLIST ITEM #3]; please rename it to something clear like cache or cache_store. Once you address these two points, your decorator will correctly distinguish calls that differ only in keyword arguments and will align with the naming requirements, while the rest of your logic (per-function storage and printed messages) already matches the task well. As a minor, optional improvement, you could wrap the inner function with functools.wraps and add a short docstring noting that all arguments must be hashable, but these are not required for acceptance.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| def wrapper(*args, **kwargs) -> Any: | ||
| if args in cash: | ||
| print("Getting from cache") | ||
| return cash[args] | ||
| else: | ||
| print("Calculating new result") | ||
| result = func(*args, **kwargs) |
There was a problem hiding this comment.
This uses only args as the cache key and completely ignores kwargs, so calls that differ only by keyword arguments will be cached incorrectly. The task requires a decorator that "stores results of completed runs with different arguments, number of arguments can be also different." Consider creating a composite, hashable key that includes both positional and keyword arguments (for example: key = (args, frozenset(kwargs.items()))).
| def cache(func: Callable) -> Callable: | ||
| # Write your code here | ||
| pass | ||
| cash = {} |
There was a problem hiding this comment.
Variable name cash looks like a typo and doesn't follow the checklist requirement: "Use descriptive and correct variable names." Rename it to a clearer name such as cache or storage to improve readability and correctness.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because it correctly implements the required cache decorator and satisfies the task’s functional requirements. In app/main.py, your decorator keeps a separate cache_store per decorated function, builds a composite key from both positional and keyword arguments, and prints the correct messages when computing or reusing results, so repeated calls with the same arguments avoid unnecessary recomputation. One small improvement to consider is documenting that all arguments must be hashable (e.g., in a short docstring on the decorator), because passing mutable/unhashable values like lists would raise a TypeError when used as dictionary keys. Another optional enhancement is to wrap your inner wrapper function with functools.wraps(func) so that the original function’s __name__ and __doc__ are preserved after decoration. Overall, you’ve implemented the core behavior cleanly and concisely—nice work on meeting the requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.