Solution#2849
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 several core behaviors of your Distance class still don’t match the task requirements and are causing test failures. The string methods need precise formatting: __str__ must return "Distance: {self.km} kilometers." (no space before the colon and with the trailing period), and __repr__ must be "Distance(km={self.km})" with the closing parenthesis. Arithmetic methods must return new Distance instances without mutating self: __mul__ should compute self.km * other and return Distance(result), and __truediv__ should return Distance(round(self.km / other, 2)); both should accept int | float and return NotImplemented for unsupported types. Your comparison methods (__lt__, __gt__, __eq__, __le__, __ge__) should all return bool and have -> bool annotations, and they should avoid unnecessary elif branches by using if and a final return NotImplemented when other is neither Distance nor a number [CHECKLIST ITEM #1, #2]. Also, because you annotate methods to return Distance, you should either add from __future__ import annotations at the top or adjust the annotations to avoid forward references [CHECKLIST ITEM #2]. Once you fix these functional and annotation issues, your implementation will align well with the task and should pass the tests; the overall structure is solid, so focus on these targeted corrections and you’ll 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
| self.km = km | ||
|
|
||
| def __str__(self) -> str: | ||
| return f"Distance : {self.km} kilometers." |
There was a problem hiding this comment.
__str__ output must match the task exactly: "Distance: 20 kilometers." (no space before the colon). Adjust the format string to remove the extra space and ensure punctuation matches the description.
| return f"Distance : {self.km} kilometers." | ||
|
|
||
| def __repr__(self) -> str: | ||
| return f"Distance(km={self.km}" |
There was a problem hiding this comment.
__repr__ is missing the closing parenthesis, so repr(Distance(20)) won’t equal "Distance(km=20)" as required. Add the closing ) in the f-string.
| def __repr__(self) -> str: | ||
| return f"Distance(km={self.km}" | ||
|
|
||
| def __add__(self, other : Distance | int) -> Distance: |
There was a problem hiding this comment.
Using Distance directly in annotations without from __future__ import annotations can cause a NameError, and the return type here should be Distance, but you also have an unnecessary elif branch. This violates checklist item #1 (avoid unnecessary else) and checklist item #2 (proper type annotations). Consider importing annotations from __future__ and replacing elif with a simple return after the first if.
| def __mul__(self, other : int) -> Distance: | ||
| if isinstance(other, int): | ||
| self.km *= other |
There was a problem hiding this comment.
__mul__ is required to return a new Distance instance with km equal to self.km * other; it should not modify self.km. Currently you mutate self and return it, which breaks the expectation that distance2 = distance1 * 5 yields a separate object with km == 100. Also, the method should accept int | float according to the checklist.
| def __truediv__(self, other : int) -> Distance: | ||
| if isinstance(other, int): | ||
| self.km /= other |
There was a problem hiding this comment.
__truediv__ must return a new Distance with km equal to self.km / other rounded to 2 decimals, not modify self.km. Right now it mutates self and doesn’t round, so distance1 / 7 won’t produce 2.85 in a new instance as required.
| self.km /= other | ||
| return self | ||
|
|
||
| def __lt__(self, other : Distance | int) -> Distance: |
There was a problem hiding this comment.
| def __gt__(self, other : Distance | int) -> Distance: | ||
| if isinstance(other, Distance): | ||
| return self.km > other.km | ||
| elif isinstance(other, int): | ||
| return self.km > other | ||
|
|
||
| def __eq__(self, other: Distance | int) -> Distance: | ||
| if isinstance(other, Distance): | ||
| return self.km == other.km | ||
| elif isinstance(other, int): | ||
| return self.km == other | ||
|
|
||
| def __le__(self, other: Distance | int) -> Distance: | ||
| if isinstance(other, Distance): | ||
| return self.km <= other.km | ||
| elif isinstance(other, int): | ||
| return self.km <= other | ||
|
|
||
| def __ge__(self, other: Distance | int) -> Distance: |
There was a problem hiding this comment.
For __gt__, __eq__, __le__, and __ge__, the same issues apply: their return type should be bool, and the elif branches are unnecessary given you return from the if branch. This pattern matches the “bad example” in checklist item #1 about unnecessary else statements.
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 critical functionality gaps around numeric operands that will cause tests to fail. In app/main.py, your __add__ method (around where you check isinstance(other, Distance) and isinstance(other, int)) does not handle float values and lacks a final return NotImplemented, so Distance + float can return None instead of a Distance or a proper fallback; the task explicitly requires adding both int and float numbers. Similarly, __mul__ and __truediv__ are annotated to accept int | float, but the implementation only treats int specially and will incorrectly return NotImplemented for float, even though multiplication and division by floats are required and must return a Distance object with correct km (division rounded to 2 decimals). These issues are critical blockers because they violate the “Support correct types” checklist item and break arithmetic operations used in the tests.
To fix this, update __add__ so that it accepts float as well as int (e.g., using isinstance(other, (int, float))), always returns a new Distance for supported types, and ends with return NotImplemented for unsupported ones. Do the same for __mul__ and __truediv__, treating both int and float the same way (compute with self.km and wrap the result in Distance, with division rounded to two decimal places). It would also be good to extend __iadd__ and the comparison methods to accept float explicitly, since the task describes interaction with “numbers” and tests already exercise these cases. Your overall class structure, string/representation methods, and handling of Distance operands are solid; once you close these type-handling gaps, your solution should meet the requirements fully.
✨ 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
| def __add__(self, other : Distance | int) -> Distance: | ||
| if isinstance(other, Distance): | ||
| return Distance(km=self.km + other.km) | ||
| if isinstance(other, int): | ||
| return Distance(km=self.km + other) |
There was a problem hiding this comment.
__add__ is annotated to accept Distance | int, but the task requires supporting adding both Distance and a number (int or float), and with the current implementation adding a float will return None. This violates the description’s "both variants ^ are possible" (for numbers) and the checklist’s type-support expectations. Consider handling float as well and returning NotImplemented when the type is unsupported instead of falling off the end.
| def __mul__(self, other : int | float) -> Distance: | ||
| if isinstance(other, int): | ||
| return Distance(km=self.km * other) | ||
| return NotImplemented |
There was a problem hiding this comment.
__mul__ is annotated as int | float, but only int is handled; passing a float will incorrectly return NotImplemented even though floats are supposed to be supported, per the description and the "Support correct types" checklist item for arithmetic operations. You should treat both int and float similarly (compute self.km * other and wrap in Distance) and reserve NotImplemented for truly unsupported types.
| def __truediv__(self, other : int | float) -> Distance: | ||
| if isinstance(other, int): | ||
| new_dist = round((self.km / other), 2) | ||
| return Distance(km=new_dist) | ||
| return NotImplemented |
There was a problem hiding this comment.
__truediv__ similarly only supports int despite being annotated as int | float; tests may use float divisors and the description requires division by a number with rounding to 2 decimals. To meet the requirements and checklist, handle both int and float (e.g., any isinstance(other, (int, float))) and keep the Distance(round(self.km / other, 2)) behavior, returning NotImplemented only for unsupported types.
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 as an educational override so you can keep progressing while you refine it further. You’ve done a solid job implementing most magic methods correctly: __str__, __repr__, arithmetic with Distance and integers, rounding in __truediv__, and supporting floats in __iadd__, __mul__, and __truediv__ all show good understanding of operator overloading. The main issues to fix next are that __add__ currently only treats int specially (so Distance + 3.5 returns NotImplemented), and all comparison methods (__lt__, __gt__, __eq__, __le__, __ge__) only handle Distance | int, not float as required by the “numbers (int and float)” requirement [CHECKLIST ITEM #1]. To improve, update the type annotations and isinstance checks in __add__ and each comparison to use isinstance(other, (int, float)) and compare against self.km, just like you already do in __mul__ and __truediv__. As a minor refinement, double-check the division example 20 / 7 from the instructions and ensure your rounding behavior matches the expected value in the task. Overall, you’re very close—your structure is clean, the logic is mostly correct, and with these small adjustments your Distance class will fully meet the numeric-operand and checklist requirements. Keep up the good work; you’re clearly building strong intuition for Python’s magic methods.
✨ 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
| def __add__(self, other : Distance | int | float) -> Distance: | ||
| if isinstance(other, Distance): | ||
| return Distance(km=self.km + other.km) | ||
| if isinstance(other, int): | ||
| return Distance(km=self.km + other) |
There was a problem hiding this comment.
__add__ currently handles Distance and int but not float. This violates the requirement to support adding both int and float numbers and the previous review instruction to "update __add__ so that it accepts float as well as int"; consider checking isinstance(other, (int, float)) instead of only int.
| def __lt__(self, other : Distance | int) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km < other.km | ||
| if isinstance(other, int): | ||
| return self.km < other | ||
| return NotImplemented | ||
|
|
||
| def __gt__(self, other : Distance | int) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km > other.km | ||
| if isinstance(other, int): | ||
| return self.km > other | ||
| return NotImplemented | ||
|
|
||
| def __eq__(self, other: Distance | int) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km == other.km | ||
| if isinstance(other, int): | ||
| return self.km == other | ||
| return NotImplemented | ||
|
|
||
| def __le__(self, other: Distance | int) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km <= other.km | ||
| if isinstance(other, int): | ||
| return self.km <= other | ||
| return NotImplemented | ||
|
|
||
| def __ge__(self, other: Distance | int) -> bool: | ||
| if isinstance(other, Distance): | ||
| return self.km >= other.km | ||
| if isinstance(other, int): | ||
| return self.km >= other |
There was a problem hiding this comment.
All comparison methods (__lt__, __gt__, __eq__, __le__, __ge__) only accept Distance | int and treat int specifically. This conflicts with the extended requirement to support numeric operands generally (including float), so distance < 60.5 would return NotImplemented; update the annotations and isinstance checks to include float as you did in __iadd__, __mul__, and __truediv__.
Elena-Bruyako
left a comment
Elena-Bruyako
left a comment
There was a problem hiding this comment.
this HW isn't related to Java course
Yes, I know, but I didn't ask for my homework to be checked—it happened automatically after a few failed attempts. I want the AI bot to keep checking my homework, or, if that's not possible, for you to check it, and I'll continue studying. |
hladiivolodymyr
left a comment
hladiivolodymyr
left a comment
There was a problem hiding this comment.
The tests failed. Please fix the failing test cases and try again.
| if isinstance(other, Distance): | ||
| return self.km > other.km | ||
| if isinstance(other, (int, float)): | ||
| return self.km > other | ||
| return NotImplemented |
There was a problem hiding this comment.
You are performing this check many times; I recommend moving its logic into a separate method and using that method wherever it is needed.
5 participants
No description provided.