Create cache decorator #1427
Create cache decorator #1427
Conversation
There was a problem hiding this comment.
Nice job!
But a few things to note: two return statements are unnecessary—you can go with just one. Like this:
if cache_data_check in cached_data:
print("Getting from cache")
else:
result = func(*args)
cached_data.update({cache_data_check: result})
print("Calculating new result")
return cached_data[cache_data_check]This way, when you reach the return statement, it's guaranteed that the key you want to store is there, because it was either already there, or you just added it if it wasn't.
It's a minor detail, but good to know)
Also, you have a few typos:
Not "cash" but "cache".
Not "cashed" but "cached." Cash is money)
Oh, and your wrapper doesn't actually return None, as you specified in the type hint. It returns whatever the decorated function returns. This is why you should use typing.Any as the return type. Like this:
from typing import Callable, Any
...
def wrapper(*args) -> Any:
...Otherwise, as I said, great work! Fix this minor stuff, and it'll be outright perfect)
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
Too many changes in the pull request. Please make sure that you have added only the necessary changes to the pull request.
[CODE: 6]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
Too many changes in the pull request. Please make sure that you have added only the necessary changes to the pull request.
[CODE: 6]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
Too many changes in the pull request. Please make sure that you have added only the necessary changes to the pull request.
[CODE: 6]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
Too many changes in the pull request. Please make sure that you have added only the necessary changes to the pull request.
[CODE: 6]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
app/main.py
Outdated
|
|
||
| def wrapper(*args) -> None: | ||
|
|
||
| cache_data_check = args |
app/main.py
Outdated
| pass | ||
| cached_data = {} | ||
|
|
||
| def wrapper(*args) -> None: |
There was a problem hiding this comment.
It looks that you can return not only None
app/main.py
Outdated
| result = func(*args) | ||
| cached_data.update({cache_data_check: result}) |
There was a problem hiding this comment.
Try to write it into one line, and think about do you need an update method here, or it can be simplified
app/main.py
Outdated
| pass | ||
| cached_data = {} | ||
|
|
||
| def wrapper(*args) -> Callable: |
There was a problem hiding this comment.
Wrapper return value from function that was calculated, looks like Any type, not Callable here
app/main.py
Outdated
| if args in cached_data: | ||
| print("Getting from cache") | ||
| return cached_data[args] | ||
| else: | ||
| print("Calculating new result") | ||
| cached_data[args] = func(*args) | ||
| return cached_data[args] |
There was a problem hiding this comment.
double return looks bad, maybe we try can to use only return one here?
No description provided.