Solution #1914
Solution #1914
Conversation
There was a problem hiding this comment.
Review Summary
I am approving your solution so you can move forward, but there's an important concept to review for your decorator.
You've done a great job implementing the cache decorator! The use of a closure to maintain separate caches for each function is excellent, and the core logic for storing and retrieving results works well.
The main area for improvement is how the cache_key is created in app/main.py. Your current method doesn't handle cases where a function is called with the same arguments but in different ways (e.g., func(1, 2) vs. func(a=1, b=2)). These calls are equivalent but will produce different cache keys, leading to unnecessary recalculations. A more robust approach would create a canonical representation of the arguments that is consistent regardless of how they are passed.
This is a subtle but important detail in writing powerful decorators. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| @wraps(func) | ||
| def wrapper(*args, **kwargs) -> Any: | ||
| sorted_kwargs_tuple = tuple(sorted(kwargs.items())) | ||
| cache_key = args + sorted_kwargs_tuple |
There was a problem hiding this comment.
Your current method for creating cache_key is not robust enough for all argument-passing styles. For a function like def func(a, b): ..., the calls func(1, 2) and func(a=1, b=2) are equivalent, but they will produce different cache keys with this logic. This means you would unnecessarily recalculate the result. A more robust solution would create a canonical representation of the arguments that is the same for both calls.
| print("Calculating new result") | ||
| result = func(*args, **kwargs) | ||
| func_cache[cache_key] = result | ||
| return result |
There was a problem hiding this comment.
try to use only one return in wrapper func
3 participants
No description provided.