Implement Distance class with all required magic methods#2797
Implement Distance class with all required magic methods#2797Tiken-dev wants to merge 2 commits into
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
The Distance class implementation is correct and all runtime functionality works as expected - addition, in-place addition, multiplication, division, and all comparison operators handle both Distance and numeric types properly. The code passes functional requirements.
However, there are two categories of issues to address in future work:
Type annotation corrections needed:
- The
__iadd__method should returnDistanceinstead ofint | float - Parameters like
other: intin__mul__,__truediv__, and comparison operators should beother: int | float
Style checklist violations:
- Several methods have unnecessary
elseblocks afterreturnstatements (__add__,__iadd__,__eq__,__lt__,__gt__,__le__,__ge__) - these should use early return pattern per checklist item #1
These are type safety and code style improvements rather than functional blockers, so your solution meets the core requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| else: | ||
| self.km += other | ||
| return self | ||
|
|
There was a problem hiding this comment.
The type annotation other: int doesn't match requirements - should accept both int and float. Change to other: int | float.
|
|
||
| def __mul__(self, other: int) -> Distance: | ||
| return Distance(self.km * other) | ||
|
|
There was a problem hiding this comment.
The type annotation other: int doesn't match requirements - should accept both int and float. Change to other: int | float.
| else: | ||
| return Distance(self.km + other) | ||
|
|
||
| def __iadd__(self, other: Distance) -> int | float: |
There was a problem hiding this comment.
The return type annotation -> int | float is incorrect - __iadd__ should return Distance instance (self). Change to -> Distance.
| def __add__(self, other: Distance) -> Distance: | ||
| if isinstance(other, Distance): | ||
| return Distance(self.km + other.km) | ||
| else: |
There was a problem hiding this comment.
Checklist item #1 violation: unnecessary else statement. Use early return pattern instead:
if isinstance(other, Distance):
return Distance(self.km + other.km)
return Distance(self.km + other)| def __iadd__(self, other: Distance) -> int | float: | ||
| if isinstance(other, Distance): | ||
| self.km += other.km | ||
| else: |
There was a problem hiding this comment.
Checklist item #1 violation: unnecessary else statement. Use early return pattern instead:
if isinstance(other, Distance):
self.km += other.km
return self
self.km += other
return self|
|
||
| def __lt__(self, other: Distance) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km < other.km |
There was a problem hiding this comment.
Checklist item #1 violation: unnecessary else statement. Use early return pattern instead:
if isinstance(other, Distance):
return self.km < other.km
return self.km < other|
|
||
| def __gt__(self, other: Distance) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km > other.km |
There was a problem hiding this comment.
Checklist item #1 violation: unnecessary else statement. Use early return pattern instead:
if isinstance(other, Distance):
return self.km > other.km
return self.km > other|
|
||
| def __eq__(self, other: Distance) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km == other.km |
There was a problem hiding this comment.
Checklist item #1 violation: unnecessary else statement. Use early return pattern instead:
if isinstance(other, Distance):
return self.km == other.km
return self.km == other|
|
||
| def __le__(self, other: Distance) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km <= other.km |
There was a problem hiding this comment.
Checklist item #1 violation: unnecessary else statement. Use early return pattern instead:
if isinstance(other, Distance):
return self.km <= other.km
return self.km <= other|
|
||
| def __ge__(self, other: Distance) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km >= other.km |
There was a problem hiding this comment.
Checklist item #1 violation: unnecessary else statement. Use early return pattern instead:
if isinstance(other, Distance):
return self.km >= other.km
return self.km >= other2 participants
No description provided.