Skip to content

py_cache_decorator solution#2193

Open
neokitoss wants to merge 1 commit intomate-academy:masterfrom
neokitoss:develop
Open

py_cache_decorator solution#2193
neokitoss wants to merge 1 commit intomate-academy:masterfrom
neokitoss:develop

Conversation

@neokitoss
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

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

Review Summary

Your solution meets the core task requirements: the cache decorator correctly stores results per decorated function, avoids repeated computations for the same positional arguments, and prints exactly "Calculating new result" and "Getting from cache" in app/main.py. The logic using a per-function results dictionary is a solid design choice that keeps caches isolated and ensures correct behavior for multiple decorated functions. One improvement to consider in the wrapper definition (near the def wrapper(...): in app/main.py) is to support keyword arguments by changing the signature to def wrapper(*args, **kwargs) and building a cache key that combines args with a deterministic representation of kwargs, such as (args, tuple(sorted(kwargs.items()))). This would prevent logically equivalent calls that differ only in keyword usage or ordering from bypassing the cache. Another non-blocking enhancement is to preserve the original function’s metadata by importing wraps from functools and decorating the wrapper with @wraps(func), so attributes like __name__ and __doc__ remain accurate. Overall, your implementation is clear, avoids duplication, and satisfies the functional requirements, with these suggestions serving as good next steps for robustness and best practices.


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

Footnotes

  1. Rate AI review example

Comment thread app/main.py
pass
results = {}

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

Choose a reason for hiding this comment

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

The wrapper only accepts positional arguments. To correctly cache calls that use keyword arguments (and to avoid treating equivalent calls differently), change the signature to def wrapper(*args, **kwargs) and incorporate a hashable representation of kwargs into the cache key (for example, key = (args, tuple(sorted(kwargs.items())))).

Comment thread app/main.py
Comment on lines +8 to +9
if args not in results:
results[args] = func(*args)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code uses args alone as the dictionary key. If you add **kwargs support, update key construction here and where you store/lookup results so both positional and keyword arguments influence the cache key consistently and deterministically.

Comment thread app/main.py
print("Getting from cache")
return results[args]

return wrapper
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider preserving the decorated function's metadata by using functools.wraps. Add from functools import wraps at the top and decorate wrapper with @wraps(func) so __name__, __doc__, etc. are preserved.

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