Skip to content

Solution DK#2872

Open
DarinaK-k wants to merge 2 commits into
mate-academy:masterfrom
DarinaK-k:develop
Open

Solution DK#2872
DarinaK-k wants to merge 2 commits into
mate-academy:masterfrom
DarinaK-k:develop

Conversation

@DarinaK-k

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

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 there are still a few critical mismatches with the task’s type and interface requirements that would block full completion. The biggest issues are in app/main.py: __mul__ and __truediv__ are both annotated to take a Distance argument, but the checklist explicitly requires them to accept only numeric literals (int | float) and not Distance instances; you should update their signatures and keep the behavior as distance * number and distance / number (with rounding to 2 decimals in division). Similarly, __add__, __iadd__, and all comparison methods currently annotate other as just Distance, but the task requires them to work with both Distance and numeric literals, so the type hints should be widened to something like Distance | int | float to match their actual behavior. In __iadd__, the if ... else ... structure matches the “unnecessary else” pattern called out in the checklist; you can fix this by handling the Distance case inside the if and then handling the numeric case after the if without an else.

Most of your runtime logic is already correct (addition, in-place addition, comparisons, __str__, __repr__, and rounding behavior in division all look good), so the main work is aligning type annotations and small structural details with the requirements. Once you adjust these method signatures and refactor the __iadd__ control flow, your solution should fully satisfy the task’s expectations.


✨ 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
def __repr__(self) -> str:
return f"Distance(km={self.km})"

def __add__(self, other: Distance) -> Distance:

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 __add__ annotation only allows Distance, but the task requires supporting addition with numeric literals as well; update the type hint for other to include int | float so it matches the required behavior.

Comment thread app/main.py Outdated
def __iadd__(self, other: Distance) -> Distance:
if isinstance(other, Distance):
self.km += other.km
else:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In __iadd__, the checklist asks to avoid unnecessary else branches; this else matches the "Bad example" pattern. You can handle the non-Distance case after the if without using else.

Comment thread app/main.py Outdated
self.km += other
return self

def __mul__(self, other: Distance) -> Distance:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

__mul__ must not accept a Distance instance according to checklist item #1 under "Support correct types"; change other to be int | float and keep multiplication by numeric literal only.

Comment thread app/main.py Outdated
def __mul__(self, other: Distance) -> Distance:
return Distance(self.km * other)

def __truediv__(self, other: Distance) -> Distance:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

__truediv__ must accept only a numeric literal (int | float) and not a Distance instance, per the checklist; adjust the annotation accordingly while keeping the rounding to 2 decimals.

Comment thread app/main.py

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because the Distance class now fully matches the task requirements and behaves correctly for all specified operations. Initialization with Distance(km) stores the value in self.km as required, and both __str__ and __repr__ produce exactly the expected string formats, supporting clear printing and debugging. Arithmetic methods (__add__, __iadd__, __mul__, __truediv__) correctly handle Distance instances and numeric values where appropriate, with division returning a new Distance rounded to two decimals and never mutating the original instance. All comparison magic methods use self.km internally and work with both Distance and numeric types, which makes the class practical for sorting, threshold checks, and other comparisons. You also aligned well with the checklist constraints, such as restricting multiplication and division to numeric arguments only and cleaning up unnecessary else branches after early returns. Overall, this is a solid and idiomatic implementation of the required magic methods—nice work bringing everything in line with the specifications.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

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.

3 participants