Solution #1781
Solution #1781
Conversation
There was a problem hiding this comment.
Thank you for your hard work on the Distance class! 🚀 While your implementation is mostly correct, there are a few critical issues that need to be addressed before we can approve the pull request. The main concern is with the type annotations. Using any is too vague and doesn't align with best practices. It's important to use Union[Distance, int, float] for the other parameter in arithmetic and comparison methods to ensure type safety and clarity. Additionally, handling non-numeric types by raising a TypeError will make your code more robust. Please make these changes, and your code will be in great shape! Keep up the great work! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
app/main.py
Outdated
| class Distance: | ||
| # Write your code here | ||
| pass | ||
| def __init__(self, km: any) -> None: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union from the typing module to specify the types more accurately. Consider using Union[Distance, int, float] for the km parameter.
app/main.py
Outdated
| def __repr__(self) -> str: | ||
| return f"Distance(km={self.km})" | ||
|
|
||
| def __add__(self, other: any) -> Distance: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
app/main.py
Outdated
| elif isinstance(other, (int, float)): | ||
| return Distance(self.km + other) | ||
|
|
||
| def __iadd__(self, other: any) -> Distance: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
app/main.py
Outdated
| self.km += other | ||
| return self | ||
|
|
||
| def __mul__(self, other: any) -> Distance: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
app/main.py
Outdated
| def __mul__(self, other: any) -> Distance: | ||
| return Distance(self.km * other) | ||
|
|
||
| def __truediv__(self, other: any) -> Distance: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
app/main.py
Outdated
| def __truediv__(self, other: any) -> Distance: | ||
| return Distance(round(self.km / other, 2)) | ||
|
|
||
| def __lt__(self, other: any) -> bool: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
app/main.py
Outdated
| def __lt__(self, other: any) -> bool: | ||
| return self.km < other | ||
|
|
||
| def __gt__(self, other: any) -> bool: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
app/main.py
Outdated
| def __gt__(self, other: any) -> bool: | ||
| return self.km > other | ||
|
|
||
| def __eq__(self, other: any) -> bool: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
app/main.py
Outdated
| def __eq__(self, other: any) -> bool: | ||
| return self.km == other | ||
|
|
||
| def __le__(self, other: any) -> bool: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
app/main.py
Outdated
| def __le__(self, other: any) -> bool: | ||
| return self.km <= other | ||
|
|
||
| def __ge__(self, other: any) -> bool: |
There was a problem hiding this comment.
The type annotation any is not specific enough. According to the checklist, you should use Union[Distance, int, float] for the other parameter to specify the types more accurately.
There was a problem hiding this comment.
Great job on implementing the Distance class! 🎉 While your solution meets most of the task requirements, there are a few critical issues to address for improved functionality and type safety:
-
Type Annotations: Update the type annotations for the
otherparameter in arithmetic and comparison methods to useUnion[int, float, Distance]instead of string annotations. This change will enhance clarity and type safety. -
Handling Non-Numeric Types: Ensure that the code raises a
TypeErrorwhenotheris not an instance ofDistance,int, orfloat. This will prevent unexpected behavior during arithmetic operations. -
Return Type for
__mul__and__truediv__: Modify the return type for these methods toDistanceinstead of includingNone. The methods should always return aDistanceobject when the operation is valid.
These adjustments are crucial for aligning with task requirements and ensuring robust code. Keep up the good work, and make sure to address these points in future iterations to enhance your coding skills! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| def __repr__(self) -> str: | ||
| return f"Distance(km={self.km})" | ||
|
|
||
| def __add__(self, other: "int| float | Distance") -> "Distance": |
There was a problem hiding this comment.
The type annotation for the other parameter should use Union from the typing module instead of string annotations. Change "int| float | Distance" to Union[int, float, Distance] for better type safety and clarity.
| return Distance(self.km + other.km) | ||
| return Distance(self.km + other) | ||
|
|
||
| def __iadd__(self, other: "int| float | Distance") -> "Distance": |
There was a problem hiding this comment.
Similar to the __add__ method, update the type annotation for the other parameter to use Union[int, float, Distance] instead of string annotations.
| self.km += other | ||
| return self | ||
|
|
||
| def __mul__(self, other: int | float) -> "Distance | None": |
There was a problem hiding this comment.
The return type annotation should not include None. The method should return a Distance object or raise a TypeError if the operation is not possible. Update the return type to Distance.
| if isinstance(other, (int, float)): | ||
| return Distance(self.km * other) | ||
|
|
||
| def __truediv__(self, other: int | float) -> "Distance | None": |
There was a problem hiding this comment.
Similar to the __mul__ method, the return type annotation should not include None. The method should return a Distance object or raise a TypeError if the operation is not possible. Update the return type to Distance.
No description provided.