Skip to content

Solution#2201

Open
VadymMoldovan wants to merge 3 commits intomate-academy:masterfrom
VadymMoldovan:develop
Open

Solution#2201
VadymMoldovan wants to merge 3 commits intomate-academy:masterfrom
VadymMoldovan:develop

Conversation

@VadymMoldovan
Copy link
Copy Markdown

Task solved

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

I am requesting changes because, while your cache decorator behaves correctly overall, there is one checklist violation and a few issues to address before this can be considered complete. First, in app/main.py you still have demonstration calls at the bottom of the file that run on import (the calls to your cached functions), which conflicts with the requirement to remove debugging/demo code before committing [CHECKLIST ITEM]; please either delete these calls or move them under an if __name__ == "__main__": guard so the module has no side effects on import. Second, flake8 is failing on line 22 due to single-letter variable names a, b, and c; rename these to descriptive names that reflect their purpose and update all their usages so the linter passes. Also, your wrapper is annotated with the builtin any type instead of typing.Any, and long_time_func_2 is annotated as returning int even though it returns a list; updating these annotations will make your type hints accurate and avoid confusion. Since the decorator relies on arguments being hashable, consider documenting this expectation or catching the TypeError where you build the cache key so users get a clear message if they pass mutable arguments. On the positive side, your per-function caching behavior, the required print messages, and handling of repeated arguments all look solid and match the core functionality requirements. Once you address the checklist violation and the small issues above, your solution will be in great shape.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

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

I am requesting changes because, while your cache decorator works correctly and meets the functional requirements, there is still a critical checklist violation and a couple of type-hint issues to address before this can be considered complete. The critical issue is that app/main.py runs demonstration calls at module import time (near the bottom of the file where you call long_time_func_1 and long_time_func_2 directly); these side effects must either be removed or wrapped in an if __name__ == "__main__": block so the module is import-safe and complies with the requirement to remove debug/demo code before committing [CHECKLIST ITEM #1]. In your decorator wrapper, the return type is annotated as -> any, but Python’s built-in any is a function, not a type; instead, import and use typing.Any (for example, from typing import Any, Callable) and update the annotation to -> Any so type checkers and readers are not misled. Similarly, long_time_func_2 is annotated as returning int, but it actually returns a list comprehension; adjust its signature to -> list or -> List[int] and import List from typing if you choose the generic form. As an optional improvement, you might document that the decorator assumes all arguments are hashable (immutable) or catch TypeError when building the cache key and raise a clearer message if users pass mutable arguments. Once these issues are fixed, your solution will both satisfy the checklist and present a clean, well-typed implementation of the caching behavior you’ve already implemented successfully.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread app/main.py Outdated
stored_results = {}

@wraps(func)
def wrapper(*args, **kwargs) -> 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 is annotated with the builtin any type (-> any) which is incorrect. Use typing.Any (capital A) and add it to the imports (for example: from typing import Callable, Any). This will make the type hint accurate.

Comment thread app/main.py Outdated


@cache
def long_time_func_2(n_tuple: tuple, power: int) -> int:
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 function long_time_func_2 is annotated as returning int but actually returns a list ([number ** power for number in n_tuple]). Update the return annotation to list or List[int] and import List from typing if you choose List[int].

Comment thread app/main.py
Comment on lines +31 to +36
long_time_func(1, 2, 3)
long_time_func(2, 2, 3)
long_time_func_2((5, 6, 7), 5)
long_time_func(1, 2, 3)
long_time_func_2((5, 6, 7), 10)
long_time_func_2((5, 6, 7), 10)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These calls run at import time and produce side effects. Remove these demonstration calls or move them under an if __name__ == "__main__": guard so the module has no side effects on import. This violates the checklist item: "Add comments, prints, and functions to check your solution when you write your code. Don't forget to delete them when you are ready to commit and push your solution."

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

I’m approving your solution as an educational override so you can keep progressing, but there is one important issue to fix for future submissions. Your cache decorator is implemented very well: it correctly caches results per function and per argument set, prints the required messages, and uses reasonable type hints. The main problem is in app/main.py where lines 31–36 run demonstration calls at the top level; this causes side effects whenever the module is imported, which violates the guideline about removing or guarding debug/demo code before committing [CHECKLIST ITEM #2]. This is problematic because test runners and other modules that import app.main will unintentionally execute those calls, potentially printing output or performing work they didn’t ask for. To fix this, move those demonstration calls inside an if __name__ == "__main__": block, or remove them entirely once you’ve finished testing, for example:

if __name__ == "__main__":
    print(slow_add(1, 2))
    print(slow_add(1, 2))

As a minor improvement, you might also document that the decorator expects hashable (immutable) arguments, and optionally tighten the type hints using Callable[..., Any] or generics. Overall, your core logic is strong; tightening these checklist-related and polish details will make your code more robust and production-ready.


✨ 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
Comment on lines +31 to +36
long_time_func(1, 2, 3)
long_time_func(2, 2, 3)
long_time_func_2((5, 6, 7), 5)
long_time_func(1, 2, 3)
long_time_func_2((5, 6, 7), 10)
long_time_func_2((5, 6, 7), 10)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the checklist item: "2. Add comments, prints, and functions to check your solution when you write your code. Don't forget to delete them when you are ready to commit and push your code." The calls at module level cause side effects on import. Move or wrap these demonstration calls (lines 31–36) inside an if __name__ == "__main__": block (or remove them) so the module is import-safe.

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