Resolução do exercício por Anderson Reichardt#2871
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still checklist violations in your type annotations and one required style fix that need to be addressed before this solution is considered complete. Your __init__ currently uses km: int, but the task and tests require supporting floats (e.g., Distance(12.5)), so you should widen this to km: int | float to correctly reflect allowed inputs [CHECKLIST ITEM #1]. Likewise, __mul__ and __truediv__ are annotated with other: int, while the tests multiply and divide by floats (e.g., 5.88, 3.3), so these parameters must be typed as other: int | float and continue to reject Distance instances [CHECKLIST ITEM #1 / Support correct types].
Several methods in app/main.py use string-based union annotations such as "Distance | int", which the checklist marks as a bad pattern; replace these with modern unions like other: Distance | int | float (no quotes) across all affected magic methods [CHECKLIST ITEM #1]. In __iadd__, there is an unnecessary else after the if isinstance(other, Distance): branch; to follow the checklist rule on avoiding unnecessary else, handle the Distance case in the if, then directly handle the numeric case afterwards without else, and return self once at the end. Functionally, your arithmetic, comparisons, and string representations are all very close to perfect, so once you fix the annotations and simplify that __iadd__ branch, your solution will fully meet the task and checklist requirements—nice work so far, you’re almost there.
✨ 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
| class Distance: | ||
| # Write your code here | ||
| pass | ||
| def __init__(self, km: int) -> None: |
There was a problem hiding this comment.
This violates checklist item Type Annotations #1: your parameter is annotated as km: int, but the tests and description require supporting floats (e.g., Distance(12.5)), so km should be typed to accept both int and float (e.g., km: int | float).
| def __repr__(self) -> str: | ||
| return f"Distance(km={self.km})" | ||
|
|
||
| def __add__(self, other: "Distance | int") -> "Distance": |
There was a problem hiding this comment.
This violates checklist item Type Annotations #1: using the string annotation "Distance | int" is considered a bad example; you should use proper union types like Distance | int | float (and drop the quotes) so the method matches the checklist’s good examples.
| def __iadd__(self, other: "Distance | int") -> "Distance": | ||
| if isinstance(other, Distance): | ||
| self.km += other.km | ||
| else: |
There was a problem hiding this comment.
Checklist item Avoid unnecessary else statements #1: after handling the Distance case, you can directly handle the numeric case without an else; removing else here will follow the recommended pattern and keep the code simpler.
|
|
||
| return self | ||
|
|
||
| def __mul__(self, other: int) -> "Distance": |
There was a problem hiding this comment.
| def __mul__(self, other: int) -> "Distance": | ||
| return Distance(self.km * other) | ||
|
|
||
| def __truediv__(self, other: int) -> "Distance": |
| value = other.km if isinstance(other, Distance) else other | ||
| return self.km < value | ||
|
|
||
| def __gt__(self, other: "Distance | int") -> bool: |
There was a problem hiding this comment.
This uses a string-based union annotation "Distance | int", which matches the checklist’s bad examples. According to Type Annotations #1, you should use proper union types like Distance | int | float (without quotes).
| value = other.km if isinstance(other, Distance) else other | ||
| return self.km > value | ||
|
|
||
| def __eq__(self, other: "Distance | int") -> bool: |
There was a problem hiding this comment.
This uses a string-based union annotation "Distance | int", which the checklist marks as a bad example; update to a proper union type (e.g., Distance | int | float) to comply with Type Annotations #1.
| value = other.km if isinstance(other, Distance) else other | ||
| return self.km == value | ||
|
|
||
| def __le__(self, other: "Distance | int") -> bool: |
There was a problem hiding this comment.
This uses a string-based union annotation "Distance | int". Per checklist item Type Annotations #1, you should use modern union annotations like Distance | int | float without quotes.
| value = other.km if isinstance(other, Distance) else other | ||
| return self.km <= value | ||
|
|
||
| def __ge__(self, other: "Distance | int") -> bool: |
There was a problem hiding this comment.
This uses a string-based union annotation "Distance | int", which is listed as a bad pattern in the checklist. To meet Type Annotations #1, change it to a proper union such as other: Distance | int | float.
| @pytest.mark.parametrize( | ||
| 'kilometers,output', | ||
| "kilometers,output", | ||
| [ |
There was a problem hiding this comment.
This violates checklist item "Avoid unnecessary else statements": after the if isinstance(other, Distance): branch, you can update self.km for the non-Distance case without using else, then return self once at the end.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because it now fully matches the task requirements and behaves correctly across all tested scenarios. Your Distance class implements all the required magic methods, and they correctly handle both Distance instances and numeric values for arithmetic and comparisons. Multiplication and division are properly restricted to int | float operands, and division results are rounded to two decimal places as specified. You also addressed the earlier checklist points by updating type annotations to modern union syntax, supporting both int and float where required, and simplifying __iadd__ to avoid unnecessary control flow. Overall, the implementation is clean, consistent, and should pass all the provided tests. Nice work incorporating the previous feedback into a polished final solution.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.