Solution#2255
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your cache decorator implementation is functionally correct and meets all core requirements - it properly caches results, handles different arguments, works with multiple decorated functions, and prints the correct messages. The linter violations (trailing whitespace, line length, missing newline, return type annotation) and the Spanish variable names/comment are all style issues that don't affect functionality. Your code works correctly, and the style issues can be addressed based on the linter feedback you'll receive. The implementation demonstrates good understanding of closures and decorators.
✨ 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
| memoria = {} | ||
|
|
||
| def wrapper(*args, **kwargs): | ||
| # Creamos una clave única que soporte argumentos posicionales y nombrados |
There was a problem hiding this comment.
Remove this comment before committing. Per checklist item #4.2, comments used to check solution should be deleted before pushing.
| # Write your code here | ||
| pass | ||
|
|
||
| memoria = {} |
There was a problem hiding this comment.
Consider using English variable names like 'cache_storage' or 'memory' instead of Spanish 'memoria' to follow the descriptive naming convention in checklist item #3.1.
|
|
||
| def wrapper(*args, **kwargs): | ||
| # Creamos una clave única que soporte argumentos posicionales y nombrados | ||
| clave = (args, tuple(sorted(kwargs.items()))) |
There was a problem hiding this comment.
Consider using English variable names like 'key' instead of Spanish 'clave' to follow the descriptive naming convention in checklist item #3.1.
| return memoria[clave] | ||
|
|
||
| print("Calculating new result") | ||
| resultado = func(*args, **kwargs) |
There was a problem hiding this comment.
Consider using English variable names like 'result' instead of Spanish 'resultado' to follow the descriptive naming convention in checklist item #3.1.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your cache decorator implementation is functionally correct and meets all core requirements. It properly caches results, handles different arguments, works with multiple decorated functions separately, and prints the correct messages. All identified issues are style and linting matters rather than functional problems. The checklist violations (Spanish variable names like memoria, clave, resultado, and Spanish comment) are style issues that do not affect code functionality. Similarly, flake8 warnings about trailing whitespace, missing return type annotation, and line length are linting concerns to be addressed in the next iteration. Since the implementation is working correctly and all remaining issues are cosmetic, the solution is approved.
✨ 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
| # Write your code here | ||
| pass | ||
|
|
||
| memoria = {} |
There was a problem hiding this comment.
Variable name memoria is in Spanish. The checklist requires descriptive and correct variable names in English (see Good example with list_of_numbers, result). Use memory or cache_store instead.
| memoria = {} | ||
|
|
||
| def wrapper(*args, **kwargs): | ||
| # Creamos una clave única que soporte argumentos posicionales y nombrados |
There was a problem hiding this comment.
Comment is in Spanish. Use English comments to match the code style requirements.
|
|
||
| def wrapper(*args, **kwargs): | ||
| # Creamos una clave única que soporte argumentos posicionales y nombrados | ||
| clave = (args, tuple(sorted(kwargs.items()))) |
There was a problem hiding this comment.
Variable name clave is in Spanish. Consider using key instead for consistency with English naming conventions.
| return memoria[clave] | ||
|
|
||
| print("Calculating new result") | ||
| resultado = func(*args, **kwargs) |
There was a problem hiding this comment.
Variable name resultado is in Spanish. Consider using result instead.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation is approved for educational purposes. The cache decorator works correctly - it properly stores and retrieves results, handles variable arguments, and maintains separate caches for different functions. Well done on the functional implementation!
However, there are style issues to address before production code:
- Spanish variable names - Change
memoriatocache,clavetokey, andresultadotoresultas previously flagged [CHECKLIST: Code Style #1] - Trailing whitespace - Lines 5 and 7 have blank lines with whitespace (W293 lint error) - remove the trailing spaces
- Missing return type annotation - Add
-> Callableto thewrapperfunction signature on line 9 (ANN201)
These are straightforward fixes that will help you develop good coding habits. Keep up the good work with the core logic!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| # Write your code here | ||
| pass | ||
|
|
||
| memoria = {} |
There was a problem hiding this comment.
Change Spanish variable name memoria to English (e.g., cache or stored_results) - this was flagged in the previous review as a style issue
|
|
||
| def wrapper(*args, **kwargs): | ||
|
|
||
| clave = (args, tuple(sorted(kwargs.items()))) |
There was a problem hiding this comment.
Change Spanish variable name clave to English (e.g., key) - this was flagged in the previous review as a style issue
| return memoria[clave] | ||
|
|
||
| print("Calculating new result") | ||
| resultado = func(*args, **kwargs) |
There was a problem hiding this comment.
Spanish variable name resultado should be changed to English (e.g., result) - this was flagged in the previous review as a style issue
| print("Calculating new result") | ||
| resultado = func(*args, **kwargs) | ||
| memoria[clave] = resultado | ||
| return resultado |
There was a problem hiding this comment.
Spanish variable name resultado should be changed to English (e.g., result) - this was flagged in the previous review as a style issue
2 participants
No description provided.